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
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 15 additions & 8 deletions rubygems-plugin/rubygems_plugin.rb
@@ -1,11 +1,18 @@
# Yes borrowed from rbenv. Couldn't take my mind off that implementation

Gem.post_install do |installer|

if installer.spec.executables.any? && installer.bin_dir == Gem.default_bindir
installer.spec.executables.each do |executable|
`asdf reshim ruby #{RUBY_VERSION} bin/#{executable}`
end
module ReshimInstaller
def install(options)
super
# We don't know which gems were installed, so always reshim.
`asdf reshim ruby`
end
end

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.

Bundler::Installer.prepend ReshimInstaller
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.

end
Gem.post_install &maybe_reshim
Gem.post_uninstall &maybe_reshim
end