| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@EliahKagan I think this take on bash.exe handling (inspired by gix) is better, and I hope you will come to the same conclusion. |
Sorry, something went wrong.
There was a problem hiding this comment.
Here are some automated review suggestions for this pull request.
Reviewed commit: 701259a3e4
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
There was a problem hiding this comment.
This pull request refines GitPython’s Windows commit-hook execution path to reliably select the intended Git-for-Windows Bash interpreter (instead of the WSL System32\bash.exe launcher) by preferring a Bash that matches GitPython’s selected git.exe installation, and by hardening the PATH-based fallback.
Changes:
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| git/index/fun.py | Adds Git-for-Windows-root/Bash discovery and a safer PATH resolver; updates run_commit_hook to prefer Git-associated Bash before falling back. |
| test/test_index.py | Adds/adjusts tests to cover the new resolver behavior under realistic Windows PATH ordering and configuration scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
<!-- agent --> The resolver merged in #2199 only worked when Git for Windows Bash itself preceded System32 on PATH. That is typical in Git Bash and CI but not in a normal system-wide installation, where PATH commonly contains System32 followed by Git\cmd. It also skipped an explicitly listed current directory even though explicit PATH entries are trusted configuration, and its test mocked the resolver rather than exercising its precedence. The machine producing this commit provides Ubuntu under WSL 2, C:\Windows\System32\bash.exe, and Git for Windows. Direct CreateProcess-style invocation of bare bash.exe reported Linux 6.18.33.2-microsoft-standard-WSL2, while `git var GIT_SHELL_PATH` reported C:/Program Files/Git/usr/bin/sh.exe and that shell reported MINGW64. With PATH reordered to System32 followed by Git\usr\bin, the merged resolver selected the System32 WSL launcher and an actual GitPython hook wrote a Linux marker. With the more typical System32 followed by Git\cmd PATH, Bash was not present on PATH at all. Locate the Bash associated with GitPython's selected Git executable before general PATH lookup. Recognize the standard Git for Windows layouts Git\cmd\git.exe, Git\bin\git.exe, and Git\<platform>\bin\git.exe, with the platform names used by MSYS2. Configured relative Git executable paths are resolved against the parent process working directory, matching measured CreateProcess behavior even when Popen supplies a different hook working directory. Root-level bin is accepted for a selected Git executable, while usr\bin is deliberately not used to infer an unbounded parent layout. From the trusted Git root, follow gix-path's precedence of bin/bash.exe before usr/bin/bash.exe. If the Git layout is unrecognized, search explicit PATH entries while excluding candidates below SystemRoot so the WSL launcher cannot win. Empty PATH entries are ignored according to Windows semantics, but an explicitly named directory remains eligible even when it is the current directory. Finally, retain the prior bare fallback for nonstandard installations; safer_popen sets NoDefaultCurrentDirectoryInExePath, so that fallback does not reintroduce current-directory lookup. The main regression models System32 before Git\cmd with Bash absent from PATH and checks the real resolver selects the associated Git\bin\bash.exe. Additional tests distinguish an explicit current-directory entry from an empty entry and cover an explicitly configured Git\bin\git.exe. On this machine, an end-to-end hook run under exactly System32;Git\cmd selected C:\Program Files\Git\bin\bash.exe and wrote a MINGW64 marker instead of the earlier Linux/WSL marker. Validated with the focused hook suite (6 passed), the complete test_index.py module (32 passed, one expected xfail, one existing xpass), repository-wide Ruff lint and format checks targeting Python 3.7, and git diff --check. A standalone Python 3.7 interpreter was not available for an additional py_compile run. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
| Back | FazBrowse Home | New Git URL |
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by Codex GPT-5.
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Summary
Follow-up to #2199 and its post-merge review of the Windows hook Bash resolver. Refs #2198.
The merged resolver still selected the WSL launcher when System32 preceded Git Bash on PATH, and it could not locate Git Bash in the common configuration where PATH contains only Git\cmd. It also skipped an explicitly configured current-directory PATH entry and mocked the resolver in its regression test.
This change first locates the Bash associated with GitPython's selected Git executable. It recognizes standard Git\cmd\git.exe, Git\bin\git.exe, and Git\<platform>\bin\git.exe layouts, then follows gix-path's bin/bash.exe before usr/bin/bash.exe precedence. The PATH fallback excludes candidates below SystemRoot, ignores empty entries according to Windows semantics, and still honors explicitly named directories. The existing bare fallback is retained for nonstandard installations and remains protected from implicit current-directory lookup by safer_popen.
Windows and WSL validation
This machine has Ubuntu on WSL 2, C:\Windows\System32\bash.exe, and Git for Windows.
Tests