diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 08ff4efdf..b7aa150f9 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -35,11 +35,11 @@ jobs: python-version: ${{ matrix.python-version }} allow-prereleases: ${{ matrix.experimental }} - - name: Set up WSL (Windows) - if: startsWith(matrix.os, 'windows') - uses: Vampire/setup-wsl@v2.0.2 - with: - distribution: Debian + # - name: Set up WSL (Windows) + # if: startsWith(matrix.os, 'windows') + # uses: Vampire/setup-wsl@v2.0.2 + # with: + # distribution: Debian - name: Prepare this repo for tests run: | diff --git a/AUTHORS b/AUTHORS index 9311b3962..862c6579f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -54,5 +54,6 @@ Contributors are: -Wenhan Zhu <wzhu.cosmos _at_ gmail.com> -Eliah Kagan <eliah.kagan _at_ gmail.com> -Ethan Lin <et.repositories _at_ gmail.com> +-Randy Eckman <emanspeaks _at_ gmail.com> Portions derived from other open source works and are clearly marked. diff --git a/git/__init__.py b/git/__init__.py index c6a52ef30..f9050dbfa 100644 --- a/git/__init__.py +++ b/git/__init__.py @@ -120,12 +120,20 @@ def refresh(path: Optional[PathLike] = None) -> None: - """Convenience method for setting the git executable path.""" + """ + Convenience method for setting the git and bash executable paths. + + Note that the default behavior of invoking commit hooks on Windows has + changed to not prefer WSL bash with the introduction of + `Git.GIT_PYTHON_BASH_EXECUTABLE`. See the `refresh_bash()` documentation + for details on the default values and search paths. + """ global GIT_OK GIT_OK = False if not Git.refresh(path=path): return + Git.refresh_bash() if not FetchInfo.refresh(): # noqa: F405 return # type: ignore [unreachable] diff --git a/git/cmd.py b/git/cmd.py index 4413182e0..b4f1bc329 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -15,6 +15,7 @@ import subprocess import threading from textwrap import dedent +from pathlib import Path from git.compat import defenc, force_bytes, safe_decode from git.exc import ( @@ -360,9 +361,107 @@ def __setstate__(self, d: Dict[str, Any]) -> None: the top level ``__init__``. """ + _bash_exec_env_var = "GIT_PYTHON_BASH_EXECUTABLE" + + bash_exec_name = "bash" + """Default bash command.""" + + GIT_PYTHON_BASH_EXECUTABLE = None + """ + Provides the path to the bash executable used for commit hooks. This is + ordinarily set by `Git.refresh_bash()`. Note that the default behavior of + invoking commit hooks on Windows has changed to not prefer WSL bash with + the introduction of this variable. See the `Git.refresh_bash()` + documentation for details on the default values and search paths. + """ + + @classmethod + def _get_default_bash_path(cls) -> str: + # Assumes that, if 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()`. + # Uses the output of `git --exec-path` for the currently configured + # Git command to find its `git-core` directory. If one assumes that + # the `git-core` directory is always three levels deeper than the + # root directory of the Git installation, we can try going up three + # levels and then navigating to (root)/bin/bash.exe. If this exists, + # prefer it over the WSL version in System32, direct access to which + # is reportedly deprecated. Fail back to default "bash.exe" if + # the Git for Windows lookup doesn't work. + # + # 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. A noteworthy example are repos + # with Git LFS, where Git LFS may be installed in Windows but not + # in WSL. + if os.name != "nt": + return "bash" + gitcore = Path(cls()._call_process("--exec-path")) + gitroot = gitcore.parent.parent.parent + gitbash = gitroot / "bin" / "bash.exe" + return str(gitbash) if gitbash.exists() else "bash.exe" + + @classmethod + def refresh_bash(cls, path: Union[None, PathLike] = None) -> bool: + """ + Refreshes the cached path to the bash executable used for executing + commit hook scripts. This gets called by the top-level `refresh()` + function on initial package import (see the top level __init__), but + this method may be invoked manually if the path changes after import. + + This method only checks one path for a valid bash executable at a time, + using the first non-empty path provided in the following priority + order: + + 1. the explicit `path` argument to this method + 2. the environment variable `GIT_PYTHON_BASH_EXECUTABLE` if it is set + and available via `os.environ` upon calling this method + 3. if the current platform is not Windows, the simple string `"bash"` + 4. if the current platform is Windows, inferred from the current + provided Git executable assuming it is part of a Git for Windows + distribution. + + The current platform is checked based on the call `os.name`. + + This is a change to the default behavior from previous versions of + GitPython. In the event backwards compatibility is needed, the `path` + argument or the environment variable may be set to the string + `"bash.exe"`, which on most systems invokes the WSL bash by default. + + This change to default behavior 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. A noteworthy example are + repos with Git LFS, where Git LFS may be installed in Windows but not + in WSL. + """ + # Discern which path to refresh with. + if path is not None: + new_bash = os.path.expanduser(path) + # new_bash = os.path.abspath(new_bash) + else: + new_bash = os.environ.get(cls._bash_exec_env_var) + if not new_bash: + new_bash = cls._get_default_bash_path() + + # Keep track of the old and new bash executable path. + # old_bash = cls.GIT_PYTHON_BASH_EXECUTABLE + cls.GIT_PYTHON_BASH_EXECUTABLE = new_bash + + # Test if the new git executable path exists. + has_bash = Path(cls.GIT_PYTHON_BASH_EXECUTABLE).exists() + return has_bash + @classmethod def refresh(cls, path: Union[None, PathLike] = None) -> bool: - """This gets called by the refresh function (see the top level __init__).""" + """ + This gets called by the refresh function (see the top level __init__). + + Note that calling this method directly does not automatically update + the cached path to `bash`; either invoke the top level `refresh()` + function or call `Git.refresh_bash()` directly. + """ # Discern which path to refresh with. if path is not None: new_git = os.path.expanduser(path) diff --git a/git/index/fun.py b/git/index/fun.py index 580493f6d..0532bbb48 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -18,7 +18,7 @@ ) import subprocess -from git.cmd import handle_process_output, safer_popen +from git.cmd import handle_process_output, Git, safer_popen from git.compat import defenc, force_bytes, force_text, safe_decode from git.exc import HookExecutionError, UnmergedEntriesError from git.objects.fun import ( @@ -96,7 +96,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: # Windows only uses extensions to determine how to open files # (doesn't understand shebangs). Try using bash to run the hook. relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix() - cmd = ["bash.exe", relative_hp] + cmd = [Git.GIT_PYTHON_BASH_EXECUTABLE, relative_hp] process = safer_popen( cmd + list(args), diff --git a/test/test_index.py b/test/test_index.py index 22eac355b..660ab3695 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -1019,16 +1019,16 @@ class Mocked: rel = index._to_relative_path(path) self.assertEqual(rel, os.path.relpath(path, root)) - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.Absent, - reason="Can't run a hook on Windows without bash.exe.", - rasies=HookExecutionError, - ) - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.WslNoDistro, - reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", - raises=HookExecutionError, - ) + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.Absent, + # reason="Can't run a hook on Windows without bash.exe.", + # rasies=HookExecutionError, + # ) + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.WslNoDistro, + # reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", + # raises=HookExecutionError, + # ) @with_rw_repo("HEAD", bare=True) def test_run_commit_hook(self, rw_repo): index = rw_repo.index @@ -1064,41 +1064,41 @@ def test_hook_uses_shell_not_from_cwd(self, rw_dir, case): shutil.copy(fixture_path("polyglot"), hook_path("polyglot", repo.git_dir)) payload = Path(rw_dir, "payload.txt") - if type(_win_bash_status) in {WinBashStatus.Absent, WinBashStatus.WslNoDistro}: - # The real shell can't run, but the impostor should still not be used. - with self.assertRaises(HookExecutionError): - with maybe_chdir: - run_commit_hook("polyglot", repo.index) - self.assertFalse(payload.exists()) - else: - # The real shell should run, and not the impostor. - with maybe_chdir: - run_commit_hook("polyglot", repo.index) - self.assertFalse(payload.exists()) - output = Path(rw_dir, "output.txt").read_text(encoding="utf-8") - self.assertEqual(output, "Ran intended hook.\n") - - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.Absent, - reason="Can't run a hook on Windows without bash.exe.", - rasies=HookExecutionError, - ) - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.WslNoDistro, - reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", - raises=HookExecutionError, - ) + # if type(_win_bash_status) in {WinBashStatus.Absent, WinBashStatus.WslNoDistro}: + # # The real shell can't run, but the impostor should still not be used. + # with self.assertRaises(HookExecutionError): + # with maybe_chdir: + # run_commit_hook("polyglot", repo.index) + # self.assertFalse(payload.exists()) + # else: + # The real shell should run, and not the impostor. + with maybe_chdir: + run_commit_hook("polyglot", repo.index) + self.assertFalse(payload.exists()) + output = Path(rw_dir, "output.txt").read_text(encoding="utf-8") + self.assertEqual(output, "Ran intended hook.\n") + + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.Absent, + # reason="Can't run a hook on Windows without bash.exe.", + # rasies=HookExecutionError, + # ) + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.WslNoDistro, + # reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", + # raises=HookExecutionError, + # ) @with_rw_repo("HEAD", bare=True) def test_pre_commit_hook_success(self, rw_repo): index = rw_repo.index _make_hook(index.repo.git_dir, "pre-commit", "exit 0") index.commit("This should not fail") - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.WslNoDistro, - reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", - raises=AssertionError, - ) + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.WslNoDistro, + # reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", + # raises=AssertionError, + # ) @with_rw_repo("HEAD", bare=True) def test_pre_commit_hook_fail(self, rw_repo): index = rw_repo.index @@ -1121,21 +1121,21 @@ def test_pre_commit_hook_fail(self, rw_repo): else: raise AssertionError("Should have caught a HookExecutionError") - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.Absent, - reason="Can't run a hook on Windows without bash.exe.", - rasies=HookExecutionError, - ) - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.Wsl, - reason="Specifically seems to fail on WSL bash (in spite of #1399)", - raises=AssertionError, - ) - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.WslNoDistro, - reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", - raises=HookExecutionError, - ) + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.Absent, + # reason="Can't run a hook on Windows without bash.exe.", + # rasies=HookExecutionError, + # ) + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.Wsl, + # reason="Specifically seems to fail on WSL bash (in spite of #1399)", + # raises=AssertionError, + # ) + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.WslNoDistro, + # reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", + # raises=HookExecutionError, + # ) @with_rw_repo("HEAD", bare=True) def test_commit_msg_hook_success(self, rw_repo): commit_message = "commit default head by Frèderic Çaufl€" @@ -1149,11 +1149,11 @@ def test_commit_msg_hook_success(self, rw_repo): new_commit = index.commit(commit_message) self.assertEqual(new_commit.message, "{} {}".format(commit_message, from_hook_message)) - @pytest.mark.xfail( - type(_win_bash_status) is WinBashStatus.WslNoDistro, - reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", - raises=AssertionError, - ) + # @pytest.mark.xfail( + # type(_win_bash_status) is WinBashStatus.WslNoDistro, + # reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", + # raises=AssertionError, + # ) @with_rw_repo("HEAD", bare=True) def test_commit_msg_hook_fail(self, rw_repo): index = rw_repo.index