From 451e042835f7776de52a475e5dcf82395d4d1123 Mon Sep 17 00:00:00 2001 From: Noah Pendleton Date: Thu, 5 Aug 2021 13:02:19 -0400 Subject: [PATCH] Permit installing to core.hooksPath if within repo ### Summary There's [several threads](https://github.com/pre-commit/pre-commit/issues?q=is%3Aissue+core+hooksPath+is%3Aclosed) around using pre-commit with global git hooks directories, but I didn't see anything around setting `core.hooksPath` to a directory within the repo, as the `husky` tool does. This patch adds support for installing/uninstalling pre-commit when `core.hooksPath` references a path within the git repo. ### Testing - add a couple of simple tests - a repo with core.hooksPath set within the repo, install/uninstall does the right thing - a repo with core.hooksPath set to `../hooks`, install/uninstall fails - a repo with core.hooksPath unset, install/uninstall works as before --- pre_commit/commands/install_uninstall.py | 38 +++++++++++++++++++++--- pre_commit/git.py | 8 +++-- tests/commands/install_uninstall_test.py | 13 ++++++-- 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 73c8d605..8268e51f 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -3,6 +3,7 @@ import logging import os.path import shutil import sys +from pathlib import Path from typing import Optional from typing import Sequence from typing import Tuple @@ -36,12 +37,36 @@ POSIX_SEARCH_PATH = ('/usr/local/bin', '/usr/bin', '/bin') SYS_EXE = os.path.basename(os.path.realpath(sys.executable)) +def _hooks_dir(git_dir: Optional[str] = None) -> str: + if git_dir is not None: + return os.path.join(git_dir, 'hooks') + + # prefer core.hooksPath config setting + pth = git.get_core_hookspath() + if pth: + return pth + else: + git_dir = git.get_git_dir() + return os.path.join(git_dir, 'hooks') + + +def _is_hooksPath_in_repo() -> bool: + if not git.has_core_hookpaths_set(): + return True + + gitroot = Path(git.get_root()) + pth = Path(git.get_core_hookspath()) + if not pth.is_absolute(): + pth = Path(gitroot / pth.resolve()) + + return gitroot in pth.parents + + 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() - pth = os.path.join(git_dir, 'hooks', hook_type) + pth = os.path.join(_hooks_dir(git_dir), hook_type) return pth, f'{pth}.legacy' @@ -128,9 +153,10 @@ def install( skip_on_missing_config: bool = False, git_dir: Optional[str] = None, ) -> int: - if git_dir is None and git.has_core_hookpaths_set(): + if git_dir is None and not _is_hooksPath_in_repo(): logger.error( - 'Cowardly refusing to install hooks with `core.hooksPath` set.\n' + 'Cowardly refusing to install hooks with `core.hooksPath` set to a' + f" location outside the repo: '{git.get_core_hookspath()}'\n" 'hint: `git config --unset-all core.hooksPath`', ) return 1 @@ -155,6 +181,10 @@ def install_hooks(config_file: str, store: Store) -> int: def _uninstall_hook_script(hook_type: str) -> None: + # don't bother if hooksPath is outside the repo + if not _is_hooksPath_in_repo(): + return + hook_path, legacy_path = _hook_paths(hook_type) # If our file doesn't exist or it isn't ours, gtfo. diff --git a/pre_commit/git.py b/pre_commit/git.py index 4bf28235..888abb67 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -173,9 +173,13 @@ def has_diff(*args: str, repo: str = '.') -> bool: return cmd_output_b(*cmd, cwd=repo, retcode=None)[0] == 1 -def has_core_hookpaths_set() -> bool: +def get_core_hookspath() -> str: _, out, _ = cmd_output_b('git', 'config', 'core.hooksPath', retcode=None) - return bool(out.strip()) + return out.strip().decode() + + +def has_core_hookpaths_set() -> bool: + return bool(get_core_hookspath()) def init_repo(path: str, remote: str) -> None: diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 314b8b96..ed511c95 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -105,9 +105,18 @@ def test_install_multiple_hooks_at_once(in_git_dir, store): assert not in_git_dir.join('.git/hooks/pre-push').exists() -def test_install_refuses_core_hookspath(in_git_dir, store): - cmd_output('git', 'config', '--local', 'core.hooksPath', 'hooks') +def test_install_refuses_core_hookspath_outside_repo(in_git_dir, store): + cmd_output('git', 'config', '--local', 'core.hooksPath', '../hooks') assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) + cmd_output('git', 'config', '--local', 'core.hooksPath', '/tmp/hooks') + assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) + + +def test_install_core_hookspath_inside_repo(in_git_dir, store): + hook = in_git_dir.join('.hooks').ensure_dir().join('pre-commit') + cmd_output('git', 'config', '--local', 'core.hooksPath', '.hooks') + install(C.CONFIG_FILE, store, hook_types=['pre-commit']) + assert hook.exists() def test_install_hooks_dead_symlink(in_git_dir, store):