chef-cookbooks / iptables

Development repository for Chef Cookbook iptables
https://supermarket.chef.io/cookbooks/iptables
Apache License 2.0
102 stars 141 forks source link

use a system-independent shebang line in rebuild-iptables script #19

Closed WattsInABox closed 8 years ago

WattsInABox commented 10 years ago

Fixes #18

chef-supermarket commented 10 years ago

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.

chef-supermarket commented 10 years ago

Hi. Your friendly Curry bot here. Just letting you know that all commit authors have become authorized to contribute. I have added the "Signed CLA" label to this issue so it can easily be found in the future.

juliandunn commented 9 years ago

So... it's not quite that easy, because Ruby isn't necessarily in the path on systems that don't otherwise have Ruby but only have the Chef Client full-stack-installer package there. That's why there's this "complication".

For example, I have a Fedora system which only has the RPM. Invoking /usr/bin/env ruby on this box doesn't find Ruby, and the binstubs for Chef itself in /usr/bin/chef-{apply,client,solo,shell} have a hardcoded hashbang to /opt/chef/embedded/bin/ruby for this purpose.

WattsInABox commented 9 years ago

Well that's weird cause the iptable stuff didn't work on our centos systems until I made this change...

WattsInABox commented 9 years ago

Thanks for the education on the system-independence issue, though. So how else can we fix it? Make the code a little more complicated to try and find the right ruby?

I can also try that hardcoded hashbang line to see if it works now. Maybe I was doing something else wrong.

juliandunn commented 9 years ago

I kind of feel like the current behavior is adequate; either that, or just substitute the second term in the ternary expression to /usr/bin/env ruby.

tas50 commented 8 years ago

At it stands this breaks chef users that don't have a system ruby installed. I think something could be put together to check to see if /usr/bin/env ruby returns true and if not use chef's ruby, but I'm going to close this PR out for now.

WattsInABox commented 8 years ago

so maybe changing this

variables(      
   :hashbang => ::File.exist?('/usr/bin/ruby') ? '/usr/bin/ruby' : '/opt/chef/embedded/bin/ruby'        
)

to something like this

def system_ruby
  '/usr/bin/env ruby'
end

def system_ruby_installed?
  `#{system_ruby} --help` != ''
end

variables(    
   :hashbang => system_ruby_installed? ? system_ruby : '/opt/chef/embedded/bin/ruby'    
)

?