Repository navigation
Fix $0 for scripts resolved through PATH - #4
theduardomaciel wants to merge 1 commit into
Conversation
|
Hello! Happy to review and potentially merge this. But first, could you please attribute your git commits to which model (if any) were used. |
Bash sets $0 to the full path when a command is found via PATH, and keeps the word as typed when it contains a separator. Generated launchers such as pnpm's bin/pnpm use $0 to locate their target, so a bare $0 makes them fail with a relative path. Use the resolved POSIX path for PATH lookups, and pass the POSIX form when running a script through an external child shell. Co-authored-by: DeepSeek V4.1 Flash <noreply@opencode.ai>
3851f1e to
27ccd05
Compare
|
Just did it! I've used DeepSeek V4.1 Flash to test Gash locally, since I've been having some issues with Git Bash recently. I ran into a few issues along the way, particularly when using tools like Anyway, congratulations on the project, and keep up the great work! oh, one more note: I've updated the commits to include the model name. However, I was using the OpenCode harness, so if you'd prefer to have it listed as the co-author, just let me know! |
There was a problem hiding this comment.
Thanks for this. The PATH-lookup fix is correct and matches bash. Line comments below.
Missing regression test: no test covers this change, and no CI ran on the branch. A tests/scripts case would keep it from breaking again. For example: create a temp dir holding a small script, prepend that dir to PATH, run the script by bare name, and print case $0 in */*) echo dir;; *) echo bare;; esac. Then run it again as ./x and check that the typed form is kept. That output is the same on any machine, so it can be diffed against Git Bash.
| ExeKind::Script => { | ||
| cmd = Proc::new(&crate::global().exe); | ||
| cmd.arg(res.path.as_os_str()); | ||
| cmd.arg(crate::path::to_posix(&res.path)); |
There was a problem hiding this comment.
$0 depends on in-process vs spawned execution. The "keep the word as typed if it contains a separator" rule only exists in run_script_inproc. This branch always passes the resolved path. It runs for background jobs (exec_background sets spawn_only) and when GASH_NO_INPROC is set. As a result:
./foo.shgives$0=./foo.sh./foo.sh &gives$0=/c/Users/.../foo.sh
Bash gives ./foo.sh in both cases. Suggestion: compute arg0 once in a small helper used by both paths, and pass that word to the child gash here. The child runs in the same cwd, so a relative ./foo.sh still resolves.
| ExeKind::Script => { | ||
| cmd = Proc::new(&crate::global().exe); | ||
| cmd.arg(res.path.as_os_str()); | ||
| cmd.arg(crate::path::to_posix(&res.path)); |
There was a problem hiding this comment.
Lossy conversion (minor). The old code passed res.path to the child as an OsStr. to_posix goes through to_string_lossy, so a path that isn't valid UTF-16 (an unpaired surrogate is legal on NTFS) reaches the child gash with U+FFFD in it. The child then fails with "No such file or directory" (exit 127). Rare, but it's a regression from passing the path through unchanged.
| let arg0 = if has_path_sep(&argv[0]) { | ||
| argv[0].clone() | ||
| } else { | ||
| crate::path::to_posix(&res.path) |
There was a problem hiding this comment.
Verbatim UNC paths come out relative. to_posix calls strip_verbatim, which turns \\?\UNC\server\share\x into server\share\x. Its comment says the caller adds back the leading \\, but to_posix never does, so the result is server/share/x, a relative path. That bug was already in to_posix, but this PR now sends its output into $0 and (at line 1513) into the script path given to the child, which then exits 127. Probably rare, but worth fixing in to_posix while you're here.
| // Bash sets $0 to the resolved path when the command came from a PATH | ||
| // lookup, and keeps the word as typed when it contains a separator. | ||
| // Generated launchers (pnpm, corepack, ...) rely on $0 having a directory. AACA | ||
| let arg0 = if has_path_sep(&argv[0]) { |
There was a problem hiding this comment.
Relative or empty PATH entries (nit). find_in_path turns relative or empty PATH entries into absolute paths, so $0 here ends up absolute where bash keeps the form from PATH:
PATH=bin:$PATH: bash givesbin/hello, gash gives/c/proj/bin/hello- empty entry or
.: bash gives./hello, gash gives/c/proj/./hello
Launchers still work, so this is low priority. It does go against the aim of matching bash, though.
Problem
When gash runs a shell script found through
PATH, it sets$0to the bare command name instead of the resolved path:This breaks generated launchers that locate files relative to
$0. Concretely, pnpm'sbin/pnpmdoes:so
pnpm <cmd>fails with:Fix
Match bash semantics:
PATH->$0is the resolved pathTesting
pnpm --versionworks with pnpm's stock, unmodified shim