boxen / our-boxen

Copy me for your team.
https://github.com/boxen/our-boxen/
MIT License
2.58k stars 883 forks source link

boxen module exists twice in module paths #789

Closed jarrettone closed 8 years ago

jarrettone commented 8 years ago

It appears the boxen module appears twice in the module search path. The below output shows that the custom facter facts are being picked up twice, resulting in an error about a constant and the facts.d dir being parsed twice.

$ boxen
--> Preparing to auto-update...
--> Complete! Nothing new from upstream.
Fact file /opt/boxen/repo/facts.d/example.yaml was parsed but returned an empty data set
/opt/boxen/repo/shared/boxen/lib/facter/boxen_facts_d.rb:11: warning: already initialized constant BoxenFactsDirectoryLoader::EXTERNAL_FACT_WEIGHT
/opt/boxen/repo/modules/boxen/lib/facter/boxen_facts_d.rb:11: warning: previous definition of EXTERNAL_FACT_WEIGHT was here
Fact file /opt/boxen/repo/facts.d/example.yaml was parsed but returned an empty data set
Notice: Compiled catalog for [REDACTED] in environment production in 1.98 seconds

Is this intentional?

eugeneius commented 8 years ago

/opt/boxen/repo/modules/boxen/ shouldn't exist; it certainly doesn't in this repo: https://github.com/boxen/our-boxen/tree/master/modules

You can safely remove that directory, which will fix the warning.

jarrettone commented 8 years ago

I think it got pulled in in one of the librarian-puppet runs. I'll rebuild from scratch and verify.

jarrettone commented 8 years ago

@eugeneius looks like it gets pulled in as part of the librarian-puppet run when trying to update to current our-boxen per the FAQ*. This seems like either the Puppetfile should not include boxen and the module should just be part of shared or it shouldn't be in shared.

eugeneius commented 8 years ago

It looks like that FAQ entry is wrong.

When you run the boxen command, it tells librarian-puppet to install modules into the shared/ directory: https://github.com/boxen/boxen/blob/2.8.0/lib/boxen/puppeteer.rb#L105

The FAQ entry is telling you to run librarian-puppet without passing the --path option, which means it will use the default path for modules - which is modules/: https://github.com/rodjek/librarian-puppet/blob/v1.0.9/lib/librarian/puppet/environment.rb#L20

I've never run into this problem because I always run boxen, whether I'm integrating upstream changes or making them myself.

Do you want to submit a pull request adding that flag to the command in the FAQ?

jacobbednarz commented 8 years ago

@eugeneius That would be great if you could! :tada:

eugeneius commented 8 years ago

I was suggesting that @jarrettone could do it... but okay :grin: https://github.com/boxen/our-boxen/pull/811

jacobbednarz commented 8 years ago

ah, my apologies! thanks.

jarrettone commented 8 years ago

Apologies gents, been away for a while. Toddlers are time thieves.

I was about to say that I'll gladly update the faq, but @eugeneius beat me to it. @jacobbednarz any chance on getting that merged in the not-too-distant? Is there more that needs to be done here to get it merged?

jacobbednarz commented 8 years ago

Consider it done. Thanks! :)