gix-command: pass shell name (not --) as $0 to <shell> -c#2607
gix-command: pass shell name (not --) as $0 to <shell> -c#2607Sarthak Bhardwaj (SarthakB11) wants to merge 4 commits into
--) as $0 to <shell> -c#2607Conversation
Eliah Kagan (EliahKagan)
left a comment
Eliah Kagan (EliahKagan)
left a comment
There was a problem hiding this comment.
Having all shells, even ones that are not sh, receive sh as $0 and announce themselves as sh, is not correct. For that to happen even occasionally would be much worse than what we have now.
But what we have now is also not good, and this PR does identify the parts of the code to change, as well as embodying the insight that it really is possible to fix #1842 in a simple way by just using the best available information about the short name for the shell.
I'll go ahead and push commits to this with the changes needed to turn this into a correct fix. (My commits will also be written using Claude Code; I'll make sure their own trailers reflect this.)
As a separate step, I think that it will be necessary to rebase to change your commit messages, since currently they are written in the style of conventional commits, but using incorrect conventional prefixes. Also, we only use conventional commits when an associated changelog entry is needed (and one of my commits will have a fix: prefix). However, I may wait to do that until it's clear that all CI passes other than recent failures unrelated to changes in this PR, and until after getting a Copilot review.
Eliah Kagan (EliahKagan)
changed the title
sh (not --) as $0 to sh -c--) as $0 to <shell> -c
There was a problem hiding this comment.
Pull request overview
Fixes a long-standing issue where gix-command's shell invocation passed "--" as the command_name operand to sh -c <script>, causing the shell to use -- as $0 and producing confusing error messages like --: command not found. The fix derives the shell's basename and uses it as $0 so diagnostics now read like sh: command not found, with a fallback to _ for degenerate input.
Changes:
- In
gix-command::prepare::Command::from, compute the shell program's basename viaPath::file_name()and pass it (or_if absent) as thecommand_nameafter the-coperand, replacing the literal"--". - Update all assertions in
gix-command/tests/command.rsandgix-credentials/tests/program/from_custom_definition.rsthat observed"--"as$0; add aSH_BASENAMEconstant in each. - Add new tests: a unit test for the
_fallback when the shell path has no basename, and two#[cfg(unix)]spawn tests that actually invoke a shell and verify$0equalssh(default) andbash(viawith_shell_program).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| gix-command/src/lib.rs | Replaces the hardcoded "--" command_name operand with the basename of the shell program, falling back to _. |
| gix-command/tests/command.rs | Updates existing ---placeholder assertions, adds an _-fallback test and two Unix spawn tests verifying actual $0. |
| gix-credentials/tests/program/from_custom_definition.rs | Updates three assertions to expect the shell basename instead of "--" as the command_name operand. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes GitoxideLabs#1842 Co-authored-by: Claude <noreply@anthropic.com>
gix-command now passes "sh" (not "--") as the $0 placeholder to sh -c, so the three trailing-position "--" literals in gix-credentials/tests/program/from_custom_definition.rs needed updating. Co-authored-by: Claude <noreply@anthropic.com>
`gix-command` invokes shells with `sh -c <script> <command_name> [args...]`. The `command_name` operand becomes `$0` inside the shell and is what shells use to prefix their own diagnostic messages. The informative value is the basename of the shell program in use: `sh` for the default shell on Unix (`/bin/sh`), `sh.exe` for the default on Windows, or the basename of whatever was selected via `Prepare::with_shell_program(...)` (e.g. `bash`). Update the construction-test assertions in `gix-command/tests/command.rs` and `gix-credentials/tests/program/from_custom_definition.rs` to use a new `SH_BASENAME` constant alongside `SH`. `SH_BASENAME` is `"sh"` on Unix and `"sh.exe"` on Windows — the basename of the default shell path that `gix_path::env::shell()` documents. The custom-shell test (`single_and_multiple_arguments_as_part_of_command_with_given_shell`) expects `"bash"`, the basename of its `/somepath/to/bash` argument. Add two spawn-based tests under `spawn::with_shell` that run the shell and observe `$0` directly, since construction-level tests only see the argv built for `std::process::Command` and not the runtime value the shell uses. They are `#[cfg(unix)]`: the default-shell case checks `$0 == "sh"`, and the case configuring bash via `with_shell_program(gix_testtools::bash_program())` checks `$0 == "bash"`. Bash is a requirement of the gitoxide test suite, so no skip path is needed. Also add a construction test (`shell_program_with_no_basename_uses_underscore_placeholder`) that exercises the defensive fallback when the shell path has no extractable basename (e.g. empty string, `/`). In that case the `command_name` operand is `_` — the conventional placeholder for an unused `$0` — rather than a fake shell name. The test's name and comment make clear that this is degenerate input not expected in real use. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`gix-command` invokes shells as `sh -c <script> <command_name> [args...]`,
where the `command_name` operand becomes `$0` inside the shell. Shells
use `$0` to prefix their own diagnostic messages (e.g.
`sh: line 1: <cmd>: command not found`), so the value should identify
the actually-running shell.
Derive the value from the shell program itself, using the basename of
the path passed to `Command::new` for the shell:
- With the default shell on Unix (`/bin/sh`), `$0` is `sh`.
- With the default shell on Windows (`sh.exe` from Git for Windows),
`$0` is `sh.exe`, matching the launched binary's actual name.
- With a customized `shell_program`, `$0` reflects the chosen shell
(e.g. `bash`, `zsh`), so its diagnostics correctly identify it.
If the shell path has no extractable basename — `Path::file_name()`
returns `None` for degenerate input like an empty string or `/` — fall
back to `_`, the conventional placeholder for an unused `$0` used in
shell one-liners. This keeps the construction total without claiming a
specific shell name.
Using the basename also avoids an MSYS2-specific surprise on Windows:
Cygwin/MSYS2 shells translate some command-line content received via
`lpCommandLine`, including full Windows paths, so a full shell path
passed as `$0` would come back from the shell as its translated form.
This fixes GitoxideLabs#1842. (See GitoxideLabs#1840 and GitoxideLabs#1842 regarding some of these issues.)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eliah Kagan (EliahKagan)
left a comment
Eliah Kagan (EliahKagan)
left a comment
There was a problem hiding this comment.
CI looks good at the tip before the final rebase (92142e2), in the sense that the only failures match recent failures we've been having everywhere and are very unlikely to relate in any way to any of the changes in this PR.
Besides that last rebase, I think this is in good shape. In the rebase, I'll remove prefixes like gix-command:, which are interpreted as conventional commit prefixes. Based on your PR description, I'll also add trailers to your commits to indicate that Claude was used to write them. Our CONTRIBUTING.md asks for disclosure in individual commits and not only in PR descriptions. Fortunately, this is easy to fix while I'm rebasing anyway.
(GitHub will misleadingly show you, me, and Claude as though we are all coauthors of the first two commits. That's not the case: on those commits, you're author, Claude is coauthor, and the reason it will show me is that rebasing causes me to be the "committer"--but not the author.)
After that, I plan to enable auto-merge. But I don't plan to attempt to override branch protection. This may still not merge until a decision is made on what to do about the recent CI failures.
Eliah Kagan (EliahKagan)
force-pushed
the
fix/issue-1842
branch
from
92142e2 to
3984805
Compare
When
gix-commandbuilds a shell invocation assh -c <script> <args...>, the first trailing arg becomes the shell's$0. The crate was passing--there, which surfaced in error messages as e.g.--: command not foundinstead ofsh: command not found.The fix passes
"sh"as the explicit$0placeholder so$1and onwards still land as the script's positional arguments. All ten assertions ingix-command/tests/command.rsthat asserted the literal--as the$0placeholder are updated; other--strings in those tests are real script args (e.g.ls --fooinside the-coperand) and are unchanged.Disclosure
This PR was drafted with Claude Code (Anthropic). I reviewed and verified the diff before submission.
Closes #1842