fix: Quote values in bash activation with shlex#1392
Merged
baszalmstra merged 1 commit intoconda:mainfrom Jun 19, 2025
Merged
Conversation
e9b3d7c to
0c2cff3
Compare
Collaborator
|
thanks for contributing a fix! |
6f09b2b to
5319a95
Compare
Contributor
Author
|
I succeeded building this with pixi as a tweaked version of this patch rebased on an older rattler commit. I found everything but one thing was working - the PATH variable was set with name "Path" instead, and so the replacement PATH did not take. I made code changes I think should have worked, but it still printed the incorrect Path. I'll pick this up again later to try fixing it. |
Collaborator
|
Thank you, I appreciate you taking the time! |
On Windows, activating an environment with Git Bash fails, because paths
are being set with double-quote strings: export A="C:\Windows\Path"
By using shlex::try_quote, these variables can be set correctly.
Because setting the PATH used the primitive for setting an environment
variable, that had to change to explicitly use double-quotes, so that
the included ${PATH} is evaluated instead of handled literally.
Extended the tests for ShellScript<Bash> to include '\', an unexpanded
${} variable, all the modes of setting the PATH, and running a script
with a character that needs escaping.
Contributor
Author
|
Got it to work, and I think this is ready to merge now! Please double check the various unwrap/error handling code, I tried to follow the patterns I saw, but would be great to have someone make sure it's following the right patterns. |
baszalmstra
approved these changes
Jun 19, 2025
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 20, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 20, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 20, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 20, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 20, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
This was referenced Jun 20, 2025
Closed
Closed
Closed
Closed
Closed
Closed
Closed
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 22, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
Closed
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 23, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 25, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 26, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jun 29, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jul 9, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
fecet
pushed a commit
to fecet/rattler
that referenced
this pull request
Jul 23, 2025
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Jul 25, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Aug 10, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
mwiebe
added a commit
to mwiebe/pixi
that referenced
this pull request
Aug 16, 2025
This depends on the PR conda/rattler#1392 for the bash shell hook to work correctly on Windows. Create a function start_winbash for running bash on Windows that launches bash using the command: $ bash --init-file <temp-shell-hook> -i
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
On Windows, activating an environment with Git Bash fails. I experienced this issue trying to use the
pixi shell-hookcommand that produces output like:This is incorrect, because of using the ";" instead of ":" separator in the PATH and not escaping the "\" characters in the other paths that appear.
By using shlex::try_quote, these variables can be set correctly. Because setting the PATH used the primitive for setting an environment variable, that had to change to explicitly use double-quotes, so that the included ${PATH} is evaluated instead of handled literally.
Extended the tests for ShellScript to include '\', an unexpanded ${} variable, all the modes of setting the PATH, and running a script with a character that needs escaping.
After cherry-picking this change onto an older rattler commit that works with pixi, here's example output from pixi with correct behavior: