Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override bash executable, defaulting to Git for Windows git bash over WSL bash #1791

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Override bash executable, defaulting to Git for Windows git bash over WSL bash #1791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
77f74ce
initial patch to use git bash rather than WSL
emanspeaks Jan 9, 2024
4bde10d
fix typos preventing import
emanspeaks Jan 9, 2024
6676668
remove rogue temp file
emanspeaks Jan 9, 2024
50c74f4
updates after running black
emanspeaks Jan 9, 2024
bf919f6
Merge branch 'main' into fix-bash-exe
emanspeaks Jan 21, 2024
f065d1f
address first round of comments from PR #1791
emanspeaks Jan 21, 2024
0c14cac
address lint issue
emanspeaks Jan 21, 2024
8200ad1
possible fix to test_hook_uses_shell_not_from_cwd
emanspeaks Jan 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/pythonpackage.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected]
with:
distribution: Debian
# - name: Set up WSL (Windows)
# if: startsWith(matrix.os, 'windows')
# uses: Vampire/[email protected]
# with:
# distribution: Debian
Comment on lines +38 to +42
Copy link
Contributor

@EliahKagan EliahKagan Jan 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the improvement from this PR, we shouldn't need to install a WSL distribution on CI anymore, so this commented-out code can be removed altogether. It will still be in the history and can be brought back if needed in the future. (I'm guessing you may have been planning to do this anyway, but I figured I'd mention it where applicable.)


- name: Prepare this repo for tests
run: |
Expand Down
1 change: 1 addition & 0 deletions AUTHORS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
10 changes: 9 additions & 1 deletion git/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
101 changes: 100 additions & 1 deletion git/cmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems not to be used. I'm not sure it's needed. If it is kept, then _get_default_bash_path should probably return it on non-Windows systems and possibly return f"{cla.bash_exec_name}.exe" as the fallback on Windows systems. Although I believe it was for symmetry with git_exec_name, the treatment of git and bash has become less similar in recent changes, so I'm not sure there's a need for this anymore.

"""Default bash command."""

GIT_PYTHON_BASH_EXECUTABLE = None
"""
Provides the path to the bash executable used for commit hooks. This is
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend adding "on Windows" to the first sentence, both here and in the refresh_bash docstring. It is set on other systems, and future software that uses GitPython may itself use it directly on other systems (though that would be unusual), but GitPython itself is only using it on Windows.

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.
emanspeaks marked this conversation as resolved.
Show resolved Hide resolved
#
# 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.
emanspeaks marked this conversation as resolved.
Show resolved Hide resolved
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:
emanspeaks marked this conversation as resolved.
Show resolved Hide resolved
"""
Refreshes the cached path to the bash executable used for executing
commit hook scripts. This gets called by the top-level `refresh()`
Comment on lines +408 to +409
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above for GIT_PYTHON_BASH_EXECUTABLE, I suggest adding "on Windows" to the first sentence, since the path this sets is not used by GitPython on other systems. Otherwise, this is saying that hook scripts are executed with this bash interpreter on all systems, which is not accurate, and the documentation of the behavior on non-Windows systems lower down in this same docstring--which is itself not a problem--will reinforce that wrong information.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path argument seems to be used if not None, even if it is the empty string. Although people shouldn't do that, the behavior seems correct, since passing a string as path should cause that to be used.

This description could be changed to just say "the first path provided..." because it is reasonable to interpret an empty environment variable as not providing a path, which is what I would suggest. But if you feel that's not accurate enough, then it could be expanded to be more specific, or something about the environment variable's value being nonempty could be added in item 2 of the numbered list.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this line about os.environ is not needed. But this is subjective and if you feel it is better to keep this then I think that is okay.

It's possible to set environment variables in a way that does not make them available in os.environ, such as by calling os.putenv directly, but this is rare and the limitations of doing so are documented. Environment variables set that way are not expected to be visible to Python code in the same process. Another possible reason to mention os.environ is for the distinction between it and os.environb, though other places where a path is obtained from os.environ don't mention this. (If the distinction between Unicode and bytes is what makes this significant, then that should probably be stated more directly.)

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.
Comment on lines +420 to +423
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This omits the final fallback of bash.exe on Windows.


The current platform is checked based on the call `os.name`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is either an implementation detail, or an attempt to state something about the behavior that should be stated directly instead, and that it could be removed. But this is another subjective thing that I think you should feel free to leave as is, if you believe it is best to have it.

If any information related to the platform check is given, then I think the most relevant is that Cygwin isn't treated as Windows for this purpose. It is not treated as Windows for most purposes (in GitPython and also more broadly) so I don't think that has to be said, but the confusion that led to git.compat.is_win being deprecated was related to Cygwin.


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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commented-out line can be removed. If it is valuable to note that this step is not being done, then a comment could be added stating the relevant behavior.

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
Comment on lines +448 to +449
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commented-out code should be removed, including the comment above it that only applies to the now-commented-out line.

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)
Expand Down
4 changes: 2 additions & 2 deletions git/index/fun.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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]
emanspeaks marked this conversation as resolved.
Show resolved Hide resolved

