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

Fix auto-reshim on bundle install and gem uninstall. #162

Merged
merged 2 commits into from May 1, 2020

Conversation

djmarcin
Copy link
Contributor

This fixes #63 but depends on rubygems/rubygems#3534 being merged in order to actually work. There is a long-standing bug in bundler that prevents it from loading rubygems_plugin.rb from RUBYLIB. However, there is no problem having this hook exist with a version of bundler that doesn't support it -- this file will just never be evaluated in that case.

It also fixes shims not being removed on gem uninstall where installer.bin_dir is not set, so it can never match Gem.default_bindir. Additionally, the #{RUBY_VERSION} bin/#{executable} syntax does not work for removing shims, so in the interest of simplicity, the hook now just runs asdf reshim ruby which works in both the install and uninstall cases, but is potentially less efficient for installation via gem install.

@djmarcin djmarcin force-pushed the master branch 3 times, most recently from d4f59aa to 08a2f79 Compare April 21, 2020 05:08
else
maybe_reshim = lambda do |installer|
# If any gems with executables were installed or uninstalled, reshim.
`asdf reshim ruby` if installer.spec.executables.any?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not specify the RUBY_VERSION here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand why the bin/${executable} part was removed, but the version might still be worth including.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is a bug with asdf itself, but while the #{RUBY_VERSION} does seem to work after gem install, it does not remove the dead shim after gem uninstall.

With version:

$ gem install railties ; echo "Rails: $(which rails)" ; yes | gem uninstall --force railties; echo "Rails: $(which rails)"
Fetching railties-6.0.2.2.gem
Successfully installed railties-6.0.2.2
Parsing documentation for railties-6.0.2.2
Installing ri documentation for railties-6.0.2.2
Done installing documentation for railties after 0 seconds
1 gem installed
Rails: /Users/djmarcin/.asdf/shims/rails
Removing rails
Successfully uninstalled railties-6.0.2.2
Rails: /Users/djmarcin/.asdf/shims/rails

Without version:

$ gem install railties ; echo "Rails: $(which rails)" ; yes | gem uninstall --force railties; echo "Rails: $(which rails)"
Fetching railties-6.0.2.2.gem
Successfully installed railties-6.0.2.2
Parsing documentation for railties-6.0.2.2
Installing ri documentation for railties-6.0.2.2
Done installing documentation for railties after 0 seconds
1 gem installed
Rails: /Users/djmarcin/.asdf/shims/rails
Removing rails
Successfully uninstalled railties-6.0.2.2
Rails:

Although, since installs are likely more frequent than uninstalls, I could change the behavior to iterate over the binaries on install, and just reshim everything on uninstall.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added the customized behavior for install & uninstall.


if installer.spec.executables.any? && installer.bin_dir == Gem.default_bindir
if defined?(Bundler::Installer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is defined, wouldn't we still want the Gem hooks setup too? It doesn't seem like these two things are mutually exclusive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that Bundler::Installer is only defined when the bundle command is running. While Bundler is operating, we wouldn't want the gem hooks to run because we want to batch one reshim operation at the end rather than after each Gem.

@Stratus3D Stratus3D merged commit 8cbfdf4 into asdf-vm:master May 1, 2020
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.

Automatic reshim after gem installing/uninstalling
2 participants