Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
This fixes the path search bug where the current directory is
included on Windows, by setting NoDefaultCurrentDirectoryInExePath
for the caller. (Setting for the callee env would not work.)

This sets it only on Windows, only for the duration of the Popen
call, and then automatically unsets it or restores its old value.

NoDefaultCurrentDirectoryInExePath is documented at:
https://learn.microsoft.com/en-us/windows/win32/api/processenv/nf-processenv-needcurrentdirectoryforexepathw

It automatically affects the behavior of subprocess.Popen on
Windows, due to the way Popen uses the Windows API. (In contrast,
it does not, at least currently on CPython, affect the behavior of
shutil.which. But shutil.which is not being used to find git.exe.)
  • Loading branch information
EliahKagan committed Aug 30, 2023
1 parent e19abe7 commit 6029211
Showing 1 changed file with 21 additions and 17 deletions.
38 changes: 21 additions & 17 deletions git/cmd.py
Expand Up @@ -5,7 +5,7 @@
# the BSD License: http://www.opensource.org/licenses/bsd-license.php
from __future__ import annotations
import re
from contextlib import contextmanager
import contextlib
import io
import logging
import os
Expand All @@ -14,6 +14,7 @@
import subprocess
import threading
from textwrap import dedent
import unittest.mock

from git.compat import (
defenc,
Expand Down Expand Up @@ -963,8 +964,11 @@ def execute(
redacted_command,
'"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"})
else:
cmd_not_found_exception = FileNotFoundError # NOQA # exists, flake8 unknown @UndefinedVariable
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 @@ -980,21 +984,21 @@ def execute(
istream_ok,
)
try:
proc = Popen(
command,
env=env,
cwd=cwd,
bufsize=-1,
stdin=istream or DEVNULL,
stderr=PIPE,
stdout=stdout_sink,
shell=shell is not None and shell or self.USE_SHELL,
close_fds=is_posix, # unsupported on windows
universal_newlines=universal_newlines,
creationflags=PROC_CREATIONFLAGS,
**subprocess_kwargs,
)

with patch_caller_env:
proc = Popen(
command,
env=env,
cwd=cwd,
bufsize=-1,
stdin=istream or DEVNULL,
stderr=PIPE,
stdout=stdout_sink,
shell=shell is not None and shell or self.USE_SHELL,
close_fds=is_posix, # unsupported on windows
universal_newlines=universal_newlines,
creationflags=PROC_CREATIONFLAGS,
**subprocess_kwargs,
)
except cmd_not_found_exception as err:
raise GitCommandNotFound(redacted_command, err) from err
else:
Expand Down Expand Up @@ -1144,7 +1148,7 @@ def update_environment(self, **kwargs: Any) -> Dict[str, Union[str, None]]:
del self._environment[key]
return old_env

@contextmanager
@contextlib.contextmanager
def custom_environment(self, **kwargs: Any) -> Iterator[None]:
"""
A context manager around the above ``update_environment`` method to restore the
Expand Down

0 comments on commit 6029211

Please sign in to comment.