Merge pull request #195 from akaihola/relative-GIT_DIR

Improve Git robustness

Author: akaihola

src/darker/__main__.py

@@ -349,7 +349,7 @@ def main(argv: List[str] = None) -> int:
                 " Either --diff or --check must be used.",
             )
 
-    missing = get_missing_at_revision(paths, revrange.rev2)
+    missing = get_missing_at_revision(paths, revrange.rev2, root)
     if missing:
         missing_reprs = " ".join(repr(str(path)) for path in missing)
         rev2_repr = "the working tree" if revrange.rev2 == WORKTREE else revrange.rev2

src/darker/git.py

@@ -77,7 +77,6 @@ def git_get_content_at_revision(path: Path, revision: str, cwd: Path) -> TextDoc
         abspath = cwd / path
         return TextDocument.from_file(abspath)
     cmd = ["show", f"{revision}:./{path.as_posix()}"]
-    logger.debug("[%s]$ %s", cwd, " ".join(cmd))
     try:
         return TextDocument.from_lines(
             _git_check_output_lines(cmd, cwd, exit_on_error=False),
@@ -177,7 +176,7 @@ def _git_check_output_lines(
     cmd: List[str], cwd: Path, exit_on_error: bool = True
 ) -> List[str]:
     """Log command line, run Git, split stdout to lines, exit with 123 on error"""
-    logger.debug("[%s]$ %s", cwd, " ".join(cmd))
+    logger.debug("[%s]$ git %s", cwd, " ".join(cmd))
     try:
         return check_output(
             ["git"] + cmd,
@@ -200,36 +199,39 @@ def _git_check_output_lines(
         sys.exit(123)
 
 
-def _git_exists_in_revision(path: Path, rev2: str) -> bool:
+def _git_exists_in_revision(path: Path, rev2: str, cwd: Path) -> bool:
     """Return ``True`` if the given path exists in the given Git revision
 
     :param path: The path of the file or directory to check
     :param rev2: The Git revision to look at
+    :param cwd: The Git repository root
     :return: ``True`` if the file or directory exists at the revision, or ``False`` if
              it doesn't.
 
     """
     # Surprise: On Windows, `git cat-file` doesn't work with backslash directory
     # separators in paths. We need to use Posix paths and forward slashes instead.
     cmd = ["git", "cat-file", "-e", f"{rev2}:{path.as_posix()}"]
-    result = run(cmd, check=False, stderr=DEVNULL, env={"LC_ALL": "C"})
+    logger.debug("[%s]$ %s", cwd, " ".join(cmd))
+    result = run(cmd, cwd=str(cwd), check=False, stderr=DEVNULL, env={"LC_ALL": "C"})
     return result.returncode == 0
 
 
-def get_missing_at_revision(paths: Iterable[Path], rev2: str) -> Set[Path]:
+def get_missing_at_revision(paths: Iterable[Path], rev2: str, cwd: Path) -> Set[Path]:
     """Return paths missing in the given revision
 
     In case of ``WORKTREE``, just check if the files exist on the filesystem instead of
     asking Git.
 
     :param paths: Paths to check
     :param rev2: The Git revision to look at, or ``WORKTREE`` for the working tree
+    :param cwd: The Git repository root
     :return: The set of file or directory paths which are missing in the revision
 
     """
     if rev2 == WORKTREE:
         return {path for path in paths if not path.exists()}
-    return {path for path in paths if not _git_exists_in_revision(path, rev2)}
+    return {path for path in paths if not _git_exists_in_revision(path, rev2, cwd)}
 
 
 def _git_diff_name_only(

src/darker/tests/conftest.py

@@ -1,6 +1,5 @@
 """Configuration and fixtures for the Pytest based test suite"""
 
-import os
 from pathlib import Path
 from subprocess import check_call
 from typing import Dict, Optional
@@ -19,9 +18,10 @@ def __init__(self, root: Path, env: Dict[str, str]):
     @classmethod
     def create_repository(cls, root: Path) -> "GitRepoFixture":
         """Fixture method for creating a Git repository in the given directory"""
-        env = os.environ.copy()
-        # for testing, ignore ~/.gitconfig settings like templateDir and defaultBranch
-        env["HOME"] = str(root)
+        # For testing, ignore ~/.gitconfig settings like templateDir and defaultBranch.
+        # Also, this makes sure GIT_DIR or other GIT_* variables are not set, and that
+        # Git's messages are in English.
+        env = {"HOME": str(root), "LC_ALL": "C"}
         instance = cls(root, env)
         # pylint: disable=protected-access
         instance._run("init")
@@ -87,6 +87,10 @@ def git_repo(tmp_path, monkeypatch):
     """Create a temporary Git repository and change current working directory into it"""
     repository = GitRepoFixture.create_repository(tmp_path)
     monkeypatch.chdir(tmp_path)
+    # While `GitRepoFixture.create_repository()` already deletes `GIT_*` environment
+    # variables for any Git commands run by the fixture, let's explicitly remove
+    # `GIT_DIR` in case a test should call Git directly:
+    monkeypatch.delenv("GIT_DIR", raising=False)
     return repository
 
 

src/darker/tests/test_git.py

@@ -396,10 +396,11 @@ def test_git_exists_in_revision_git_call(retval, expect):
     with patch.object(git, "run") as run:
         run.return_value.returncode = retval
 
-        result = git._git_exists_in_revision(Path("path.py"), "rev2")
+        result = git._git_exists_in_revision(Path("path.py"), "rev2", Path("."))
 
     run.assert_called_once_with(
         ["git", "cat-file", "-e", "rev2:path.py"],
+        cwd=".",
         check=False,
         stderr=DEVNULL,
         env={"LC_ALL": "C"},
@@ -429,7 +430,9 @@ def test_git_exists_in_revision(git_repo, rev2, path, expect):
     del_a = git_repo.get_hash()
     git_repo.add({"dir/b.py": None}, commit="Delete dir/b.py")
 
-    result = git._git_exists_in_revision(Path(path), rev2.format(add=add, del_a=del_a))
+    result = git._git_exists_in_revision(
+        Path(path), rev2.format(add=add, del_a=del_a), git_repo.root
+    )
 
     assert result == expect
 
@@ -450,6 +453,7 @@ def test_get_missing_at_revision(git_repo, rev2, expect):
     result = git.get_missing_at_revision(
         {Path("dir"), Path("dir/a.py"), Path("dir/b.py")},
         rev2.format(add=add, del_a=del_a),
+        git_repo.root,
     )
 
     assert result == expect
@@ -462,7 +466,7 @@ def test_get_missing_at_revision_worktree(git_repo):
     paths["dir/b.py"].unlink()
 
     result = git.get_missing_at_revision(
-        {Path("dir"), Path("dir/a.py"), Path("dir/b.py")}, git.WORKTREE
+        {Path("dir"), Path("dir/a.py"), Path("dir/b.py")}, git.WORKTREE, git_repo.root
     )
 
     assert result == {Path("dir/a.py"), Path("dir/b.py")}