Make --default and --install-dir options to gem install play nice together
#3906
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`Gem.use_paths` already does this.
Currently tests use two different gem layouts at the same time. For example: * `tmp/test_rubygems_25698/default/`, where default gem executables are installed. * `tmp/test_rubygems_25698/gemhome/`, where other gems are installed. This is quite unintuitive, and does not match realworld rubygems setups, where these locations are usually the same. This commit changes the setup so that this location is shared. Two tweaks are needed to get tests passing because there's a couple of places in the code where `rubygems` does some stuff differently if `Gem.default_dir` != `Gem.dir`. In particular, * `bindir` is set to `File.join(Gem.dir, "bin")` instead of `RbConfig::CONFIG["bindir"]`. * Checking for executable overwrites is skipped. Since after make this change these locations are now the same, we need to adapt to these behavioral differences.
The previous `Gem.ensure_gem_subdirectories` ensures this.
`install_default_specs` already installs default specs.
With this combination of flags, `rubygems` should install the default specification to `--install-dir`, not to the default rubygems installation. I doubt this has any realworld use cases, but fixing it is handy for me in order to be able to write a test for a bug fix in `bundler` related to `bundle clean --force` removing executables of default gems.
4 tasks
|
Basically it seems good. I will check the behavior changes of the default gems installation. |
|
@hsbt Did you have time to look at this? I don't think it should affect any realworld behavior since it's a very niche combination of flags. In any case, if it would affect I believe it's too fix a bug so it's fine. But great to double check of course 👍. |
|
Sorry, I'm working on RubyKaigi takeout 2020 as stuff in this week. I'll review this after that 🙇 |
|
Sure, thanks! And keep up the good work 💪. |
|
@hsbt friendly ping :) |
hsbt
approved these changes
Sep 15, 2020
|
Thanks so much @hsbt! |
deivid-rodriguez
deleted the
fix_install_as_default_with_install_dir
branch
deivid-rodriguez
added a commit
that referenced
this pull request
Dec 7, 2020
…stall_dir Make `--default` and `--install-dir` options to `gem install` play nice together (cherry picked from commit 40fc322)
deivid-rodriguez
added a commit
that referenced
this pull request
Dec 7, 2020
…stall_dir Make `--default` and `--install-dir` options to `gem install` play nice together (cherry picked from commit 40fc322)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
These options are probably not really used in real life together, but I happened to need them while writing a spec for a bundler issue fix. In particular, for #3757.
What was the end-user or developer problem that led to this PR?d
The problem is that when
--defaultand--install-diroptions are given togem install, the default specification file should be installed to thespecifications/defaultfolder ininstall-dir, not to the one in the default gem directory.What is your fix for the problem, implemented in this PR?
The fix seemed really simple, but due to the cumbersome setup of rubygems tests, it broke a lot of tests, so I had to refactor rubygems test setup to play nice with the change. More information is given in the individual commits.
Tasks:
I will abide by the code of conduct.