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
This commit is contained in:
Noah Pendleton 2021-08-05 13:02:19 -04:00
parent 6cfdabb69a
commit 451e042835
3 changed files with 51 additions and 8 deletions

View file

@ -3,6 +3,7 @@ import logging
import os.path import os.path
import shutil import shutil
import sys import sys
from pathlib import Path
from typing import Optional from typing import Optional
from typing import Sequence from typing import Sequence
from typing import Tuple 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)) 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( def _hook_paths(
hook_type: str, hook_type: str,
git_dir: Optional[str] = None, git_dir: Optional[str] = None,
) -> Tuple[str, str]: ) -> Tuple[str, str]:
git_dir = git_dir if git_dir is not None else git.get_git_dir() pth = os.path.join(_hooks_dir(git_dir), hook_type)
pth = os.path.join(git_dir, 'hooks', hook_type)
return pth, f'{pth}.legacy' return pth, f'{pth}.legacy'
@ -128,9 +153,10 @@ def install(
skip_on_missing_config: bool = False, skip_on_missing_config: bool = False,
git_dir: Optional[str] = None, git_dir: Optional[str] = None,
) -> int: ) -> 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( 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`', 'hint: `git config --unset-all core.hooksPath`',
) )
return 1 return 1
@ -155,6 +181,10 @@ def install_hooks(config_file: str, store: Store) -> int:
def _uninstall_hook_script(hook_type: str) -> None: 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) hook_path, legacy_path = _hook_paths(hook_type)
# If our file doesn't exist or it isn't ours, gtfo. # If our file doesn't exist or it isn't ours, gtfo.

View file

@ -173,9 +173,13 @@ def has_diff(*args: str, repo: str = '.') -> bool:
return cmd_output_b(*cmd, cwd=repo, retcode=None)[0] == 1 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) _, 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: def init_repo(path: str, remote: str) -> None:

View file

@ -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() assert not in_git_dir.join('.git/hooks/pre-push').exists()
def test_install_refuses_core_hookspath(in_git_dir, store): def test_install_refuses_core_hookspath_outside_repo(in_git_dir, store):
cmd_output('git', 'config', '--local', 'core.hooksPath', 'hooks') cmd_output('git', 'config', '--local', 'core.hooksPath', '../hooks')
assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) 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): def test_install_hooks_dead_symlink(in_git_dir, store):