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 issue when plugin stubs would sometimes not be properly removed by gem uninstall #7631

Conversation

deivid-rodriguez
Copy link
Member

What was the end-user or developer problem that led to this PR?

gem uninstall <gem> --install-dir <dir> will fail to remove plugin stubs if the version being removed is also installed globally.

See for example how, without rubygems-server installed globally, plugin is properly removed from --install-dir:

$ gem list rubygems-server

*** LOCAL GEMS ***


$ gem install rubygems-server --install-dir foo && gem uninstall rubygems-server --install-dir foo && tree foo/plugins
Fetching rubygems-server-0.3.0.gem
Successfully installed rubygems-server-0.3.0
1 gem installed
Successfully uninstalled rubygems-server-0.3.0
foo/plugins

0 directories, 0 files

However, if I have rubygems-server installed in my standard GEM_HOME, gem uninstall will fail to clean it up from --install-dir:

$ gem install rubygems-server --install-dir foo && gem uninstall rubygems-server --install-dir foo && tree foo/plugins
/home/deivid/code/rubygems/rubygems/foo/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:36: warning: already initialized constant Gem::Server::SEARCH
/home/deivid/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:36: warning: previous definition of SEARCH was here
/home/deivid/code/rubygems/rubygems/foo/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:46: warning: already initialized constant Gem::Server::DOC_TEMPLATE
/home/deivid/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:46: warning: previous definition of DOC_TEMPLATE was here
/home/deivid/code/rubygems/rubygems/foo/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:130: warning: already initialized constant Gem::Server::RDOC_CSS
/home/deivid/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:130: warning: previous definition of RDOC_CSS was here
/home/deivid/code/rubygems/rubygems/foo/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:340: warning: already initialized constant Gem::Server::RDOC_NO_DOCUMENTATION
/home/deivid/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:340: warning: previous definition of RDOC_NO_DOCUMENTATION was here
/home/deivid/code/rubygems/rubygems/foo/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:374: warning: already initialized constant Gem::Server::RDOC_SEARCH_TEMPLATE
/home/deivid/.asdf/installs/ruby/3.3.1/lib/ruby/gems/3.3.0/gems/rubygems-server-0.3.0/lib/rubygems/server.rb:374: warning: previous definition of RDOC_SEARCH_TEMPLATE was here
Successfully installed rubygems-server-0.3.0
1 gem installed
Successfully uninstalled rubygems-server-0.3.0
foo/plugins
└── rubygems-server_plugin.rb

1 directory, 1 file

What is your fix for the problem, implemented in this PR?

In order to keep plugins up to date, gem uninstall does the following:

  • First removes the plugin stub for the gem being removed.
  • Then check whether any versions of the gem are still installed, and regenerates the plugin stub again to point to the latest installed version.

The second check, however, was checking globally installed gems instead of gems in --install-dir.

The solution is to make sure that check uses --install-dir rather than the default GEM_HOME.

To implement it, I extracted the functionality that deals with the list of know specifications to a separate SpecificationRecord class that can take a custom directory (--install-dir in this case).

Make sure the following tasks are checked

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/fix-uninstall-with-install-dir-not-removing-plugin-stubs branch from f828536 to c85b6d6 Compare May 6, 2024 17:17
@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review May 6, 2024 17:55
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/fix-uninstall-with-install-dir-not-removing-plugin-stubs branch from a3a5293 to 586edb6 Compare May 6, 2024 17:58
@deivid-rodriguez deivid-rodriguez marked this pull request as draft May 6, 2024 17:58
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/fix-uninstall-with-install-dir-not-removing-plugin-stubs branch 2 times, most recently from 783a3eb to 922aca7 Compare May 6, 2024 19:03
@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review May 7, 2024 13:45
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/fix-uninstall-with-install-dir-not-removing-plugin-stubs branch 2 times, most recently from 6f41b63 to 5f55aa6 Compare May 13, 2024 14:27
If we move test directory removal to the very end, I think we should not
leak anything.
I don't think this method is any worse than others, let's only document
what it does.
Other analog methods are documented, so document this one too.
This class handles all logic to handle the list of specifications, given
a set of GEM_PATH directories. Makes `Gem::Specification` has less
responsibilities and will help with fixing some bugs next.
When `gem uninstall <gem> --install-dir <dir>` is run, if the version
removed had a plugin, and that same version happened to also be
installed globally, then the plugin stub would fail to be removed.
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/fix-uninstall-with-install-dir-not-removing-plugin-stubs branch from 5f55aa6 to 4e2fa0b Compare May 14, 2024 14:56
@deivid-rodriguez deivid-rodriguez merged commit 8a0c7e4 into master May 14, 2024
75 checks passed
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/fix-uninstall-with-install-dir-not-removing-plugin-stubs branch May 14, 2024 16:07
deivid-rodriguez added a commit that referenced this pull request May 16, 2024
…-with-install-dir-not-removing-plugin-stubs

Fix issue when plugin stubs would sometimes not be properly removed by `gem uninstall`

(cherry picked from commit 8a0c7e4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant