search

nodenv / node-build

Install NodeJS versions
MIT License
271 stars 80 forks source link

Don't remove prefix before installing #716

Open pudiva opened 3 years ago

pudiva commented 3 years ago

Back in cda9b11733546c23db2ab70817648dcc7a48cba9, node-build started removing the prefix path before an install to prevent conflicts and hard to debug errors.

However, the operation might inadvertently delete user data (for example if they try to install to /usr/local), and is also incompatible with ruby-build, that does not clean the prefix.

pudiva commented 3 years ago

cc @eregon

eregon commented 3 years ago

FYI there is some discussion on this in https://github.com/rbenv/ruby-build/pull/1783 and https://github.com/rbenv/ruby-build/pull/1784 I think an adaptation of the check from https://github.com/rbenv/ruby-build/pull/1784 would be good (checking bin/node). Installing node into /usr/local is basically making it impossible to uninstall it, and likely to cause troubles (e.g. hard to pick the right headers etc if multiple node versions exist on the system), I believe it's always much better to install in a clean directory.

pudiva commented 3 years ago

Thanks for linking the previous discussions. Indeed I'm also trying to install it on Docker and I'm currently using a hacky sed patch to remove that line ~ which makes everything work just fine. :sparkles:

So ruby-build will only remove the prefix path if installing Truffle Ruby, and only if the directory already contains a previous installation, am I correct? Unfortunately a patch like that wouldn't serve my use case, as I want to install to /usr/local, which is not empty. :slightly_frowning_face:

Btw I have to say that I strongly disagree with that special behaviour for Truffle Ruby. Special behaviours can cause even more confusion and in this case irreversible damage, especially considering how conventional it is to install to /usr/local. So I ask you to pleeeeeease reconsider. :pleading_face::pleading_face::pleading_face:

pudiva commented 3 years ago

Btw²: This cleanup could be done by nodenv instead of node-build for the most conventional use cases.

eregon commented 3 years ago

So ruby-build will only remove the prefix path if installing Truffle Ruby, and only if the directory already contains a previous installation, am I correct?

Yes, or if the directory doesn't exist because we're installing a new directory (then of course removing it does nothing)

So I ask you to pleeeeeease reconsider.

I think you misunderstood, it only removes the directory if it's a previous installation of truffleruby at the same place. For instance this is very important when re-installing a dev version to the same directory (to not keep files from the previous installation around).

In Docker you should really be able to use the system packages or an existing image, or download a node binary, no? Using <x>-build in Docker feels like an anti-pattern.

pudiva commented 3 years ago

I think you misunderstood, it only removes the directory if it's a previous installation of truffleruby at the same place.

Yes, that's exactly what I meant. And if the directory exists and there is no previous installation, it doesn't do anything, right? Which would make the installation to /usr/local impossible.

In Docker you should really be able to use the system packages or an existing image, or download a node binary, no? Using <x>-build in Docker feels like an anti-pattern.

I'd rather use apt-get, but unfortunately it doesn't have the exact node (and ruby) versions I need. Changing my node image is not an option. And regarding installing a binary manually: I could that, but node-build is really more convenient (and by the time it takes to run, looks like it downloads binaries as well).

Would you mind elaborating on why you feel like it's an anti-pattern? As far as I know, the convention of /usr/local exists exactly for sysadmins to install software without conflicting with pkg managers, such as apt-get.

From the FHS:

4.9. /usr/local : Local hierarchy

4.9.1. Purpose

The /usr/local hierarchy is for use by the system administrator when installing software locally. It needs to be safe from being overwritten when the system software is updated. It may be used for programs and data that are shareable amongst a group of hosts, but not found in /usr.

Locally installed software must be placed within /usr/local rather than /usr unless it is being installed to replace or upgrade software in /usr. [28]

There are many benefits to adhering to the standard, like having PATH, MANPATH, PKG_CONFIG_PATH, ... all because distros expect it. Matter of fact, as with most other GNU/Linux software around, chances are that if you ./configure && make install ruby or node today, the default prefix will be /usr/local.

eregon commented 3 years ago

I don't want to take more time for this, obviously we have different opinions. IMHO if a developer installs anything into /usr/local, they are asking for trouble:

So IMHO:

Then I would call that a sane environment because you can opt in or out, reinstall, update, etc cleanly.

I think most of my points apply in Docker too, it doesn't seem good if you have a jungle of mixed files in /usr/local and it's even less convenient to inspect what's going on (due to some error).

In either case, it's not hard to set PATH as needed, or to install to a given prefix.

Also as anecdotal evidence, I found most big software distributed by their authors don't install into /usr/local but some prefix like /opt/google/chrome or /usr/pgsql-10 (for postgres). I assume for making the installation more reliable, to safely update it and avoiding to break other packages needlessly.

So IMHO mixing different softwares in the same prefix without precisely tracking which files belong to what (which a package manager does) is a mistake and an anti-pattern, no matter what FHS docs might say.

Matter of fact, as with most other GNU/Linux software around, chances are that if you ./configure && make install ruby or node today, the default prefix will be /usr/local.

And that will fail because you'd need sudo, and IMHO that's basically a hint for "don't do that, you'll mess your system in unrecoverable ways" (same thing goes for e.g. sudo gem install on macOS).