Review comments

Author: chrisburr

src/setuptools_scm/git.py

@@ -5,7 +5,6 @@
 from os.path import isfile
 from os.path import join
 from os.path import samefile
-from shlex import quote
 
 from .config import Configuration
 from .scm_workdir import Workdir
@@ -39,8 +38,7 @@ def from_potential_worktree(cls, wd):
         wd = os.path.abspath(wd)
         git_dir = join(wd, ".git")
         real_wd, _, ret = do_ex(
-            f"git --git-dir={quote(git_dir)} rev-parse --show-prefix",
-            wd,
+            ["git", "--git-dir", git_dir, "rev-parse", "--show-prefix"], wd
         )
         real_wd = real_wd[:-1]  # remove the trailing pathsep
         if ret:
@@ -59,35 +57,26 @@ def from_potential_worktree(cls, wd):
 
         return cls(real_wd)
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self._git_dir = join(self.path, ".git")
+    def do_ex_git(self, cmd):
+        return self.do_ex(["git", "--git-dir", join(self.path, ".git")] + cmd)
 
     def is_dirty(self):
-        out, _, _ = self.do_ex(
-            f"git --git-dir={quote(self._git_dir)} "
-            "status --porcelain --untracked-files=no"
-        )
+        out, _, _ = self.do_ex_git(["status", "--porcelain", "--untracked-files=no"])
         return bool(out)
 
     def get_branch(self):
-        branch, err, ret = self.do_ex(
-            f"git --git-dir={quote(self._git_dir)} rev-parse --abbrev-ref HEAD"
-        )
+        branch, err, ret = self.do_ex_git(["rev-parse", "--abbrev-ref", "HEAD"])
         if ret:
             trace("branch err", branch, err, ret)
-            branch, err, ret = self.do_ex(
-                f"git --git-dir={quote(self._git_dir)} symbolic-ref --short HEAD"
-            )
+            branch, err, ret = self.do_ex_git(["symbolic-ref", "--short", "HEAD"])
             if ret:
                 trace("branch err (symbolic-ref)", branch, err, ret)
                 branch = None
         return branch
 
     def get_head_date(self):
-        timestamp, err, ret = self.do_ex(
-            f"git --git-dir={quote(self._git_dir)} -c log.showSignature=false "
-            "log -n 1 HEAD --format=%cI"
+        timestamp, err, ret = self.do_ex_git(
+            ["-c", "log.showSignature=false", "log", "-n", "1", "HEAD", "--format=%cI"]
         )
         if ret:
             trace("timestamp err", timestamp, err, ret)
@@ -100,25 +89,24 @@ def get_head_date(self):
         return datetime.strptime(date_part, r"%Y-%m-%d").date()
 
     def is_shallow(self):
-        return isfile(join(self._git_dir, "shallow"))
+        return isfile(join(self.path, ".git/shallow"))
 
     def fetch_shallow(self):
-        self.do_ex(f"git --git-dir={quote(self._git_dir)} fetch --unshallow")
+        self.do_ex_git(["fetch", "--unshallow"])
 
     def node(self):
-        node, _, ret = self.do_ex(
-            f"git --git-dir={quote(self._git_dir)} rev-parse --verify --quiet HEAD"
-        )
+        node, _, ret = self.do_ex_git(["rev-parse", "--verify", "--quiet", "HEAD"])
         if not ret:
             return node[:7]
 
     def count_all_nodes(self):
-        revs, _, _ = self.do_ex(f"git --git-dir={quote(self._git_dir)} rev-list HEAD")
+        revs, _, _ = self.do_ex_git(["rev-list", "HEAD"])
         return revs.count("\n") + 1
 
     def default_describe(self):
+        git_dir = join(self.path, ".git")
         return self.do_ex(
-            DEFAULT_DESCRIBE[:1] + ["--git-dir", self._git_dir] + DEFAULT_DESCRIBE[1:]
+            DEFAULT_DESCRIBE[:1] + ["--git-dir", git_dir] + DEFAULT_DESCRIBE[1:]
         )
 
 

testing/test_git.py

@@ -95,6 +95,7 @@ def test_parse_call_order(wd):
 
 
 @pytest.mark.issue("https://github.com/pypa/setuptools_scm/issues/707")
+@pytest.mark.xfail(run=False, reason="This test requires passwordless sudo")
 def test_not_owner(wd):
     git_dir = opj(wd.cwd)
     original_stat = os.stat(git_dir)