Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added "compile_time false if respond_to?(:compile_time)" #72

Closed
wants to merge 1 commit into from

Conversation

osgrob
Copy link

@osgrob osgrob commented Nov 4, 2015

I was having trouble including the lvm cookbook as part of a run list in an environment that does not have direct access to ruby-gems.org, but configures the instance to use nexus as part of the run list.

i.e. The gem cannot be installed compile time, but after certain other resources in the run list have converged the chef-gem resource is able to install the required gems.

I have tested that adding "compile_time false if respond_to?(:compile_time)" to both chef-gems to insure compile_time installation is disabled as discussed: https://docs.chef.io/resource_chef_gem.html both passes existing kitchen tests and resolves the issue in my environment.

Would this be a good addition to the lvm cookbook?

@chef-supermarket
Copy link

Hi. I am an automated pull request bot named Curry. There are commits in this pull request whose authors are not yet authorized to contribute to Chef Software, Inc. projects or are using a non-GitHub verified email address. To become authorized to contribute, you will need to sign the Contributor License Agreement (CLA) as an individual or on behalf of your company. You can read more on Chef's blog.

GitHub Users Who Are Not Authorized To Contribute

The following GitHub users do not appear to have signed a CLA:

Please sign the CLA here.

@scalp42
Copy link

scalp42 commented Nov 4, 2015

@osgrob I believe you need:

compile_time false if Chef::Resource::ChefGem.instance_methods(false).include?(:compile_time)

See sous-chefs/aws#110 for background.

@roblperry
Copy link
Contributor

@scalp42

https://docs.chef.io/resource_chef_gem.html says that what you suggested is for when on is using chef-sugar older than version 3.0.1. I am not using chef-sugar and my original change works flawlessly for me, but you still may have a good point. I don't want to make life bad for anyone who is using chef-sugar.

I'll try to test that code you suggested later to see what it does without chef-sugar.

Thanks
Rob

@scalp42
Copy link

scalp42 commented Nov 4, 2015

Your change won't work with older chef-sugar versions (which used to have the same compile_time method). Newer versions of chef-sugar has the method renamed.

Trust me, it's needed 😄

See http://jtimberman.housepub.org/blog/2015/03/20/chef-gem-compile-time-compatibility/ also.

@roblperry
Copy link
Contributor

@scalp42

Trusted

@osgrob osgrob closed this Nov 4, 2015
@scalp42
Copy link

scalp42 commented Nov 4, 2015

@roblperry I didn't mean you should close the PR, it's still valid I'm assuming, just need to make the changes mentioned.

@osgrob
Copy link
Author

osgrob commented Nov 4, 2015

@scalp42

I was going to have to close it anyway. I need to make the changes as roblperry instead of osgrob. But, I'll wasn't sure what would happen when I did, as I had not closed one before, and wanted to finish out conversation first.

Even though, I'm very confident of the change you suggested, I'll still want to retest and I don't know what timeline that will happen on, as I was just trying to squeeze this in between some other items.

Thanks for helping me not hurt the chef-sugar < 3.0.1 folks.

@scalp42
Copy link

scalp42 commented Nov 4, 2015

@osgrob this is the reason why it seems to be working for you: https://github.com/sethvargo/chef-sugar/blob/master/CHANGELOG.md#breaking-changes

Any version before that one will break.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants