GitPython
GitPython copied to clipboard
Override bash executable, defaulting to Git for Windows git bash over WSL bash
Provides a mechanism to set a path to bash to use on Windows for executing commit hook commands. If none is set, the default behavior should be to prefer Git Bash from Git for Windows.
If a user is running in Windows, they probably are using Git for Windows, which includes Git Bash and should be associated with the configured Git command set in refresh()
. Search first for Git Bash relative to the configured Git command, and, if it exists, use it. Otherwise, fail back to the hardcoded default of "bash.exe"
.
The original "default" bash without a path assumes that bash.exe is on the PATH. If a user chose not to make MSYS2 tools available on the PATH on Git for Windows install, another method must be used to find that bash if it should be preferred over one found on the PATH. Newer versions of Windows include a bash.exe in System32 that is then found by default, but this bash.exe is intended for use with WSL. If a user intended to invoke git in WSL, presumably this package would have been installed there and not registered as "Windows" and reduces the ambiguity. In most cases where running the python in "native" Windows, then, it is undesirable to invoke the WSL Bash that is not associated with the Git for Windows install. Furthermore, direct access to bash.exe in System32 is reportedly deprecated in lieu of commands that explicitly invoke WSL.
This addresses issues where git hooks are intended to run assuming the "native" Windows environment as seen by git.exe rather than inside the git sandbox of WSL, which is likely configured independently of the Windows Git as well. A noteworthy example are repos with Git LFS, where Git LFS may be installed in Windows but not in WSL, causing commit hooks for LFS to fail despite being installed alongside the configured Git command in this package.
@EliahKagan already prepared me for this case and I hope he can take a look at this PR as well [...]
I will try to look at this in detail today or tomorrow.
Thanks for all the comments! Been busy with the day job the last week or so, but planning to try to address these things this weekend so we can get another round of reviews going and get it merged.
I believe I have addressed all the comments. I removed the xfails from the unit tests, and had assertion errors in test_hook_uses_shell_not_from_cwd()
due to the hook actually executing in type(_win_bash_status) in {WinBashStatus.Absent, WinBashStatus.WslNoDistro}
block rather than raising the exception. I'm admittedly not understanding what this test is trying to test, but it seems like the else
block is the behavior we now want since we should generally have working hooks regardless of WSL status. I opted to just comment out the former block and make every case follow the former else branch to get the test to pass. If that is not the correct result for this test, though, please let me know/enlighten me on what this test is trying to do so I can fix the issue. Otherwise, with your concurrence, this PR should hopefully be ready :-)
The new conflict looks to be from #1819 and easily resolvable. #1819 bumped the version on the setup-wsl
action that this PR comments out.