Merge pull request #1812 from EliahKagan/refresh-cmdline

Report actual attempted Git command when Git.refresh fails

Author: Byron

Diff for: git/cmd.py

@@ -478,12 +478,12 @@ def refresh(cls, path: Union[None, PathLike] = None) -> bool:
                 # We get here if this was the initial refresh and the refresh mode was
                 # not error. Go ahead and set the GIT_PYTHON_GIT_EXECUTABLE such that we
                 # discern the difference between the first refresh at import time
-                # and subsequent calls to refresh().
+                # and subsequent calls to git.refresh or this refresh method.
                 cls.GIT_PYTHON_GIT_EXECUTABLE = cls.git_exec_name
             else:
                 # After the first refresh (when GIT_PYTHON_GIT_EXECUTABLE is no longer
                 # None) we raise an exception.
-                raise GitCommandNotFound("git", err)
+                raise GitCommandNotFound(new_git, err)
 
         return has_git
 

Diff for: git/remote.py

@@ -5,30 +5,24 @@
 
 """Module implementing a remote object allowing easy access to git remotes."""
 
+import contextlib
 import logging
 import re
 
-from git.cmd import handle_process_output, Git
+from git.cmd import Git, handle_process_output
 from git.compat import defenc, force_text
+from git.config import GitConfigParser, SectionConstraint, cp
 from git.exc import GitCommandError
+from git.refs import Head, Reference, RemoteReference, SymbolicReference, TagReference
 from git.util import (
-    LazyMixin,
-    IterableObj,
+    CallableRemoteProgress,
     IterableList,
+    IterableObj,
+    LazyMixin,
     RemoteProgress,
-    CallableRemoteProgress,
-)
-from git.util import (
     join_path,
 )
 
-from git.config import (
-    GitConfigParser,
-    SectionConstraint,
-    cp,
-)
-from git.refs import Head, Reference, RemoteReference, SymbolicReference, TagReference
-
 # typing-------------------------------------------------------
 
 from typing import (
@@ -345,18 +339,13 @@ class FetchInfo(IterableObj):
     @classmethod
     def refresh(cls) -> Literal[True]:
         """This gets called by the refresh function (see the top level __init__)."""
-        # clear the old values in _flag_map
-        try:
+        # Clear the old values in _flag_map.
+        with contextlib.suppress(KeyError):
             del cls._flag_map["t"]
-        except KeyError:
-            pass
-
-        try:
+        with contextlib.suppress(KeyError):
             del cls._flag_map["-"]
-        except KeyError:
-            pass
 
-        # set the value given the git version
+        # Set the value given the git version.
         if Git().version_info[:2] >= (2, 10):
             cls._flag_map["t"] = cls.TAG_UPDATE
         else:

Diff for: test/test_git.py

@@ -44,6 +44,14 @@ def _patch_out_env(name):
             os.environ[name] = old_value
 
 
+@contextlib.contextmanager
+def _rollback_refresh():
+    try:
+        yield Git.GIT_PYTHON_GIT_EXECUTABLE  # Provide the old value for convenience.
+    finally:
+        refresh()
+
+
 @ddt.ddt
 class TestGit(TestBase):
     @classmethod
@@ -306,14 +314,43 @@ def test_cmd_override(self):
         ):
             self.assertRaises(GitCommandNotFound, self.git.version)
 
-    def test_refresh(self):
-        # Test a bad git path refresh.
-        self.assertRaises(GitCommandNotFound, refresh, "yada")
-
-        # Test a good path refresh.
-        which_cmd = "where" if os.name == "nt" else "command -v"
-        path = os.popen("{0} git".format(which_cmd)).read().strip().split("\n")[0]
-        refresh(path)
+    def test_refresh_bad_absolute_git_path(self):
+        """Bad absolute path arg is reported and not set."""
+        absolute_path = str(Path("yada").absolute())
+        expected_pattern = rf"\n[ \t]*cmdline: {re.escape(absolute_path)}\Z"
+
+        with _rollback_refresh() as old_git_executable:
+            with self.assertRaisesRegex(GitCommandNotFound, expected_pattern):
+                refresh(absolute_path)
+            self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable)
+
+    def test_refresh_bad_relative_git_path(self):
+        """Bad relative path arg is resolved to absolute path and reported, not set."""
+        absolute_path = str(Path("yada").absolute())
+        expected_pattern = rf"\n[ \t]*cmdline: {re.escape(absolute_path)}\Z"
+
+        with _rollback_refresh() as old_git_executable:
+            with self.assertRaisesRegex(GitCommandNotFound, expected_pattern):
+                refresh("yada")
+            self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, old_git_executable)
+
+    def test_refresh_good_absolute_git_path(self):
+        """Good absolute path arg is set."""
+        absolute_path = shutil.which("git")
+
+        with _rollback_refresh():
+            refresh(absolute_path)
+            self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, absolute_path)
+
+    def test_refresh_good_relative_git_path(self):
+        """Good relative path arg is resolved to absolute path and set."""
+        absolute_path = shutil.which("git")
+        dirname, basename = osp.split(absolute_path)
+
+        with cwd(dirname):
+            with _rollback_refresh():
+                refresh(basename)
+                self.assertEqual(self.git.GIT_PYTHON_GIT_EXECUTABLE, absolute_path)
 
     def test_options_are_passed_to_git(self):
         # This works because any command after git --version is ignored.