Fix for 'forge zsh setup' command when running in MSYS2 zsh#2778
Closed
Alexx999 wants to merge 2 commits intoantinomyhq:mainfrom
Closed
Fix for 'forge zsh setup' command when running in MSYS2 zsh#2778Alexx999 wants to merge 2 commits intoantinomyhq:mainfrom
Alexx999 wants to merge 2 commits intoantinomyhq:mainfrom
Conversation
…xe in MSYS2 zsh environment forge.exe had CRLF line endings in .zsh files, and also running scripts via 'zsh -c' was failing for some weird reasons.
github-actions
bot
added
the
type: fix
|
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| let mut child = std::process::Command::new("zsh") | ||
| .arg("-c") | ||
| .arg(script_content) | ||
| .arg(script_path.to_str().unwrap()) |
Contributor
There was a problem hiding this comment.
Potential panic on non-UTF8 paths: Calling unwrap() on to_str() will panic if the temp path contains non-UTF8 characters. While rare with system temp directories, this could crash the application in certain environments.
Fix by handling the error:
.arg(script_path.to_str()
.ok_or_else(|| anyhow::anyhow!("Temp path contains invalid UTF-8"))?)Or use as_os_str() if zsh command accepts OsStr (though Command::arg() already accepts AsRef):
.arg(&script_path)
Suggested change
| .arg(script_path.to_str().unwrap()) | |
| .arg(&script_path) |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
Collaborator
|
Thanks @Alexx999! |
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
3 participants
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.
Before:
forge.exe had CRLF line endings in embedded .zsh scripts, and also running scripts via 'zsh -c' was failing for some weird reasons.
Zsh does not support CRLF line endings failing with errors like
parse error near `in^M''Git for Windows' defaults to autocrlf=true which is one possible cause for CRLFs in there, so suppress it via .gitattributes
Also, explicitly normalize line endings when embedding the strings so that any setup would work the same way.
After fixing that, the error shifted to
zsh:251: parse error near `f'- something about long/complexzsh -ccommands is broken in MSYS2 env, running from a file works perfectly though: setup completed and: hiworks with these changes