-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Deprecate --default option from install command
#7588
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
base: master
Are you sure you want to change the base?
Conversation
0f34a74 to
dc83e45
Compare
|
@hsbt I agree with this approach. The |
|
👌 It's ready for review now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now this PR is deprecating the flag, but changing the behavior by making the flag a noop and install the gem normally, correct?
I believe this may be acceptable, because I don't see how the current implementation is helpful. However, we currently document exactly what the flag does, so may be potentially useful for someone, and the more conservative approach would be to first deprecate the flag without changing behavior, and then remove everything.
I think the latter option is preferable, but open to do it differently.
| ) | ||
| installer.install | ||
| File.delete installer.spec_file | ||
| installer.write_default_spec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will install a full copy of bundler in the standard $GEM_HOME. I don't think it's a problem per se, but it makes Bundler inconsistent with other default gems with executables provided with Ruby (for example, irb, which includes a placeholder gem directory with just the executable).
We could try to iterate on this but perhaps for now it's better to not use Gem::Installer at all here and create the files necessary (executables, and gemspec) manually, to keep the exact same folder structure gem update --system creates now.
| "Add the gem's full specification to", | ||
| "specifications/default and extract only its bin") do |v,_o| | ||
| options[:install_as_default] = v | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use deprecate_option here, to also give a runtime deprecation message? The way it is right now, I think it only appears as deprecated in documentation.
|
🤔 It seems it was never finished feature (according to #566). |
dc83e45 to
30f3881
Compare
|
@headius is it ok to remove? Isn't that something JRuby specific? 🤔 |
|
Wow, this goes back a ways. When I originally added default gem support, I had intended As it is now – a partially-implemented feature – it's obviously not very useful. It would have been nice to get it fully working, but even then it's not something that should be user-facing; what we really want is something that can be used by Ruby implementers to install default gems into the standard library. CRuby and JRuby both have their own custom scripts to do this. TruffleRuby just copies out of the CRuby repository. None of these approaches are great. I think it's fine to remove |
This may not entirely be true; there are probably users that would like to alter or update the "default" gems in their Ruby installs, and currently there's no way to do that without manually manipuating those gems' files. But I'm not sure what that functionality should look like. |
aab2062 to
72b0e36
Compare
72b0e36 to
9e8100a
Compare
9e8100a to
6004b8c
Compare
6004b8c to
be47f6d
Compare
What was the end-user or developer problem that led to this PR?
The current
--defaultoption is confused for RubyGems users.This option is special behavior for default gems. But I'm not sure why it only generate executable, not copy library files and other processes.
What is your fix for the problem, implemented in this PR?
It leads complicated environment and there is no merit for them. I don't allow that users install gem as default gems from core maintainer's view.
Finally, I'll show deprecated message and removed code about this option from rubygems.
Closes #7005.
Make sure the following tasks are checked