process = safer_popen(
cmd + list(args),
Expand Down
118 changes: 59 additions & 59 deletions test/test_index.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
# )
Comment on lines +1022 to +1031
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These commented-out decorators can be removed. I don't think we'll need them anymore, and they'll remain in the history, so they can be brought back if needed. (As in pythonpackage.yml, I'm guessing you may already have been planning to do this here, but I figured I'd mention it.)

For tests, including this one, that are neither regression tests for security vulnerabilities nor tests of what happens when running hooks fails, I don't think there's any need to cover WSL or absent bash cases anymore.

If WinBashStatus.check is modified no longer to hard-code bash.exe, as described in my comment below on test_hook_uses_shell_not_from_cwd (approach 2), then an alternative to removing these decorators could be to restore them, since they would then be correct. Whatever is done, it should be done on the corresponding decorators on each of the functions where they appear. It seems to me that they can be removed even if WinBashStatus.check is modified in a way that would allow them to work.

@with_rw_repo("HEAD", bare=True)
def test_run_commit_hook(self, rw_repo):
index = rw_repo.index
Expand Down Expand Up @@ -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:
Comment on lines +1067 to +1073
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the best thing is to do here right now. Some of the modifications to the test suite to take advantage of the improvements in this pull request could be done later.

The purpose of this test method is to verify that an untrusted repository (or, more realistically, an untrusted branch, possibly from a fork) with a malicious bash.exe cannot cause that bash.exe to run, which was part of CVE-2024-22190. There are two general scenarios that should be tested: when there is another bash.exe that should run hooks, and when there isn't another one. In both cases, the impostor bash.exe should not run.

It is a shortcoming of #1792, which fixed CVE-2024-22190 and added this test, that both scenarios would not be tested in a single run of the test suite. This would have been tricky to do prior to the changes in this pull request, but I probably could and maybe should have done it. Instead, this tested whichever of the two scenarios applied to the test system. Both scenarios were common prior to the changes in this pull request, which makes the broken-or-unavailable bash scenario uncommon, though still possible.

Possible approaches:

  1. Assume a working bash.exe is available for the test. That's what this PR currently does, by commenting out the code for the broken-or-unavailable bash scenario. That code could even be removed, like other commented-out code, since it is in the repository's history. It could be replaced with a comment “TODO: Test that an impostor bash doesn't run in a realistic case of the "real" bash being broken or absent.” (or maybe there is a better wording).
  2. Modify WinBashStatus.check to work for checking the status of the bash interpreter that GitPython uses now. One way is to have it take an argument for the bash command and run that instead of hard-coding bash.exe. That would be passed in the call used to bind _win_bash_status. Then the existing check would work properly again. (It would even work in xfail conditions, but I think there is no good reason to cover it in those tests.) Further improvements could come later, including eventual removal of the WinBashStatus class. This is the approach that I would lean toward taking at this time.
  3. Produce both scenarios as separate test cases. This is ideal, but also seems like it may be outside this PR's scope. It may also be a bit tricky to do well, since the goal is for it to fail in a realistic regression of CVE-2024-22190. If such a bug were inadvertently introduced in the future, the exploit would probably be a bash.exe impostor in an untrusted repository/branch, and it would probably affect only the situation where the "bash.exe" fallback is used for GIT_PYTHON_BASH_EXECUTABLE. This can be hard to test reliably because Windows searches in some directories before using PATH, such as System32. However, the changes in this PR allow this to be tested a different way, by temporarily setting GIT_PYTHON_BASH_EXECUTABLE to a command name that is very unlikely to exist already, such as bash-7de052ca-c9e6-4e4c-abe8-4d168ecd74c6.exe, and naming the impostor that as well. This could be done without excessive code duplication by expanding the way the test is parametrized, or by splitting it into two separate methods that call a shared helper (or use a shared fixture).

I think any of those approaches, and probably various others I have not thought of, are reasonable.

# 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,
# )
Comment on lines +1081 to +1090
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As on test_run_commit_hook, this commented-out code can be removed.

@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,
# )
Comment on lines +1097 to +1101
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As on test_run_commit_hook and test_pre_commit_hook_success, this commented-out code can be removed.

The situation with the if type(_win_bash_status) is WinBashStatus.Absent: check in the method body is more subtle, because it would be useful to continue testing that we get a HookExecutionError, with the expected attributes, when bash is absent.

This is roughly analogous to the situation in test_hook_uses_shell_not_from_cwd. But the stakes are much lower here, because this does not test for a security vulnerability. Also, this is passing on CI, because the WinBashStatus.Absent case is fairly rare and not produced there: WinBashStatus.check finds bash.exe in System32 and concludes bash is present, then the Git Bash bash.exe is used due to the changes in this PR.

The possible approaches to test_hook_uses_shell_not_from_cwd are also possible here, but because this test is not failing, there is the further option of just commenting to note that the check is not always accurate. As there, I think further improvements to the cases this covers should eventually be made, and could be made here, but that their absence should not get in the way of this PR being merged.

@with_rw_repo("HEAD", bare=True)
def test_pre_commit_hook_fail(self, rw_repo):
index = rw_repo.index
Expand All @@ -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,
# )
Comment on lines +1124 to +1138
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As on test_run_commit_hook, test_pre_commit_hook_success, and test_pre_commit_hook_fail, this commented-out code can be removed.

I am not concerned about losing mention of the "Specifically seems to fail on WSL bash (in spite of #1399)" situation here, and I don't think even that commented-out decoration needs to be kept as comments here. After the improvements of this PR, I think most use of the WSL bash for hooks will be for people whose workflows were already working and who need it for backward compatibility. The solution to this problem, for people who experience it, is just to use the new default behavior.

If the #1399-related failure with WSL is considered important to keep track of, then a new issue can be opened about it (I would be pleased to do this on request), or a comment could briefly mention it, but it seems to me that neither is necessary at this time.

@with_rw_repo("HEAD", bare=True)
def test_commit_msg_hook_success(self, rw_repo):
commit_message = "commit default head by Frèderic Çaufl€"
Expand All @@ -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,
# )
Comment on lines +1152 to +1156
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As on test_run_commit_hook, test_pre_commit_hook_success, test_pre_commit_hook_fail, and test_commit_msg_hook_success, this commented-out code can be removed.

@with_rw_repo("HEAD", bare=True)
def test_commit_msg_hook_fail(self, rw_repo):
index = rw_repo.index
Expand Down
Loading