Skip to content

Commit

Permalink
Fix Windows env var upcasing regression
Browse files Browse the repository at this point in the history
This uses a simple hand-rolled context manager to patch the
NoDefaultCurrentDirectoryInExePath variable, instead of
unittest.mock.patch.dict. The latter set unrelated environment
variables to the original (same) values via os.environ, and as a
result, their names were all converted to upper-case on Windows.

Because only environment variables that are actually set through
os.environ have their names upcased, the only variable whose name
should be upcased now is NoDefaultCurrentDirectoryInExePath, which
should be fine (it has a single established use/meaning in Windows,
where it's treated case-insensitively as environment variables in
Windows *usually* are).
  • Loading branch information
EliahKagan committed Sep 7, 2023
1 parent 7296e5c commit c7fad20
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
9 changes: 4 additions & 5 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import subprocess
import threading
from textwrap import dedent
import unittest.mock

from git.compat import (
defenc,
Expand All @@ -24,7 +23,7 @@
is_win,
)
from git.exc import CommandError
from git.util import is_cygwin_git, cygpath, expand_path, remove_password_if_present
from git.util import is_cygwin_git, cygpath, expand_path, remove_password_if_present, patch_env

from .exc import GitCommandError, GitCommandNotFound, UnsafeOptionError, UnsafeProtocolError
from .util import (
Expand Down Expand Up @@ -965,10 +964,10 @@ def execute(
'"kill_after_timeout" feature is not supported on Windows.',
)
# Only search PATH, not CWD. This must be in the *caller* environment. The "1" can be any value.
patch_caller_env = unittest.mock.patch.dict(os.environ, {"NoDefaultCurrentDirectoryInExePath": "1"})
maybe_patch_caller_env = patch_env("NoDefaultCurrentDirectoryInExePath", "1")
else:
cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable
patch_caller_env = contextlib.nullcontext()
maybe_patch_caller_env = contextlib.nullcontext()
# end handle

stdout_sink = PIPE if with_stdout else getattr(subprocess, "DEVNULL", None) or open(os.devnull, "wb")
Expand All @@ -984,7 +983,7 @@ def execute(
istream_ok,
)
try:
with patch_caller_env:
with maybe_patch_caller_env:
proc = Popen(
command,
env=env,
Expand Down
16 changes: 15 additions & 1 deletion git/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,20 @@ def cwd(new_dir: PathLike) -> Generator[PathLike, None, None]:
os.chdir(old_dir)


@contextlib.contextmanager
def patch_env(name: str, value: str) -> Generator[None, None, None]:
"""Context manager to temporarily patch an environment variable."""
old_value = os.getenv(name)
os.environ[name] = value
try:
yield
finally:
if old_value is None:
del os.environ[name]
else:
os.environ[name] = old_value


def rmtree(path: PathLike) -> None:
"""Remove the given recursively.
Expand Down Expand Up @@ -935,7 +949,7 @@ def _obtain_lock_or_raise(self) -> None:
)

try:
with open(lock_file, mode='w'):
with open(lock_file, mode="w"):
pass
except OSError as e:
raise IOError(str(e)) from e
Expand Down

0 comments on commit c7fad20

Please sign in to comment.