From eba15d128b3765f044be2a2308f108f7b11985bc 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 1/4] 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 | 23 +++++++++++++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 73c8d6056..6f7aa4a2e 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -40,7 +40,7 @@ def _hook_paths( hook_type: str, git_dir: Optional[str] = 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 4bf282357..bbdcd38db 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -55,7 +55,7 @@ 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()) + git_dir = os.path.abspath(get_git_common_dir()) except CalledProcessError: raise FatalError( 'git failed. Is it installed, and are you in a Git repository ' @@ -70,15 +70,26 @@ 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 branch (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() From cc9f8f66c8e675bf5955a6de7ce824955d5c8886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Fri, 6 Aug 2021 13:23:21 +0200 Subject: [PATCH 2/4] Add a case test for get_git_dir() and get_git_common_dir() --- tests/git_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/git_test.py b/tests/git_test.py index 51d5f8c43..0dfadfa97 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -38,6 +38,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)) From c43698e942fa9f0be2b55613c48d1920399cb2c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Fri, 6 Aug 2021 22:48:11 +0200 Subject: [PATCH 3/4] fix get_root() for the case of called from the git dir --- pre_commit/git.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pre_commit/git.py b/pre_commit/git.py index bbdcd38db..2b048a32c 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -55,13 +55,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_common_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.', From 0d9f2d146615bd51970121c62aa39aa50dcf1c59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Fri, 6 Aug 2021 23:23:44 +0200 Subject: [PATCH 4/4] Added a test for failing get_root_dir inside the git folder --- tests/git_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/git_test.py b/tests/git_test.py index 0dfadfa97..7cd1c549a 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -19,6 +19,13 @@ 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_in_exactly_dot_git(in_git_dir): with in_git_dir.join('.git').as_cwd(), pytest.raises(FatalError): git.get_root()