Shopify / bootboot

Dualboot your Ruby app made easy
MIT License
416 stars 35 forks source link

Patch bundler to allow different rubies #30

Closed jacobthemyth closed 4 years ago

jacobthemyth commented 4 years ago

This allows using bootboot for dual booting different versions of Ruby. For example:

source "https://rubygems.org"

plugin 'bootboot'
Plugin.send(:load_plugin, 'bootboot') if Plugin.installed?('bootboot')

if ENV['DEPENDENCIES_NEXT']
  enable_dual_booting if Plugin.installed?('bootboot')
  ruby '2.5.7'
else
  ruby '2.4.9'
end

I updated the env var from SKIP_BUNDLER_PATCH to BOOTBOOT_UPDATING_ALTERNATE_LOCKFILE because I thought it would be confusing if the RubyVersion patch took effect when that var was present, which is what needs to happen.

jacobthemyth commented 4 years ago

I'm not sure why the build it failing, I think it must be something to do with the bundler version on Travis, but the tests all pass locally 🤔.

jacobthemyth commented 4 years ago

@Edouard-chin I'm not sure if you monitor this repo regularly, so just want to ping you directly 😊.

Edouard-chin commented 4 years ago

Hey, thanks for the PR ❤️ . I think this feature make sense. I don't have the bandwidth to do a proper review now, but I'll try to take some time next week to look.

jacobthemyth commented 4 years ago

Sure, thanks! I'm okay if you end up deciding it's not a good fit, but I'm using it already so figured I'd upstream it if possible.

jacobthemyth commented 4 years ago

I think the additions made to README.md also address #28

jacobthemyth commented 4 years ago

Instead of just adding an error message assertion, I actually reimplemented the patch to add Ruby to the Metadata source instead of overriding RubyVersion.system. The main reason is that when you asked for a test against the error message, I realized with the original implementation, the error message was pretty cryptic:

Could not find gem 'Ruby (~> x.x.x.x)' in the local ruby installation.
The source contains 'Ruby' at: x.x.x.x

Your Ruby version is x.x.x, but your Gemfile specified x.x.x is a better message, but required a different patch in order for the Bundler code to get that far. I tried to implement the feature using Bundler::Plugin::API::Source but as far as I could tell, Bundler has a hard coded "rule" that Ruby must come from the Metadata source, so I still had to use a patch instead of the Plugin API unfortunately.

jacobthemyth commented 4 years ago

After reviewing my previous commit, I realized that patching Metadata was too tightly coupled to the internals of Bundler. I reimplemented the Ruby dual booting again using Bundler::Plugin::API::Source instead. Even though it still requires patching Bundler::Definition, it seems far less coupled to the internals this way.

Also, overriding Bundler::RubyVersion.system turns out to be required because even though Metadata is used for resolution, system is used for writing the lock file. I've modified the tests to correctly inspect the locked Ruby version.

jacobthemyth commented 4 years ago

Thanks for taking the time to review and give feedback!