-
Notifications
You must be signed in to change notification settings - Fork 80
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
Don't remove prefix before installing #716
base: main
Are you sure you want to change the base?
Conversation
cc @eregon |
Back in cda9b11, 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.
FYI there is some discussion on this in rbenv/ruby-build#1783 and rbenv/ruby-build#1784 |
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. ✨ 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. 🙁 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. 🥺🥺🥺 |
Btw²: This cleanup could be done by nodenv instead of node-build for the most conventional use cases. |
Yes, or if the directory doesn't exist because we're installing a new directory (then of course removing it does nothing)
I think you misunderstood, it only removes the directory if it's a previous installation of truffleruby at the same place. In Docker you should really be able to use the system packages or an existing image, or download a node binary, no? Using |
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
I'd rather use Would you mind elaborating on why you feel like it's an anti-pattern? As far as I know, the convention of From the FHS:
There are many benefits to adhering to the standard, like having |
I don't want to take more time for this, obviously we have different opinions.
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 Also as anecdotal evidence, I found most big software distributed by their authors don't install into 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.
And that will fail because you'd need |
Back in cda9b11, 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.