ruby-build icon indicating copy to clipboard operation
ruby-build copied to clipboard

The prefix directory is not cleaned when reinstalling jruby-dev

Open eregon opened this issue 4 years ago • 1 comments

I had a old jruby-dev on my system, installed by ruby-build jruby-dev ~/.rubies/jruby-dev. Then I reinstalled the latest jruby-dev with the same command.

The result is there is a mix of old and new files, and so e.g. of old and new gems:

$ gem list

*** LOCAL GEMS ***

bundler (default: 2.2.29, default: 2.2.14)
cmath (default: 1.0.0)
csv (default: 3.1.2)
did_you_mean (1.3.0, 1.2.1)
e2mmap (default: 0.1.0)
ffi (default: 1.15.4 java, default: 1.15.1 java)
fileutils (default: 1.4.1)
forwardable (default: 1.2.0)
io-console (default: 0.5.9 java)
ipaddr (default: 1.2.2)
irb (default: 1.0.0)
jar-dependencies (default: 0.4.1)
jruby-launcher (1.1.19 java, 1.1.17 java)
jruby-openssl (default: 0.10.7 java, default: 0.10.5 java)
jruby-readline (default: 1.3.7 java)
json (default: 2.5.1 java)
logger (default: 1.3.0)
matrix (default: 0.3.0)
minitest (5.11.3)
mutex_m (default: 0.1.0)
net-telnet (0.1.1)
ostruct (default: 0.3.3, default: 0.1.0)
power_assert (1.1.3)
prime (default: 0.1.0)
psych (default: 3.3.2 java)
racc (default: 1.5.2 java)
rake (12.3.3)
rake-ant (default: 1.0.4)
rdoc (default: 6.1.2.1, default: 6.1.2)
rexml (default: 3.1.9.1, default: 3.1.9)
rss (default: 0.2.7)
rubygems-update (default: 3.2.29, default: 3.2.14)
scanf (default: 1.0.0)
shell (default: 0.7)
sync (default: 0.5.0)
test-unit (3.2.9)
thwait (default: 0.1.0)
tracer (default: 0.1.0)
webrick (default: 1.7.0)
xmlrpc (0.3.0)

(for comparison, here is the gem list after manual rm -rf ~/.rubies/jruby-dev and reinstalling with the same command)

I think it would be best to clear the destination directory if we detect it's a JRuby installation, similar to what's done for TruffleRuby: https://github.com/rbenv/ruby-build/blob/766e9c781f6898bd872a808e14c4afab39b16095/bin/ruby-build#L796-L807

@headius @enebo What do you think should happen in that case?


This used to be done for all build_package_copy Rubies no matter what was in the destination directory, but that's problematic for installing to /usr/local: https://github.com/rbenv/ruby-build/pull/1783

eregon avatar Nov 07 '21 14:11 eregon

@headius @enebo Ping, could you share your opinion on the above?

eregon avatar Jul 11 '22 10:07 eregon

@eregon Could we offload to the user the responsibility of removing the old installation files?

I'm not 100% sold on that JRuby and Truffleruby installation methods should do something "magic" and different from Ruby intallations.

mislav avatar Nov 16 '22 17:11 mislav

In general we tell people never to share or reuse a gem directory across installs, which I think would apply in this case. We have never supported installing a new JRuby by overlaying a previous installation and would not expect it to work, even if it might by accident.

headius avatar Nov 16 '22 17:11 headius

I'm not 100% sold on that JRuby and Truffleruby installation methods should do something "magic" and different from Ruby intallations.

I would say it's also a problem for CRuby (or any Ruby) to reuse an existing prefix, especially if it mixes gems like that and e.g. there are 2 "default" gem versions. If it's not gems it could be stdlib files which are still there even though they should be removed, etc.

The current logic is https://github.com/rbenv/ruby-build/blob/66311de0f8089564a232722a6b3db824b85235b3/bin/ruby-build#L806-L819. The only part really TruffleRuby-specific there is "detecting if it's a TruffleRuby prefix" but that could fairly easily be more portable if e.g. we pass the *ruby executable name to that function. Maybe we should improve it to account for package managers installing /usr/bin/*ruby or /usr/local/bin/*ruby, where we still wouldn't want to rm -rf or override the files (e.g., we could have a list of such prefixes, there are not that many).

I think another possibility would be to just error out if trying to install to a non-empty directory, and then let the user remove that directory, or pass a flag to force installation to a non-empty directory, where they won't be able to cleanup properly after, if that's really what the user wants (e.g., /usr/local). That would be less convenient for reinstalling dev versions though.

We could also just warn, but that could be easily missed and addressing the warning would mean the user cancelling the current build and retrying which seems messy.

eregon avatar Nov 16 '22 19:11 eregon

This is a bigger issue for *-dev than releases, so another option would be to only rm -rf or error out for dev builds. It could be an issue for a release too though if e.g. some gem has been messed up (e.g. half compiled in a broken state with a bad Makefile, or miscompiled by a bad XCode and segfaulting) it won't be cleaned up by reinstalling that release (assuming no GEM_HOME set, as with rbenv).

eregon avatar Nov 16 '22 19:11 eregon

I dig the idea of scoping the fix to just *-dev builds. For non-dev build definitions, ruby-build should just respect the prefix given and install there regardless of what is currently present there. The user should be responsible for preparing the installation location in the way they see fit.

For *-dev builds we could error out if a bin/ruby already exists in the prefix directory, indicating the presence of a previous Ruby installation. Then the user could rm -rf the prefix directory themselves if they wanted to work around that.

Note that this would constitute a backwards-incompatible change for scripts.

mislav avatar Nov 16 '22 20:11 mislav

As per my previous comment, I was open for a PR that changes behavior for *-dev builds, but since there was no chatter nor upvotes around this in the years since, I'm inclined to close this to clean out the backlog.

People building *-dev versions of Ruby are typically power users and I'm fine with the responsibility of a preemptive rm -rf being on their side.

mislav avatar Oct 11 '23 13:10 mislav

I've had cases where e.g. the truffleruby installation was broken while working on the definition, and then not cleaning would result in pretty confusing behavior, and this could potentially happen to users of ruby-build too. So I think it's only for *-dev even though that's even more necessary. I'm OK with things as they are (do this only for truffleruby), so yeah fine to close.

eregon avatar Oct 11 '23 14:10 eregon