gix-transport: run git-upload-pack for local repositories even if it's not in PATH#2700
Conversation
…oxideLabs#2313) Local clones spawn the service program, like git-upload-pack, by name and would fail if it could not be found in PATH. This is common on Windows, where git can be installed such that git itself is available but its subcommands are not, for instance with scoop. Now, when spawning the service program for a local repository fails because it cannot be found, find the same program in the directory that git --exec-path reports - the location git itself uses to run its subcommands - and run it from there. If it cannot be found there either, run the git binary that is always findable with the service as its subcommand, which it can always dispatch. Remote transports are unaffected - they keep the standard invocation so servers can provide their own implementations - and no shell is involved in any of this. The newly added gix_path::env::core_dir_program() provides the lookup and may serve other Git-provided programs in the future.
Git for Windows builds with SKIP_DASHED_BUILT_INS and thus does not provide programs for builtin subcommands like git-upload-pack in its core directory. The test now grounds itself in a listing of that directory instead, and the documentation of core_dir_program() points out the difference - such programs are still run through git itself thanks to the second fallback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf86e75b59
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".
| /// with `SKIP_DASHED_BUILT_INS`, like Git for Windows, omit programs for builtin subcommands, which | ||
| /// can then still be run through `git` itself. | ||
| pub fn core_dir_program(name: &str) -> Option<PathBuf> { | ||
| let path = core_dir()?.join(format!("{name}{}", std::env::consts::EXE_SUFFIX)); |
There was a problem hiding this comment.
Because this helper is public and documented as finding a Git-provided program within core_dir(), callers that pass a user/config-provided name containing .., separators, or an absolute path can get back any existing file outside Git's exec path (Path::join does not confine the result to the base directory). Please validate that name is only a bare program name before joining so the returned path cannot escape the core dir.
Useful? React with 👍 / 👎.
1 participant
Fixes #2313.
Problem
Local clones spawn the service program, like
git-upload-pack, by name, and fail withNotFoundwhen it isn't inPATH— even thoughgititself is findable. As Eliah Kagan (@EliahKagan)'s analysis in the issue lays out, this is common on Windows, where installations like scoop's putgitinPATHbut not its subcommands, and where we already supportgitnot being inPATHat all.Changes
Following the resolution order discussed in the issue:
PATHas before.NotFound, find it wheregitfinds it: the newgix_path::env::core_dir_program(name)looks the program up in the directory reported bygit --exec-path(via the cachedcore_dir()), which is the locationgititself uses to run its subcommands — and run it from there by absolute path. This works on all platforms and is the "conscious choice" variant: it's very clear which program runs.gitrun it: spawnexe_invocation()with the service as its subcommand — passed as a separate argument, no shell involved.The fallback can only ever engage for local repositories: it is gated on the non-SSH arm of the transport, so remote invocations keep the standard
git-upload-packcommand name that servers may customize. The async client has no file transport, so nothing changes there. As the retry is keyed on the failed spawn itself, there is no TOCTOU between checking and running.git-receive-packgets the same treatment as the logic is service-generic.The
auxiliary.rsmachinery for Git-for-Windows directories is deliberately left untouched —--exec-pathalready answers this particular question on every platform, while the broader executable-finding facility discussed in the issue (and #1886) remains open for its own design.Validation
Reproduced the issue and verified the fix end-to-end on macOS with a
PATHcontaining only agitsymlink:gix clone file://…fails withNo such file or directory (os error 2)GIT_EXEC_PATHadditionally pointing at an empty directory, the clone still succeeds via thegit upload-packform, exercising the last fallback.Tests:
gix-path:core_dir_programreturns an existing absolute path forgit-upload-packandNonefor programs that don't exist.gix-transport: the fallback invocation shape is asserted for both branches — never a shell, repository path as final argument, sub-command as a separate argument in thegit-run form.cargo test -p gix-path -p gix-transport --features blocking-client -p gix -p gitoxide-core,cargo clippy --workspace --all-targets -- -D warnings,cargo fmt --all -- --checkall pass.