-
-
Notifications
You must be signed in to change notification settings - Fork 189
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
Use the Ruby directory basename to decide the GEM_HOME #419
Use the Ruby directory basename to decide the GEM_HOME #419
Conversation
e17bd15
to
9dde861
Compare
Tests passing now, I had to adapt one expectation. |
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.
- Two stylistic changes on the
local ruby_name
bit. - This will be a disruptive change, and probably warrants a 1.0.0 release. Additionally, homebrew users will need some kind of warning before updating.
- Probably need a safer way of cleaning up chruby gem homes inside
~/.gem/
without deleting gems installed outside of chruby (aka system ruby).
* Use the installed Ruby directory name for setting `GEM_HOME`. (@eregon) | ||
This guarantees a unique `GEM_HOME` per installed Ruby, even if they have | ||
the same `RUBY_VERSION`. This is important for native extensions, which | ||
might compile differently based on build-time flags such as `--enable-shared`. |
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.
We already confirmed that RubyGems keeps shared vs non-shared extensions in separate directories.
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.
Yes, but that's impractical and I think nobody wants the behavior of "half-installed gems" (i.e., the gem is there but the extension isn't compiled because of different build-time flags, example warning: Ignoring msgpack-1.3.0 because its extensions are not built. Try: gem pristine msgpack --version 1.3.0
).
I'm also quite confident we could find a set of build-time flags that would make extensions incompatible while they would use the same directory.
I can drop the sentence in the ChangeLog though if you think it's confusing.
ChangeLog.md
Outdated
|
||
In practice, updating `chruby` means you will need to install gems again as | ||
the gem directories will be different. It is also recommended to clean old | ||
gem directories which will no longer by used with `rm -rf ~/.gem`. |
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.
~/.gem/
may also contain non-chruby gems that the user might not want deleted.
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.
For gems installed with gem install --user
with the system ruby (chruby system
)?
I'm not sure what we could recommend instead, not cleaning .gem
might lead to a lot of wasted disk space.
README.md
Outdated
@@ -9,7 +9,7 @@ Changes the current Ruby. | |||
* Updates `$PATH`. | |||
* Also adds RubyGems `bin/` directories to `$PATH`. | |||
* Correctly sets `$GEM_HOME` and `$GEM_PATH`. | |||
* Users: gems are installed into `~/.gem/$ruby/$version`. | |||
* Users: gems are installed into `~/.gem/$(basename /path/to/$ruby)`. |
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.
$ruby-$version
might be more understandable? Maybe even $ruby_dir
?
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.
$ruby-$version
would only be correct if the Ruby is installed with ruby-install
or so and without changing the default installation prefix.
I would think $ruby_dir
is an absolute path (same as /path/to/$ruby
).
I'd be fine with $(basename $ruby_dir)
or $(basename /path/to/$ruby_dir)
or $(basename $ruby_prefix)
.
$(basename /path/to/$ruby)
is correct and I think rather clear (/path/to/$ruby
was already used in the README)
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.
Wouldn't $(basename /path/to/$ruby)
just be $ruby
? Also, in your example $ruby
is ambiguous. Is it the Ruby's name, name-version, arbitrary install dir name?
I would mention both /path/to/$ruby-$version
and ~/.gem/$ruby-$version
to make the mapping explicitly clear. Not everyone who will be reading the README/ChangeLog might understand what basename
is yet.
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 mean arbitrary install dir name
, so maybe /path/to/$ruby_prefix
or /path/to/$ruby_install_dir
.
There is nothing enforcing the directories to be named $ruby-$version
so I'd rather avoid that here.
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 tried to clarify by changing to this:
- Correctly sets
$GEM_HOME
and$GEM_PATH
.- Users: gems are installed into
~/.gem/$ruby_name
(e.g.,~/.gem/ruby-2.6.3
for~/.rubies/ruby-2.6.3
). - Root: gems are installed directly into
$ruby_install_dir/$gemdir
(the default).
- Users: gems are installed into
ChangeLog.md
Outdated
* When updating from `chruby` < 0.4 to `chruby` >= 0.4.0, you will need to | ||
re-install your gems because the gem directories will be different. It is also | ||
recommended to clean old gem directories which will no longer be used with | ||
`rm -rf ~/.gem`. |
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.
Maybe a cleaner way of deleting old chruby gem dirs would be something like rm -rf ~/.gem/{ruby,jruby,rbx}/
. Another option might be to mass-rename the directories from ~/.gem/$ruby/$version
to ~/.gem/$ruby-$version
using xargs
.
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.
Maybe a cleaner way of deleting old chruby gem dirs would be something like
rm -rf ~/.gem/{ruby,jruby,rbx}/
.
Would need to be rm -rf ~/.gem/{ruby,jruby,rbx,truffleruby}/
.
Could change to that, but I'm not sure what that would save compared to rm -rf ~/.gem
.
Saving ~/.gem/specs/
doesn't seem really important, it's just a cache AFAIK.
Another option might be to mass-rename the directories from
~/.gem/$ruby/$version
to~/.gem/$ruby-$version
usingxargs
.
That's only correct if Rubies were all installed in $ruby_engine-$ruby_version
format and even then it's actually:
~/.gem/$ruby_engine/$ruby_version
=> ~/.gem/$ruby_engine/$ruby_engine_version
And we probably can't easily guess the $ruby_engine_version
, except for MRI where it's the same.
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.
@eregon system ruby can install into ~/.gem/
as well. For example, on Fedora they configure rubygems to install binstubs into ~/bin/
and other bits into ~/.gem/
by default for regular users:
$ rm -rf ~/.gem/ ~/bin/*
$ gem install bundler
Fetching bundler-2.0.2.gem
Successfully installed bundler-2.0.2
Parsing documentation for bundler-2.0.2
Installing ri documentation for bundler-2.0.2
Done installing documentation for bundler after 2 seconds
1 gem installed
$ ls ~/.gem/
ruby specs
$ ls .gem/ruby
build_info cache doc extensions gems specifications
$ ls ~/bin/
bundle bundler
I do not want to recommend deleting all of ~/.gem/
and raise the ire of users once they realize their non-chruby gems are suddenly gone.
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.
Ah, one issue with selectively rm'ing the chruby generated GEM_HOMEs, ~/.gem/ruby/X.Y.Z
shadows the legitimate ~/.gem/ruby/
directory generated by system RubyGems. Perhaps something like this:
$ ls ~/.gem/ruby
2.6.3 build_info cache doc extensions gems specifications
$ ls -d ~/.gem/{ruby,jruby,rbx,truffleruby}/[0-9].[0-9].[0-9]
/home/postmodern/.gem/ruby/2.6.3
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.
RE mapping ~/.gem/$RUBY_ENGINE/$RUBY_VERSION
to ~/.gem/$RUBY_ENGINE/$release_version
, we could use the same trick that chruby uses to query RUBY_VERSION
and try invoking the absolute #!/path/to/ruby
but with -e 'p RUBY_ENGINE_VERSION'
.
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.
~/.gem/{ruby,jruby,rbx,truffleruby}/[0-9].[0-9].[0-9]
sounds good to me, I'll use that in the upgrade notes.
I'm against documenting about moving gem directories, as that's bound to fail quite badly if anything ever stores an absolute path.
Also, if we're going to change chruby's gem home directory schema, perhaps we should move it out of |
It might be possible to write an ugly migration script to rename chruby's gem dirs based on the |
Added a 1.0.0 milestone and separate branch. |
@postmodern Thank you for the great review, I'll take a look at addressing it soon. |
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.
It might be possible to write an ugly migration script to rename chruby's gem dirs based on the
#!/path/to/$ruby_dir/bin/ruby
of any bin stubs within$gem_dir/bin/
.
I'm concerned that would not work for all gems, because some extensions might capture the absolute path to where they were installed.
I would keep it under |
Thanks, should I target this PR to the 1.0.0 branch then? |
That seems difficult, because Maybe we should just mention to look at directories under |
9dde861
to
7eefe68
Compare
@postmodern I updated the |
Currently the 1.0.0 branch is identical to master. This will be merged into the 1.0.0 branch, so probably no need to rebase it. |
The 0.4.0 branch does have the addition of a manpage. If we want to add the manpage in 1.0.0, it would just need updating to reflect these changes. |
* Such that each installed Ruby has its own GEM_HOME, even if RUBY_VERSION is the same for multiple installed Rubies. * This is particularly important for TruffleRuby, where different releases with the same RUBY_VERSION might compile C extensions differently, and each release should have a different GEM_HOME.
6faed39
to
3038979
Compare
@postmodern I updated the documentation, I think it's both safe and accurate now. I updated the PR to target the 1.0.0 branch. |
@postmodern @havenwood is there anything left to do before merging this? |
Finally merged something 😛 |
@postmodern Awesome, thanks! (I reacted with a 🎉 before but might as well write it in words). Any plan as to when to release 1.0.0, and if other changes should make it for 1.0.0? |
@postmodern What's your branching model? Or should #431 target the 1.0.0 branch? But that needs test fixes from master. |
@eregon once I get ready to release 1.0.0, I merge the 1.0.0 branch into master and hit release. I need to start doing the release engineering work to get 1.0.0 ready (ChangeLog, README, man pages, homebrew recipe, etc). |
@eregon fixes and tests can go into master, and we can always rebase other branches. |
I realized, while this works well for releases, it works less nicely for |
Use the Ruby name to decide the `GEM_HOME` such that each installed Ruby has its own `GEM_HOME`, even if `RUBY_VERSION` is the same for multiple installed Rubies. This is particularly important for TruffleRuby, where different releases with the same `RUBY_VERSION` might compile C extensions differently, and each release should have a different `GEM_HOME`.
@postmodern I forgot about this, but the point above seems to make #431 (that is not setting GEM_HOME) the only solution to reliably have a gem home per Ruby build, also when having local Rubies or head Rubies installed (assuming these Rubies are |
* Use the Ruby name to decide the `GEM_HOME` such that each installed Ruby has its own `GEM_HOME`, even if `RUBY_VERSION` is the same for multiple installed Rubies. * This is particularly important for TruffleRuby, where different releases with the same `RUBY_VERSION` might compile C extensions differently, and each release should have a different `GEM_HOME`.
* Use the Ruby name to decide the `GEM_HOME` such that each installed Ruby has its own `GEM_HOME`, even if `RUBY_VERSION` is the same for multiple installed Rubies. * This is particularly important for TruffleRuby, where different releases with the same `RUBY_VERSION` might compile C extensions differently, and each release should have a different `GEM_HOME`.
* Use the Ruby name to decide the `GEM_HOME` such that each installed Ruby has its own `GEM_HOME`, even if `RUBY_VERSION` is the same for multiple installed Rubies. * This is particularly important for TruffleRuby, where different releases with the same `RUBY_VERSION` might compile C extensions differently, and each release should have a different `GEM_HOME`.
I just realized, this approach is not good enough for any head Ruby, e.g. So a combination of ruby-name + ABI version ( |
If a user is running and updating Not setting |
With ruby/ruby#5474, CRuby would raise LoadError if the ABI doesn't match for dev versions, so that should be helpful and be much clearer than a segfault or so later on. I understand your point of view, I just don't think people are easily aware they need to both With the current chruby release + CRuby head reporting RUBY_VERSION as the next release (e.g., RUBY_VERSION is 3.2.0 on ruby-master currently), dev builds and the 3.2.0 release when it comes out share the same gem home, so that is rather unfortunate, and CRuby releases will probably still not warn on ABI mismatch for releases. IMHO it's hard for RubyGems to detect ABI mismatches (and it doesn't currently), that should be the job of the Ruby implementation and e.g. TruffleRuby always checks the ABI matches. RubyGems could try to better separate per ABI version, but the current design when GEM_HOME is set is RubyGems just uses that path and assumes it's correct whether it is or not. Hence the responsibility is moved to whatever sets GEM_HOME as things stand. |
Linking things together, I think #410 is actually better than this merged PR, because it also works e.g. for |
is the same for multiple installed Rubies.
with the same RUBY_VERSION might compile C extensions differently, and
each release should have a different GEM_HOME.
This is the better version of #410, discussed in that PR starting from #410 (comment)
I believe this is the right fix for always setting
GEM_HOME
correctly to a separate directory per Ruby installation.I added an entry in the changelog along with upgrade notes.
I also updated the README.
Fixes oracle/truffleruby#1723.
@postmodern @havenwood What do you think? Could we merge this?