Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit ef3192c

Browse files
authoredJan 10, 2024
Merge pull request #1792 from EliahKagan/popen
Fix two remaining Windows untrusted search path cases
2 parents 32c02d1 + 1f3caa3 commit ef3192c

File tree

9 files changed

+289
-84
lines changed

9 files changed

+289
-84
lines changed
 

‎.pre-commit-config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ repos:
2929
hooks:
3030
- id: shellcheck
3131
args: [--color]
32-
exclude: ^git/ext/
32+
exclude: ^test/fixtures/polyglot$|^git/ext/
3333

3434
- repo: https://github.com/pre-commit/pre-commit-hooks
3535
rev: v4.4.0

‎git/cmd.py

Lines changed: 76 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
Iterator,
4747
List,
4848
Mapping,
49+
Optional,
4950
Sequence,
5051
TYPE_CHECKING,
5152
TextIO,
@@ -102,7 +103,7 @@ def handle_process_output(
102103
Callable[[bytes, "Repo", "DiffIndex"], None],
103104
],
104105
stderr_handler: Union[None, Callable[[AnyStr], None], Callable[[List[AnyStr]], None]],
105-
finalizer: Union[None, Callable[[Union[subprocess.Popen, "Git.AutoInterrupt"]], None]] = None,
106+
finalizer: Union[None, Callable[[Union[Popen, "Git.AutoInterrupt"]], None]] = None,
106107
decode_streams: bool = True,
107108
kill_after_timeout: Union[None, float] = None,
108109
) -> None:
@@ -207,6 +208,68 @@ def pump_stream(
207208
finalizer(process)
208209

209210

211+
def _safer_popen_windows(
212+
command: Union[str, Sequence[Any]],
213+
*,
214+
shell: bool = False,
215+
env: Optional[Mapping[str, str]] = None,
216+
**kwargs: Any,
217+
) -> Popen:
218+
"""Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search.
219+
220+
This avoids an untrusted search path condition where a file like ``git.exe`` in a
221+
malicious repository would be run when GitPython operates on the repository. The
222+
process using GitPython may have an untrusted repository's working tree as its
223+
current working directory. Some operations may temporarily change to that directory
224+
before running a subprocess. In addition, while by default GitPython does not run
225+
external commands with a shell, it can be made to do so, in which case the CWD of
226+
the subprocess, which GitPython usually sets to a repository working tree, can
227+
itself be searched automatically by the shell. This wrapper covers all those cases.
228+
229+
:note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath``
230+
environment variable during subprocess creation. It also takes care of passing
231+
Windows-specific process creation flags, but that is unrelated to path search.
232+
233+
:note: The current implementation contains a race condition on :attr:`os.environ`.
234+
GitPython isn't thread-safe, but a program using it on one thread should ideally
235+
be able to mutate :attr:`os.environ` on another, without unpredictable results.
236+
See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
237+
"""
238+
# CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
239+
# https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
240+
# https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
241+
creationflags = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
242+
243+
# When using a shell, the shell is the direct subprocess, so the variable must be
244+
# set in its environment, to affect its search behavior. (The "1" can be any value.)
245+
if shell:
246+
safer_env = {} if env is None else dict(env)
247+
safer_env["NoDefaultCurrentDirectoryInExePath"] = "1"
248+
else:
249+
safer_env = env
250+
251+
# When not using a shell, the current process does the search in a CreateProcessW
252+
# API call, so the variable must be set in our environment. With a shell, this is
253+
# unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
254+
# patched. If not, in the rare case the ComSpec environment variable is unset, the
255+
# shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all
256+
# cases, as here, is simpler and protects against that. (The "1" can be any value.)
257+
with patch_env("NoDefaultCurrentDirectoryInExePath", "1"):
258+
return Popen(
259+
command,
260+
shell=shell,
261+
env=safer_env,
262+
creationflags=creationflags,
263+
**kwargs,
264+
)
265+
266+
267+
if os.name == "nt":
268+
safer_popen = _safer_popen_windows
269+
else:
270+
safer_popen = Popen
271+
272+
210273
def dashify(string: str) -> str:
211274
return string.replace("_", "-")
212275

@@ -225,14 +288,6 @@ def dict_to_slots_and__excluded_are_none(self: object, d: Mapping[str, Any], exc
225288
## -- End Utilities -- @}
226289

227290

228-
if os.name == "nt":
229-
# CREATE_NEW_PROCESS_GROUP is needed to allow killing it afterwards. See:
230-
# https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
231-
PROC_CREATIONFLAGS = subprocess.CREATE_NO_WINDOW | subprocess.CREATE_NEW_PROCESS_GROUP
232-
else:
233-
PROC_CREATIONFLAGS = 0
234-
235-
236291
class Git(LazyMixin):
237292
"""The Git class manages communication with the Git binary.
238293
@@ -992,11 +1047,8 @@ def execute(
9921047
redacted_command,
9931048
'"kill_after_timeout" feature is not supported on Windows.',
9941049
)
995-
# Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value.
996-
maybe_patch_caller_env = patch_env("NoDefaultCurrentDirectoryInExePath", "1")
9971050
else:
9981051
cmd_not_found_exception = FileNotFoundError
999-
maybe_patch_caller_env = contextlib.nullcontext()
10001052
# END handle
10011053

10021054
stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb")
@@ -1011,20 +1063,18 @@ def execute(
10111063
universal_newlines,
10121064
)
10131065
try:
1014-
with maybe_patch_caller_env:
1015-
proc = Popen(
1016-
command,
1017-
env=env,
1018-
cwd=cwd,
1019-
bufsize=-1,
1020-
stdin=(istream or DEVNULL),
1021-
stderr=PIPE,
1022-
stdout=stdout_sink,
1023-
shell=shell,
1024-
universal_newlines=universal_newlines,
1025-
creationflags=PROC_CREATIONFLAGS,
1026-
**subprocess_kwargs,
1027-
)
1066+
proc = safer_popen(
1067+
command,
1068+
env=env,
1069+
cwd=cwd,
1070+
bufsize=-1,
1071+
stdin=(istream or DEVNULL),
1072+
stderr=PIPE,
1073+
stdout=stdout_sink,
1074+
shell=shell,
1075+
universal_newlines=universal_newlines,
1076+
**subprocess_kwargs,
1077+
)
10281078
except cmd_not_found_exception as err:
10291079
raise GitCommandNotFound(redacted_command, err) from err
10301080
else:

‎git/index/fun.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
)
1919
import subprocess
2020

21-
from git.cmd import PROC_CREATIONFLAGS, handle_process_output
21+
from git.cmd import handle_process_output, safer_popen
2222
from git.compat import defenc, force_bytes, force_text, safe_decode
2323
from git.exc import HookExecutionError, UnmergedEntriesError
2424
from git.objects.fun import (
@@ -98,13 +98,12 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None:
9898
relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix()
9999
cmd = ["bash.exe", relative_hp]
100100

101-
process = subprocess.Popen(
101+
process = safer_popen(
102102
cmd + list(args),
103103
env=env,
104104
stdout=subprocess.PIPE,
105105
stderr=subprocess.PIPE,
106106
cwd=index.repo.working_dir,
107-
creationflags=PROC_CREATIONFLAGS,
108107
)
109108
except Exception as ex:
110109
raise HookExecutionError(hp, ex) from ex

‎git/util.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,17 @@ def _get_exe_extensions() -> Sequence[str]:
327327

328328

329329
def py_where(program: str, path: Optional[PathLike] = None) -> List[str]:
330+
"""Perform a path search to assist :func:`is_cygwin_git`.
331+
332+
This is not robust for general use. It is an implementation detail of
333+
:func:`is_cygwin_git`. When a search following all shell rules is needed,
334+
:func:`shutil.which` can be used instead.
335+
336+
:note: Neither this function nor :func:`shutil.which` will predict the effect of an
337+
executable search on a native Windows system due to a :class:`subprocess.Popen`
338+
call without ``shell=True``, because shell and non-shell executable search on
339+
Windows differ considerably.
340+
"""
330341
# From: http://stackoverflow.com/a/377028/548792
331342
winprog_exts = _get_exe_extensions()
332343

‎test/fixtures/polyglot

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#!/usr/bin/env sh
2+
# Valid script in both Bash and Python, but with different behavior.
3+
""":"
4+
echo 'Ran intended hook.' >output.txt
5+
exit
6+
" """
7+
from pathlib import Path
8+
Path('payload.txt').write_text('Ran impostor hook!', encoding='utf-8')

‎test/lib/helper.py

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import textwrap
1515
import time
1616
import unittest
17+
import venv
1718

1819
import gitdb
1920

@@ -36,6 +37,7 @@
3637
"with_rw_repo",
3738
"with_rw_and_rw_remote_repo",
3839
"TestBase",
40+
"VirtualEnvironment",
3941
"TestCase",
4042
"SkipTest",
4143
"skipIf",
@@ -88,11 +90,11 @@ def with_rw_directory(func):
8890
test succeeds, but leave it otherwise to aid additional debugging."""
8991

9092
@wraps(func)
91-
def wrapper(self):
93+
def wrapper(self, *args, **kwargs):
9294
path = tempfile.mkdtemp(prefix=func.__name__)
9395
keep = False
9496
try:
95-
return func(self, path)
97+
return func(self, path, *args, **kwargs)
9698
except Exception:
9799
log.info(
98100
"Test %s.%s failed, output is at %r\n",
@@ -390,3 +392,46 @@ def _make_file(self, rela_path, data, repo=None):
390392
with open(abs_path, "w") as fp:
391393
fp.write(data)
392394
return abs_path
395+
396+
397+
class VirtualEnvironment:
398+
"""A newly created Python virtual environment for use in a test."""
399+
400+
__slots__ = ("_env_dir",)
401+
402+
def __init__(self, env_dir, *, with_pip):
403+
if os.name == "nt":
404+
self._env_dir = osp.realpath(env_dir)
405+
venv.create(self.env_dir, symlinks=False, with_pip=with_pip)
406+
else:
407+
self._env_dir = env_dir
408+
venv.create(self.env_dir, symlinks=True, with_pip=with_pip)
409+
410+
@property
411+
def env_dir(self):
412+
"""The top-level directory of the environment."""
413+
return self._env_dir
414+
415+
@property
416+
def python(self):
417+
"""Path to the Python executable in the environment."""
418+
return self._executable("python")
419+
420+
@property
421+
def pip(self):
422+
"""Path to the pip executable in the environment, or RuntimeError if absent."""
423+
return self._executable("pip")
424+
425+
@property
426+
def sources(self):
427+
"""Path to a src directory in the environment, which may not exist yet."""
428+
return os.path.join(self.env_dir, "src")
429+
430+
def _executable(self, basename):
431+
if os.name == "nt":
432+
path = osp.join(self.env_dir, "Scripts", basename + ".exe")
433+
else:
434+
path = osp.join(self.env_dir, "bin", basename)
435+
if osp.isfile(path) or osp.islink(path):
436+
return path
437+
raise RuntimeError(f"no regular file or symlink {path!r}")

‎test/test_git.py

Lines changed: 70 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,18 @@
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+
import contextlib
67
import gc
78
import inspect
89
import logging
910
import os
1011
import os.path as osp
12+
from pathlib import Path
1113
import re
1214
import shutil
1315
import subprocess
1416
import sys
15-
from tempfile import TemporaryDirectory, TemporaryFile
17+
from tempfile import TemporaryFile
1618
from unittest import skipUnless
1719

1820
if sys.version_info >= (3, 8):
@@ -27,6 +29,21 @@
2729
from test.lib import TestBase, fixture_path, with_rw_directory
2830

2931

32+
@contextlib.contextmanager
33+
def _patch_out_env(name):
34+
try:
35+
old_value = os.environ[name]
36+
except KeyError:
37+
old_value = None
38+
else:
39+
del os.environ[name]
40+
try:
41+
yield
42+
finally:
43+
if old_value is not None:
44+
os.environ[name] = old_value
45+
46+
3047
@ddt.ddt
3148
class TestGit(TestBase):
3249
@classmethod
@@ -97,29 +114,28 @@ def test_it_transforms_kwargs_into_git_command_arguments(self):
97114

98115
def _do_shell_combo(self, value_in_call, value_from_class):
99116
with mock.patch.object(Git, "USE_SHELL", value_from_class):
100-
# git.cmd gets Popen via a "from" import, so patch it there.
101-
with mock.patch.object(cmd, "Popen", wraps=cmd.Popen) as mock_popen:
117+
with mock.patch.object(cmd, "safer_popen", wraps=cmd.safer_popen) as mock_safer_popen:
102118
# Use a command with no arguments (besides the program name), so it runs
103119
# with or without a shell, on all OSes, with the same effect.
104120
self.git.execute(["git"], with_exceptions=False, shell=value_in_call)
105121

106-
return mock_popen
122+
return mock_safer_popen
107123

108124
@ddt.idata(_shell_cases)
109125
def test_it_uses_shell_or_not_as_specified(self, case):
110126
"""A bool passed as ``shell=`` takes precedence over `Git.USE_SHELL`."""
111127
value_in_call, value_from_class, expected_popen_arg = case
112-
mock_popen = self._do_shell_combo(value_in_call, value_from_class)
113-
mock_popen.assert_called_once()
114-
self.assertIs(mock_popen.call_args.kwargs["shell"], expected_popen_arg)
128+
mock_safer_popen = self._do_shell_combo(value_in_call, value_from_class)
129+
mock_safer_popen.assert_called_once()
130+
self.assertIs(mock_safer_popen.call_args.kwargs["shell"], expected_popen_arg)
115131

116132
@ddt.idata(full_case[:2] for full_case in _shell_cases)
117133
def test_it_logs_if_it_uses_a_shell(self, case):
118134
"""``shell=`` in the log message agrees with what is passed to `Popen`."""
119135
value_in_call, value_from_class = case
120136
with self.assertLogs(cmd.log, level=logging.DEBUG) as log_watcher:
121-
mock_popen = self._do_shell_combo(value_in_call, value_from_class)
122-
self._assert_logged_for_popen(log_watcher, "shell", mock_popen.call_args.kwargs["shell"])
137+
mock_safer_popen = self._do_shell_combo(value_in_call, value_from_class)
138+
self._assert_logged_for_popen(log_watcher, "shell", mock_safer_popen.call_args.kwargs["shell"])
123139

124140
@ddt.data(
125141
("None", None),
@@ -134,22 +150,49 @@ def test_it_logs_istream_summary_for_stdin(self, case):
134150
def test_it_executes_git_and_returns_result(self):
135151
self.assertRegex(self.git.execute(["git", "version"]), r"^git version [\d\.]{2}.*$")
136152

137-
def test_it_executes_git_not_from_cwd(self):
138-
with TemporaryDirectory() as tmpdir:
139-
if os.name == "nt":
140-
# Copy an actual binary executable that is not git.
141-
other_exe_path = os.path.join(os.getenv("WINDIR"), "system32", "hostname.exe")
142-
impostor_path = os.path.join(tmpdir, "git.exe")
143-
shutil.copy(other_exe_path, impostor_path)
144-
else:
145-
# Create a shell script that doesn't do anything.
146-
impostor_path = os.path.join(tmpdir, "git")
147-
with open(impostor_path, mode="w", encoding="utf-8") as file:
148-
print("#!/bin/sh", file=file)
149-
os.chmod(impostor_path, 0o755)
150-
151-
with cwd(tmpdir):
152-
self.assertRegex(self.git.execute(["git", "version"]), r"^git version\b")
153+
@ddt.data(
154+
# chdir_to_repo, shell, command, use_shell_impostor
155+
(False, False, ["git", "version"], False),
156+
(False, True, "git version", False),
157+
(False, True, "git version", True),
158+
(True, False, ["git", "version"], False),
159+
(True, True, "git version", False),
160+
(True, True, "git version", True),
161+
)
162+
@with_rw_directory
163+
def test_it_executes_git_not_from_cwd(self, rw_dir, case):
164+
chdir_to_repo, shell, command, use_shell_impostor = case
165+
166+
repo = Repo.init(rw_dir)
167+
168+
if os.name == "nt":
169+
# Copy an actual binary executable that is not git. (On Windows, running
170+
# "hostname" only displays the hostname, it never tries to change it.)
171+
other_exe_path = Path(os.environ["SystemRoot"], "system32", "hostname.exe")
172+
impostor_path = Path(rw_dir, "git.exe")
173+
shutil.copy(other_exe_path, impostor_path)
174+
else:
175+
# Create a shell script that doesn't do anything.
176+
impostor_path = Path(rw_dir, "git")
177+
impostor_path.write_text("#!/bin/sh\n", encoding="utf-8")
178+
os.chmod(impostor_path, 0o755)
179+
180+
if use_shell_impostor:
181+
shell_name = "cmd.exe" if os.name == "nt" else "sh"
182+
shutil.copy(impostor_path, Path(rw_dir, shell_name))
183+
184+
with contextlib.ExitStack() as stack:
185+
if chdir_to_repo:
186+
stack.enter_context(cwd(rw_dir))
187+
if use_shell_impostor:
188+
stack.enter_context(_patch_out_env("ComSpec"))
189+
190+
# Run the command without raising an exception on failure, as the exception
191+
# message is currently misleading when the command is a string rather than a
192+
# sequence of strings (it really runs "git", but then wrongly reports "g").
193+
output = repo.git.execute(command, with_exceptions=False, shell=shell)
194+
195+
self.assertRegex(output, r"^git version\b")
153196

154197
@skipUnless(
155198
os.name == "nt",
@@ -345,7 +388,7 @@ def test_environment(self, rw_dir):
345388
self.assertIn("FOO", str(err))
346389

347390
def test_handle_process_output(self):
348-
from git.cmd import handle_process_output
391+
from git.cmd import handle_process_output, safer_popen
349392

350393
line_count = 5002
351394
count = [None, 0, 0]
@@ -361,13 +404,12 @@ def counter_stderr(line):
361404
fixture_path("cat_file.py"),
362405
str(fixture_path("issue-301_stderr")),
363406
]
364-
proc = subprocess.Popen(
407+
proc = safer_popen(
365408
cmdline,
366409
stdin=None,
367410
stdout=subprocess.PIPE,
368411
stderr=subprocess.PIPE,
369412
shell=False,
370-
creationflags=cmd.PROC_CREATIONFLAGS,
371413
)
372414

373415
handle_process_output(proc, counter_stdout, counter_stderr, finalize_process)

‎test/test_index.py

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,19 @@
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+
import contextlib
67
from io import BytesIO
78
import logging
89
import os
910
import os.path as osp
1011
from pathlib import Path
1112
import re
13+
import shutil
1214
from stat import S_ISLNK, ST_MODE
1315
import subprocess
1416
import tempfile
1517

18+
import ddt
1619
import pytest
1720
from sumtypes import constructor, sumtype
1821

@@ -36,9 +39,16 @@
3639
from git.index.typ import BaseIndexEntry, IndexEntry
3740
from git.index.util import TemporaryFileSwap
3841
from git.objects import Blob
39-
from git.util import Actor, hex_to_bin, rmtree
42+
from git.util import Actor, cwd, hex_to_bin, rmtree
4043
from gitdb.base import IStream
41-
from test.lib import TestBase, fixture, fixture_path, with_rw_directory, with_rw_repo
44+
from test.lib import (
45+
TestBase,
46+
VirtualEnvironment,
47+
fixture,
48+
fixture_path,
49+
with_rw_directory,
50+
with_rw_repo,
51+
)
4252

4353
HOOKS_SHEBANG = "#!/usr/bin/env sh\n"
4454

@@ -172,6 +182,7 @@ def _make_hook(git_dir, name, content, make_exec=True):
172182
return hp
173183

174184

185+
@ddt.ddt
175186
class TestIndex(TestBase):
176187
def __init__(self, *args):
177188
super().__init__(*args)
@@ -1012,6 +1023,47 @@ def test_run_commit_hook(self, rw_repo):
10121023
output = Path(rw_repo.git_dir, "output.txt").read_text(encoding="utf-8")
10131024
self.assertEqual(output, "ran fake hook\n")
10141025

1026+
@ddt.data((False,), (True,))
1027+
@with_rw_directory
1028+
def test_hook_uses_shell_not_from_cwd(self, rw_dir, case):
1029+
(chdir_to_repo,) = case
1030+
1031+
shell_name = "bash.exe" if os.name == "nt" else "sh"
1032+
maybe_chdir = cwd(rw_dir) if chdir_to_repo else contextlib.nullcontext()
1033+
repo = Repo.init(rw_dir)
1034+
1035+
# We need an impostor shell that works on Windows and that the test can
1036+
# distinguish from the real bash.exe. But even if the real bash.exe is absent or
1037+
# unusable, we should verify the impostor is not run. So the impostor needs a
1038+
# clear side effect (unlike in TestGit.test_it_executes_git_not_from_cwd). Popen
1039+
# on Windows uses CreateProcessW, which disregards PATHEXT; the impostor may
1040+
# need to be a binary executable to ensure the vulnerability is found if
1041+
# present. No compiler need exist, shipping a binary in the test suite may
1042+
# target the wrong architecture, and generating one in a bespoke way may trigger
1043+
# false positive virus scans. So we use a Bash/Python polyglot for the hook and
1044+
# use the Python interpreter itself as the bash.exe impostor. But an interpreter
1045+
# from a venv may not run when copied outside of it, and a global interpreter
1046+
# won't run when copied to a different location if it was installed from the
1047+
# Microsoft Store. So we make a new venv in rw_dir and use its interpreter.
1048+
venv = VirtualEnvironment(rw_dir, with_pip=False)
1049+
shutil.copy(venv.python, Path(rw_dir, shell_name))
1050+
shutil.copy(fixture_path("polyglot"), hook_path("polyglot", repo.git_dir))
1051+
payload = Path(rw_dir, "payload.txt")
1052+
1053+
if type(_win_bash_status) in {WinBashStatus.Absent, WinBashStatus.WslNoDistro}:
1054+
# The real shell can't run, but the impostor should still not be used.
1055+
with self.assertRaises(HookExecutionError):
1056+
with maybe_chdir:
1057+
run_commit_hook("polyglot", repo.index)
1058+
self.assertFalse(payload.exists())
1059+
else:
1060+
# The real shell should run, and not the impostor.
1061+
with maybe_chdir:
1062+
run_commit_hook("polyglot", repo.index)
1063+
self.assertFalse(payload.exists())
1064+
output = Path(rw_dir, "output.txt").read_text(encoding="utf-8")
1065+
self.assertEqual(output, "Ran intended hook.\n")
1066+
10151067
@pytest.mark.xfail(
10161068
type(_win_bash_status) is WinBashStatus.Absent,
10171069
reason="Can't run a hook on Windows without bash.exe.",

‎test/test_installation.py

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,19 @@
44
import ast
55
import os
66
import subprocess
7-
import sys
87

9-
from test.lib import TestBase
10-
from test.lib.helper import with_rw_directory
8+
from test.lib import TestBase, VirtualEnvironment, with_rw_directory
119

1210

1311
class TestInstallation(TestBase):
14-
def setUp_venv(self, rw_dir):
15-
self.venv = rw_dir
16-
subprocess.run([sys.executable, "-m", "venv", self.venv], stdout=subprocess.PIPE)
17-
bin_name = "Scripts" if os.name == "nt" else "bin"
18-
self.python = os.path.join(self.venv, bin_name, "python")
19-
self.pip = os.path.join(self.venv, bin_name, "pip")
20-
self.sources = os.path.join(self.venv, "src")
21-
self.cwd = os.path.dirname(os.path.dirname(__file__))
22-
os.symlink(self.cwd, self.sources, target_is_directory=True)
23-
2412
@with_rw_directory
2513
def test_installation(self, rw_dir):
26-
self.setUp_venv(rw_dir)
14+
venv = self._set_up_venv(rw_dir)
2715

2816
result = subprocess.run(
29-
[self.pip, "install", "."],
17+
[venv.pip, "install", "."],
3018
stdout=subprocess.PIPE,
31-
cwd=self.sources,
19+
cwd=venv.sources,
3220
)
3321
self.assertEqual(
3422
0,
@@ -37,9 +25,9 @@ def test_installation(self, rw_dir):
3725
)
3826

3927
result = subprocess.run(
40-
[self.python, "-c", "import git"],
28+
[venv.python, "-c", "import git"],
4129
stdout=subprocess.PIPE,
42-
cwd=self.sources,
30+
cwd=venv.sources,
4331
)
4432
self.assertEqual(
4533
0,
@@ -48,9 +36,9 @@ def test_installation(self, rw_dir):
4836
)
4937

5038
result = subprocess.run(
51-
[self.python, "-c", "import gitdb; import smmap"],
39+
[venv.python, "-c", "import gitdb; import smmap"],
5240
stdout=subprocess.PIPE,
53-
cwd=self.sources,
41+
cwd=venv.sources,
5442
)
5543
self.assertEqual(
5644
0,
@@ -62,9 +50,9 @@ def test_installation(self, rw_dir):
6250
# by inserting its location into PYTHONPATH or otherwise patched into
6351
# sys.path, make sure it is not wrongly inserted as the *first* entry.
6452
result = subprocess.run(
65-
[self.python, "-c", "import sys; import git; print(sys.path)"],
53+
[venv.python, "-c", "import sys; import git; print(sys.path)"],
6654
stdout=subprocess.PIPE,
67-
cwd=self.sources,
55+
cwd=venv.sources,
6856
)
6957
syspath = result.stdout.decode("utf-8").splitlines()[0]
7058
syspath = ast.literal_eval(syspath)
@@ -73,3 +61,13 @@ def test_installation(self, rw_dir):
7361
syspath[0],
7462
msg="Failed to follow the conventions for https://docs.python.org/3/library/sys.html#sys.path",
7563
)
64+
65+
@staticmethod
66+
def _set_up_venv(rw_dir):
67+
venv = VirtualEnvironment(rw_dir, with_pip=True)
68+
os.symlink(
69+
os.path.dirname(os.path.dirname(__file__)),
70+
venv.sources,
71+
target_is_directory=True,
72+
)
73+
return venv

0 commit comments

Comments
 (0)
Please sign in to comment.