-
Notifications
You must be signed in to change notification settings - Fork 204
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
fix: improve install.sh PATH handling and general robustness #2189
Conversation
The ${var_name/#pattern/replacement} Example: s="something"
echo ${s/#s*e/other} produces "otherthing". |
I have no way to check if I tried searching for "fish_add_path spaces" to see if they require spaces, but all I found were endless unrelated discussions. We might need this: LINE="fish_add_path \"${BIN_DIR}\"" As for LINE="set path = ( \"${BIN_DIR}\" \"\$path\" )" But there's lack of documentation for those weird shells. It seems like TCSH might need https://superuser.com/questions/52423/space-in-directory-path-in-path-variable-in-linux At least Bash and ZSH, the normal shells, are working now. |
Example to demonstrate that it works in Bash and ZSH now: $ cat install.sh | env PIXI_HOME=~/.local/share/"pixi with spaces" bash $ cat ~/.zshrc
export PATH="/home/johnny/.local/share/pixi with spaces/bin:$PATH" $ which pixi
~/.local/share/pixi with spaces/bin/pixi |
Before these fixes, the shell would always break when installing Pixi. That happened regardless of whether Pixi itself contained spaces in its own $ cat ~/.zshrc
export PATH=/home/johnny/.local/share/pixi:$PATH Trying to start a new shell after PATH has been broken loses a lot of core utilities and breaks completely: zsh: wc: command not found...
Packages providing this file are:
'coreutils'
'coreutils-single'
$ ls
zsh: sort: command not found...
Packages providing this file are:
'coreutils'
'coreutils-single'
zsh: ls: command not found...
Similar commands are::
'lc'
'lz'
zsh: wc: command not found...
Packages providing this file are:
'coreutils'
'coreutils-single' This bug happened because shells drop everything after the first space unless quoted. So the old Pixi code basically drops the entire $ export FOO=bar yeah
$ echo "$FOO"
bar Edit: I will force push a new commit message to mention another bug fixed by this PR. |
- Exported paths for Bash and ZSH are now surrounded by double quotes, which is necessary to avoid breaking user systems where the existing `PATH` contains spaces. - The `PIXI_HOME` environment variable now resolves incoming `~/` prefixes to the actual user's `$HOME` value. Otherwise we would generate invalid paths, since `~/` is not a valid prefix in `PATH`. It also fixes the fact that the old code installed into a literal sub-directory of the current working dir, named `~/.pixi` (a literal `~`), instead of into the user's home directory. - We now use double quotes around all variables. This is required for two reasons. It ensures that arguments with spaces are supported. And it prevents Bash from breaking when a variable contains an empty string. Otherwise, various tests such as `[[ $HTTP_CODE -lt 200 ]]` would break with errors about not providing enough parameters. - The `PIXI_NO_PATH_UPDATE` syntax has been clarified to use `:-` to clearly show that it's using an empty default. The old `-` syntax looks more like a typo.
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.
Thank you! Also great explanation, I learned something today!
@ruben-arts @wolfv Happy to help. :) I just noticed that |
Exported paths for Bash and ZSH are now surrounded by double quotes, which is necessary to avoid breaking user systems where the existing
PATH
contains spaces.The
PIXI_HOME
environment variable now resolves incoming~/
prefixes to the actual user's$HOME
value. Otherwise we would generate invalid paths, since~/
is not a valid prefix inPATH
. It also fixes the fact that the old code installed into a literal sub-directory of the current working dir, named~/.pixi
(a literal~
), instead of into the user's home directory, whenever users tried to provide a customenv PIXI_HOME=~/.pixi
variable themselves.We now use double quotes around all variables. This is required for two reasons. It ensures that arguments with spaces are supported. And it prevents Bash from breaking when a variable contains an empty string. Otherwise, various tests such as
[[ $HTTP_CODE -lt 200 ]]
would break with errors about not providing enough parameters.The
PIXI_NO_PATH_UPDATE
syntax has been clarified to use:-
to clearly show that it's using an empty default. The old-
syntax looks more like a typo.Closes #2188