From ba132f02007f6d185a33a80c2d537f4d29335c04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 11 Jul 2021 07:59:33 +0200 Subject: [PATCH] Split get_git_dir() into get_git_dir() and get_git_common_dir() This fixes the conflicted state check when using work trees. #1972 --- pre_commit/commands/install_uninstall.py | 2 +- pre_commit/git.py | 26 ++++++++++++++------ tests/git_test.py | 30 ++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index cb2aaa5bf..b1fd8d490 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 6fff8d2a9..35392b341 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() diff --git a/tests/git_test.py b/tests/git_test.py index d9e497c5b..b9f524a14 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))