Repository: pre-commit/pre-commit
Pull request #2252 (2022-02-17 22:25:18)

Use the correct .git path in case of worktrees. 
The issue happens because `get_git_dir()` returns `git rev-parse --git-common-dir` and not `git rev-parse --git-dir` as expected. This PR introduces `get_git_common_dir()` and uses it where the common dir is required and the local .git dir of the current working copy in all other cases.   

This fixes https://github.com/pre-commit/pre-commit/issues/1972, false positives in case of worktrees. 

This PR is in the draft state to debug CI issues. 

daschuer: All green now!

Patch:
diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py
index cb2aaa5..b1fd8d4 100644
--- a/pre_commit/commands/install_uninstall.py
+++ b/pre_commit/commands/install_uninstall.py
@@ -36,7 +36,7 @@ def _hook_paths(
         hook_type: str,
         git_dir: str | None = None,
 ) -> tuple[str, str]:
-    git_dir = git_dir if git_dir is not None else git.get_git_dir()
+    git_dir = git_dir if git_dir is not None else git.get_git_common_dir()
     pth = os.path.join(git_dir, 'hooks', hook_type)
     return pth, f'{pth}.legacy'
 
diff --git a/pre_commit/git.py b/pre_commit/git.py
index 6fff8d2..35392b3 100644
--- a/pre_commit/git.py
+++ b/pre_commit/git.py
@@ -57,13 +57,15 @@ def get_root() -> str:
         root = os.path.abspath(
             cmd_output('git', 'rev-parse', '--show-cdup')[1].strip(),
         )
-        git_dir = os.path.abspath(get_git_dir())
+        inside_git_dir = cmd_output(
+            'git', 'rev-parse', '--is-inside-git-dir',
+        )[1].strip()
     except CalledProcessError:
         raise FatalError(
             'git failed. Is it installed, and are you in a Git repository '
             'directory?',
         )
-    if os.path.samefile(root, git_dir):
+    if inside_git_dir != 'false':
         raise FatalError(
             'git toplevel unexpectedly empty! make sure you are not '
             'inside the `.git` directory of your repository.',
@@ -72,15 +74,25 @@ def get_root() -> str:
 
 
 def get_git_dir(git_root: str = '.') -> str:
-    opts = ('--git-common-dir', '--git-dir')
-    _, out, _ = cmd_output('git', 'rev-parse', *opts, cwd=git_root)
-    for line, opt in zip(out.splitlines(), opts):
-        if line != opt:  # pragma: no branch (git < 2.5)
-            return os.path.normpath(os.path.join(git_root, line))
+    opt = '--git-dir'
+    _, out, _ = cmd_output('git', 'rev-parse', opt, cwd=git_root)
+    git_dir = out.strip()
+    if git_dir != opt:
+        return os.path.normpath(os.path.join(git_root, git_dir))
     else:
         raise AssertionError('unreachable: no git dir')
 
 
+def get_git_common_dir(git_root: str = '.') -> str:
+    opt = '--git-common-dir'
+    _, out, _ = cmd_output('git', 'rev-parse', opt, cwd=git_root)
+    git_common_dir = out.strip()
+    if git_common_dir != opt:
+        return os.path.normpath(os.path.join(git_root, git_common_dir))
+    else:  # pragma: no cover (git < 2.5)
+        return get_git_dir(git_root)
+
+
 def get_remote_url(git_root: str) -> str:
     _, out, _ = cmd_output('git', 'config', 'remote.origin.url', cwd=git_root)
     return out.strip()


Test patch:
diff --git a/tests/git_test.py b/tests/git_test.py
index d9e497c..b9f524a 100644
--- a/tests/git_test.py
+++ b/tests/git_test.py
@@ -21,6 +21,20 @@ def test_get_root_deeper(in_git_dir):
         assert os.path.normcase(git.get_root()) == expected
 
 
+def test_get_root_in_git_sub_dir(in_git_dir):
+    expected = os.path.normcase(in_git_dir.strpath)
+    with pytest.raises(FatalError):
+        with in_git_dir.join('.git/objects').ensure_dir().as_cwd():
+            assert os.path.normcase(git.get_root()) == expected
+
+
+def test_get_root_not_in_working_dir(in_git_dir):
+    expected = os.path.normcase(in_git_dir.strpath)
+    with pytest.raises(FatalError):
+        with in_git_dir.join('..').ensure_dir().as_cwd():
+            assert os.path.normcase(git.get_root()) == expected
+
+
 def test_in_exactly_dot_git(in_git_dir):
     with in_git_dir.join('.git').as_cwd(), pytest.raises(FatalError):
         git.get_root()
@@ -40,6 +54,22 @@ def test_get_root_bare_worktree(tmpdir):
         assert git.get_root() == os.path.abspath('.')
 
 
+def test_get_git_dir(tmpdir):
+    """Regression test for #1972"""
+    src = tmpdir.join('src').ensure_dir()
+    cmd_output('git', 'init', str(src))
+    git_commit(cwd=str(src))
+
+    worktree = tmpdir.join('worktree').ensure_dir()
+    cmd_output('git', 'worktree', 'add', '../worktree', cwd=src)
+
+    with worktree.as_cwd():
+        assert git.get_git_dir() == src.ensure_dir(
+            '.git/worktrees/worktree',
+        )
+        assert git.get_git_common_dir() == src.ensure_dir('.git')
+
+
 def test_get_root_worktree_in_git(tmpdir):
     src = tmpdir.join('src').ensure_dir()
     cmd_output('git', 'init', str(src))