Merge pull request #1650 from EliahKagan/envcase

Fix Windows environment variable upcasing bug

Author: Byron

git/cmd.py

@@ -14,7 +14,6 @@
 importsubprocess
 importthreading
 fromtextwrapimportdedent
-importunittest.mock
 
 fromgit.compatimport (
 defenc,
@@ -24,7 +23,7 @@
 is_win,
 )
 fromgit.excimportCommandError
-fromgit.utilimportis_cygwin_git, cygpath, expand_path, remove_password_if_present
+fromgit.utilimportis_cygwin_git, cygpath, expand_path, remove_password_if_present, patch_env
 
 from .excimportGitCommandError, GitCommandNotFound, UnsafeOptionError, UnsafeProtocolError
 from .utilimport (
@@ -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=PIPEifwith_stdoutelsegetattr(subprocess, "DEVNULL", None) oropen(os.devnull, "wb")
@@ -984,7 +983,7 @@ def execute(
 istream_ok,
  )
 try:
-withpatch_caller_env:
+withmaybe_patch_caller_env:
 proc=Popen(
 command,
 env=env,

git/util.py

@@ -150,6 +150,7 @@ def wrapper(self: "Remote", *args: Any, **kwargs: Any) -> T:
 
 @contextlib.contextmanager
 defcwd(new_dir: PathLike) ->Generator[PathLike, None, None]:
+"""Context manager to temporarily change directory. Not reentrant."""
 old_dir=os.getcwd()
 os.chdir(new_dir)
 try:
@@ -158,6 +159,20 @@ def cwd(new_dir: PathLike) -> Generator[PathLike, None, None]:
 os.chdir(old_dir)
 
 
+@contextlib.contextmanager
+defpatch_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:
+ifold_valueisNone:
+delos.environ[name]
+else:
+os.environ[name] =old_value
+
+
 defrmtree(path: PathLike) ->None:
 """Remove the given recursively.
 
@@ -935,7 +950,7 @@ def _obtain_lock_or_raise(self) -> None:
  )
 
 try:
-withopen(lock_file, mode='w'):
+withopen(lock_file, mode="w"):
 pass
 exceptOSErrorase:
 raiseIOError(str(e)) frome

test/fixtures/env_case.py

@@ -0,0 +1,13 @@
+importsubprocess
+importsys
+
+importgit
+
+
+_, working_dir, env_var_name=sys.argv
+
+# Importing git should be enough, but this really makes sure Git.execute is called.
+repo=git.Repo(working_dir) # Hold the reference.
+git.Git(repo.working_dir).execute(["git", "version"])
+
+print(subprocess.check_output(["set", env_var_name], shell=True, text=True))

test/test_git.py

@@ -4,35 +4,23 @@
 #
 # This module is part of GitPython and is released under
 # the BSD License: http://www.opensource.org/licenses/bsd-license.php
-importcontextlib
 importos
 importshutil
 importsubprocess
 importsys
 fromtempfileimportTemporaryDirectory, TemporaryFile
-fromunittestimportmock
+fromunittestimportmock, skipUnless
 
 fromgitimportGit, refresh, GitCommandError, GitCommandNotFound, Repo, cmd
 fromtest.libimportTestBase, fixture_path
 fromtest.libimportwith_rw_directory
-fromgit.utilimportfinalize_process
+fromgit.utilimportcwd, finalize_process
 
 importos.pathasosp
 
 fromgit.compatimportis_win
 
 
-@contextlib.contextmanager
-def_chdir(new_dir):
-"""Context manager to temporarily change directory. Not reentrant."""
-old_dir=os.getcwd()
-os.chdir(new_dir)
-try:
-yield
-finally:
-os.chdir(old_dir)
-
-
 classTestGit(TestBase):
 @classmethod
 defsetUpClass(cls):
@@ -102,9 +90,26 @@ def test_it_executes_git_not_from_cwd(self):
 print("#!/bin/sh", file=file)
 os.chmod(impostor_path, 0o755)
 
-with_chdir(tmpdir):
+withcwd(tmpdir):
 self.assertRegex(self.git.execute(["git", "version"]), r"^git version\b")
 
+@skipUnless(is_win, "The regression only affected Windows, and this test logic is OS-specific.")
+deftest_it_avoids_upcasing_unrelated_environment_variable_names(self):
+old_name="28f425ca_d5d8_4257_b013_8d63166c8158"
+ifold_name==old_name.upper():
+raiseRuntimeError("test bug or strange locale: old_name invariant under upcasing")
+os.putenv(old_name, "1") # It has to be done this lower-level way to set it lower-case.
+
+cmdline= [
+sys.executable,
+fixture_path("env_case.py"),
+self.rorepo.working_dir,
+old_name,
+ ]
+pair_text=subprocess.check_output(cmdline, shell=False, text=True)
+new_name=pair_text.split("=")[0]
+self.assertEqual(new_name, old_name)
+
 deftest_it_accepts_stdin(self):
 filename=fixture_path("cat_file_blob")
 withopen(filename, "r") asfh: