Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5d11394

Browse files
committed
Fix and expand bash.exe xfail marks on hook tests
881456b (#1679) expanded the use of shutil.which in test_index.pyto attempt to mark test_commit_msg_hook_success xfail when bash.exeis a WSL bash wrapper in System32 (because that test currentlyis not passing when the hook is run via bash in a WSL system, whichthe WSL bash.exe wraps). But this was not reliable, due tosignificant differences between shell and non-shell search behaviorfor executable commands on Windows. As the new docstring notes,lookup due to Popen generally checks System32 before going throughdirectories in PATH, as this is how CreateProcessW behaves.-https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw-python/cpython#91558 (comment)The technique I had used in881456b also had the shortcoming ofassuming that a bash.exe in System32 was the WSL wrapper. But sucha file may exist on some systems without WSL, and be a bashinterpreter unrelated to WSL that may be able to run hooks.In addition, one common situation, which was the case on CI beforea step to install a WSL distribution was added, is that WSL ispresent but no WSL distributions are installed. In that situationbash.exe is found in System32, but it can't be used to run anyhooks, because there's no actual system with a bash in it to wrap.This was not covered before. Unlike some conditions that preventa WSL system from being used, such as resource exhaustion blockingit from being started, the absence of a WSL system should probablynot fail the pytest run, for the same reason as we are trying notto have the complete *absence* of bash.exe fail the pytest run.Both conditions should be xfail. Fortunately, the error messagewhen no distribution exists is distinctive and can be checked for.There is probably no correct and reasonable way to check LBYL-stylewhich executable file bash.exe resolves to by using shutil.which,due to shutil.which and subprocess.Popen's differing seach ordersand other subtleties. So this adds code to do it EAFP-style usingsubprocess.run (which itself uses Popen, so giving the sameCreateProcessW behavior). It tries to run a command with bash.exewhose output pretty reliably shows if the system is WSL or not.We may fail to get to the point of running that command at all, ifbash.exe is not usable, in which case the failure's details tell usif bash.exe is absent (xfail), present as the WSL wrapper with no WSLsystems (xfail), or has some other error (not xfail, so the user canbecome aware of and proably fix the problem). If we do get to thatpoint, then a file that is almost always present on both WSL 1 andWSL 2 systems and almost always absent on any other system ischecked for, to distinguish whether the working bash shell operatesin a WSL system, or outside any such system as e.g. Git Bash does.Seehttps://superuser.com/a/1749811 on various techniques forchecking for WSL, including the /proc/sys/fs/binfmt_misc/WSLInteroptechnique used here (which seems overall may be the most reliable).Although the Windows CI runners have Git Bash, and this is even thebash.exe that appears first in PATH (giving rise to the problemwith shutil.which detailed above), it would be a bit awkward totest the behavior when Git Bash is actually used to run hooks onCI, because of how Popen selects the System32 bash.exe first,whether or not any WSL distribution is present. However, localtesting shows that when Git Bash's bash.exe is selected, all fourhook tests in the module pass, both before and after this change,and furthermore that bash.exe is correctly detected as "native",continuing to avoid an erroneous xfail mark in that case.
1 parent9717b8d commit5d11394

File tree

1 file changed

+113
-13
lines changed

1 file changed

+113
-13
lines changed

‎test/test_index.py

Lines changed: 113 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
# This module is part of GitPython and is released under the
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

6+
importenum
67
fromioimportBytesIO
8+
importlogging
79
importos
810
importos.pathasosp
911
frompathlibimportPath
1012
fromstatimportS_ISLNK,ST_MODE
11-
importshutil
13+
importsubprocess
1214
importtempfile
1315

1416
importpytest
@@ -34,19 +36,97 @@
3436

3537
HOOKS_SHEBANG="#!/usr/bin/env sh\n"
3638

39+
log=logging.getLogger(__name__)
3740

38-
def_found_in(cmd,directory):
39-
"""Check if a command is resolved in a directory (without following symlinks)."""
40-
path=shutil.which(cmd)
41-
returnpathandPath(path).parent==Path(directory)
4241

42+
class_WinBashMeta(enum.EnumMeta):
43+
"""Metaclass allowing :class:`_WinBash` custom behavior when called."""
4344

44-
is_win_without_bash=os.name=="nt"andnotshutil.which("bash.exe")
45+
def__call__(cls):
46+
returncls._check()
4547

46-
is_win_with_wsl_bash=os.name=="nt"and_found_in(
47-
cmd="bash.exe",
48-
directory=Path(os.getenv("WINDIR"))/"System32",
49-
)
48+
49+
@enum.unique
50+
class_WinBash(enum.Enum,metaclass=_WinBashMeta):
51+
"""Status of bash.exe for native Windows. Affects which commit hook tests can pass.
52+
53+
Call ``_WinBash()`` to check the status.
54+
55+
This can't be reliably discovered using :func:`shutil.which`, as that approximates
56+
how a shell is expected to search for an executable. On Windows, there are major
57+
differences between how executables are found by a shell and otherwise. (This is the
58+
cmd.exe Windows shell and should not be confused with bash.exe.) Our run_commit_hook
59+
function in GitPython uses subprocess.Popen, including to run bash.exe on Windows.
60+
It does not pass shell=True (and should not). Popen calls CreateProcessW, which
61+
searches several locations prior to using the PATH environment variable. It is
62+
expected to search the System32 directory, even if another directory containing the
63+
executable precedes it in PATH. (There are other differences, less relevant here.)
64+
When WSL is installed, even if no WSL *systems* are installed, bash.exe exists in
65+
System32, and Popen finds it even if another bash.exe precedes it in PATH, as
66+
happens on CI. If WSL is absent, System32 may still have bash.exe, as Windows users
67+
and administrators occasionally copy executables there in lieu of extending PATH.
68+
"""
69+
70+
INAPPLICABLE=enum.auto()
71+
"""This system is not native Windows: either not Windows at all, or Cygwin."""
72+
73+
ABSENT=enum.auto()
74+
"""No command for ``bash.exe`` is found on the system."""
75+
76+
NATIVE=enum.auto()
77+
"""Running ``bash.exe`` operates outside any WSL environment (as with Git Bash)."""
78+
79+
WSL=enum.auto()
80+
"""Running ``bash.exe`` runs bash on a WSL system."""
81+
82+
WSL_NO_DISTRO=enum.auto()
83+
"""Running ``bash.exe` tries to run bash on a WSL system, but none exists."""
84+
85+
ERROR_WHILE_CHECKING=enum.auto()
86+
"""Could not determine the status.
87+
88+
This should not trigger a skip or xfail, as it typically indicates either a fixable
89+
problem on the test machine, such as an "Insufficient system resources exist to
90+
complete the requested service" error starting WSL, or a bug in this detection code.
91+
``ERROR_WHILE_CHECKING.error_or_process`` has details about the most recent failure.
92+
"""
93+
94+
@classmethod
95+
def_check(cls):
96+
ifos.name!="nt":
97+
returncls.INAPPLICABLE
98+
99+
try:
100+
# Print rather than returning the test command's exit status so that if a
101+
# failure occurs before we even get to this point, we will detect it.
102+
script='test -e /proc/sys/fs/binfmt_misc/WSLInterop; echo "$?"'
103+
command= ["bash.exe","-c",script]
104+
proc=subprocess.run(command,capture_output=True,check=True,text=True)
105+
exceptFileNotFoundError:
106+
returncls.ABSENT
107+
exceptOSErroraserror:
108+
returncls._error(error)
109+
exceptsubprocess.CalledProcessErroraserror:
110+
no_distro_message="Windows Subsystem for Linux has no installed distributions."
111+
iferror.returncode==1anderror.stdout.startswith(no_distro_message):
112+
returncls.WSL_NO_DISTRO
113+
returncls._error(error)
114+
115+
status=proc.stdout.rstrip()
116+
ifstatus=="0":
117+
returncls.WSL
118+
ifstatus=="1":
119+
returncls.NATIVE
120+
returncls._error(proc)
121+
122+
@classmethod
123+
def_error(cls,error_or_process):
124+
ifisinstance(error_or_process,subprocess.CompletedProcess):
125+
log.error("Strange output checking WSL status: %s",error_or_process.stdout)
126+
else:
127+
log.error("Error running bash.exe to check WSL status: %s",error_or_process)
128+
cls.ERROR_WHILE_CHECKING.error_or_process=error_or_process
129+
returncls.ERROR_WHILE_CHECKING
50130

51131

52132
def_make_hook(git_dir,name,content,make_exec=True):
@@ -917,20 +997,30 @@ class Mocked:
917997
rel=index._to_relative_path(path)
918998
self.assertEqual(rel,os.path.relpath(path,root))
919999

1000+
@pytest.mark.xfail(
1001+
_WinBash()is_WinBash.WSL_NO_DISTRO,
1002+
reason="Currently uses the bash.exe for WSL even with no WSL distro installed",
1003+
raises=HookExecutionError,
1004+
)
9201005
@with_rw_repo("HEAD",bare=True)
9211006
deftest_pre_commit_hook_success(self,rw_repo):
9221007
index=rw_repo.index
9231008
_make_hook(index.repo.git_dir,"pre-commit","exit 0")
9241009
index.commit("This should not fail")
9251010

1011+
@pytest.mark.xfail(
1012+
_WinBash()is_WinBash.WSL_NO_DISTRO,
1013+
reason="Currently uses the bash.exe for WSL even with no WSL distro installed",
1014+
raises=AssertionError,
1015+
)
9261016
@with_rw_repo("HEAD",bare=True)
9271017
deftest_pre_commit_hook_fail(self,rw_repo):
9281018
index=rw_repo.index
9291019
hp=_make_hook(index.repo.git_dir,"pre-commit","echo stdout; echo stderr 1>&2; exit 1")
9301020
try:
9311021
index.commit("This should fail")
9321022
exceptHookExecutionErroraserr:
933-
ifis_win_without_bash:
1023+
if_WinBash()is_WinBash.ABSENT:
9341024
self.assertIsInstance(err.status,OSError)
9351025
self.assertEqual(err.command, [hp])
9361026
self.assertEqual(err.stdout,"")
@@ -946,10 +1036,15 @@ def test_pre_commit_hook_fail(self, rw_repo):
9461036
raiseAssertionError("Should have caught a HookExecutionError")
9471037

9481038
@pytest.mark.xfail(
949-
is_win_without_bashoris_win_with_wsl_bash,
1039+
_WinBash()in {_WinBash.ABSENT,_WinBash.WSL},
9501040
reason="Specifically seems to fail on WSL bash (in spite of #1399)",
9511041
raises=AssertionError,
9521042
)
1043+
@pytest.mark.xfail(
1044+
_WinBash()is_WinBash.WSL_NO_DISTRO,
1045+
reason="Currently uses the bash.exe for WSL even with no WSL distro installed",
1046+
raises=HookExecutionError,
1047+
)
9531048
@with_rw_repo("HEAD",bare=True)
9541049
deftest_commit_msg_hook_success(self,rw_repo):
9551050
commit_message="commit default head by Frèderic Çaufl€"
@@ -963,14 +1058,19 @@ def test_commit_msg_hook_success(self, rw_repo):
9631058
new_commit=index.commit(commit_message)
9641059
self.assertEqual(new_commit.message,"{} {}".format(commit_message,from_hook_message))
9651060

1061+
@pytest.mark.xfail(
1062+
_WinBash()is_WinBash.WSL_NO_DISTRO,
1063+
reason="Currently uses the bash.exe for WSL even with no WSL distro installed",
1064+
raises=AssertionError,
1065+
)
9661066
@with_rw_repo("HEAD",bare=True)
9671067
deftest_commit_msg_hook_fail(self,rw_repo):
9681068
index=rw_repo.index
9691069
hp=_make_hook(index.repo.git_dir,"commit-msg","echo stdout; echo stderr 1>&2; exit 1")
9701070
try:
9711071
index.commit("This should fail")
9721072
exceptHookExecutionErroraserr:
973-
ifis_win_without_bash:
1073+
if_WinBash()is_WinBash.ABSENT:
9741074
self.assertIsInstance(err.status,OSError)
9751075
self.assertEqual(err.command, [hp])
9761076
self.assertEqual(err.stdout,"")

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp