joelmoss / strano

Capistrano and Github sittin' in a tree...
http://developwithstyle.com
MIT License
579 stars 70 forks source link

Fix use Bundler.with_clean_env #21

Open goodtouch opened 11 years ago

goodtouch commented 11 years ago

Projects using Bundler may conflict with strano itself

goodtouch commented 11 years ago

As a reference about Bundler.with_clean_env: http://www.engineyard.com/blog/2011/bundler-pro-tip/

joelmoss commented 11 years ago

Has this been tested? I ask because there are some gems in the Gemfile that are need for Strano to run these "clean" commands.

goodtouch commented 11 years ago

Are you referring to those ?

# While these are not needed by Strano itself, without them installed, any project
# that requires them will die when Strano tries to run a cap task. By using
# :require => nil, these don't get required/loaded into Strano, but are installed
# for projects to use if needed.
gem 'delayed_job', :require => nil
gem 'whenever', :require => nil
gem 'airbrake', :require => nil
gem 'newrelic_rpm', :require => nil

I think this should still work as before but I'll give it a try (I'm note sure about the :require => nil) and come back to you.

The problem I had was on a project doing a local clone of the git repo, assets:precompile then rsync to a remote server.

The assets:precompile task will run this:

run_locally "cd .rsync_cache; RAILS_ENV=#{rails_env} bundle exec rake #{asset_env} assets:precompile"

and will fail because of some ENV[BUNDLE_*] variable being set while running bundled sidekiq.

If I don't find a way to fix this around Strano::CLI or Capistrano::CLI, I guess I could still monkey patch run_locally to add the Bundler.with_clean_env wrapper and reduce side effect on projects requiring the aforementioned gems if you prefer.

joelmoss commented 11 years ago

Bundler.with_clean_env will run your code without any of the Gemfile's gems loaded. Meaning that even Capistrano will not be loaded. So its not really about those 4 gems you mention above. It's about any of them gems that Strano itself needs.

goodtouch commented 11 years ago

Oh I finally understood what you meant by "Has this been tested?" :smile: Yes, already required gems are not unloaded, and Strano still works pretty fine with this.

It provides a clean environment (without any bundle related variables) for the block by replacing it to what it was before loading bundler (and restoring it after). (https://github.com/carlhuda/bundler/blob/master/lib/bundler.rb#L195).

Yet I think this will fail for the :require => nil gems if they are required in the project capfile as it won't be abe to find related files.

I will rewrite this patch to just wrap the run_locally capistrano method as it makes more sense and will not break the existing.