Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement default_install_hook_types #2322

Merged
merged 1 commit into from Apr 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions pre_commit/clientlib.py
Expand Up @@ -336,6 +336,11 @@ def check(self, dct: dict[str, Any]) -> None:
'Config', None,

cfgv.RequiredRecurse('repos', cfgv.Array(CONFIG_REPO_DICT)),
cfgv.Optional(
'default_install_hook_types',
cfgv.check_array(cfgv.check_one_of(C.HOOK_TYPES)),
['pre-commit'],
),
cfgv.OptionalRecurse(
'default_language_version', DEFAULT_LANGUAGE_VERSION, {},
),
Expand All @@ -355,6 +360,7 @@ def check(self, dct: dict[str, Any]) -> None:
cfgv.WarnAdditionalKeys(
(
'repos',
'default_install_hook_types',
'default_language_version',
'default_stages',
'files',
Expand Down
3 changes: 1 addition & 2 deletions pre_commit/commands/init_templatedir.py
Expand Up @@ -2,7 +2,6 @@

import logging
import os.path
from typing import Sequence

from pre_commit.commands.install_uninstall import install
from pre_commit.store import Store
Expand All @@ -16,7 +15,7 @@ def init_templatedir(
config_file: str,
store: Store,
directory: str,
hook_types: Sequence[str],
hook_types: list[str] | None,
skip_on_missing_config: bool = True,
) -> int:
install(
Expand Down
22 changes: 17 additions & 5 deletions pre_commit/commands/install_uninstall.py
Expand Up @@ -5,10 +5,10 @@
import shlex
import shutil
import sys
from typing import Sequence

from pre_commit import git
from pre_commit import output
from pre_commit.clientlib import InvalidConfigError
from pre_commit.clientlib import load_config
from pre_commit.repository import all_hooks
from pre_commit.repository import install_hook_envs
Expand All @@ -32,6 +32,18 @@
TEMPLATE_END = '# end templated\n'


def _hook_types(cfg_filename: str, hook_types: list[str] | None) -> list[str]:
if hook_types is not None:
return hook_types
else:
try:
cfg = load_config(cfg_filename)
except InvalidConfigError:
return ['pre-commit']
else:
return cfg['default_install_hook_types']


def _hook_paths(
hook_type: str,
git_dir: str | None = None,
Expand Down Expand Up @@ -103,7 +115,7 @@ def _install_hook_script(
def install(
config_file: str,
store: Store,
hook_types: Sequence[str],
hook_types: list[str] | None,
overwrite: bool = False,
hooks: bool = False,
skip_on_missing_config: bool = False,
Expand All @@ -116,7 +128,7 @@ def install(
)
return 1

for hook_type in hook_types:
for hook_type in _hook_types(config_file, hook_types):
_install_hook_script(
config_file, hook_type,
overwrite=overwrite,
Expand Down Expand Up @@ -150,7 +162,7 @@ def _uninstall_hook_script(hook_type: str) -> None:
output.write_line(f'Restored previous hooks to {hook_path}')


def uninstall(hook_types: Sequence[str]) -> int:
for hook_type in hook_types:
def uninstall(config_file: str, hook_types: list[str] | None) -> int:
for hook_type in _hook_types(config_file, hook_types):
_uninstall_hook_script(hook_type)
return 0
6 changes: 6 additions & 0 deletions pre_commit/constants.py
Expand Up @@ -24,4 +24,10 @@
'post-rewrite',
)

HOOK_TYPES = (
'pre-commit', 'pre-merge-commit', 'pre-push', 'prepare-commit-msg',
'commit-msg', 'post-commit', 'post-checkout', 'post-merge',
'post-rewrite',
)

DEFAULT = 'default'
34 changes: 6 additions & 28 deletions pre_commit/main.py
Expand Up @@ -4,7 +4,6 @@
import logging
import os
import sys
from typing import Any
from typing import Sequence

import pre_commit.constants as C
Expand Down Expand Up @@ -46,34 +45,10 @@ def _add_config_option(parser: argparse.ArgumentParser) -> None:
)


class AppendReplaceDefault(argparse.Action):
def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs)
self.appended = False

def __call__(
self,
parser: argparse.ArgumentParser,
namespace: argparse.Namespace,
values: str | Sequence[str] | None,
option_string: str | None = None,
) -> None:
if not self.appended:
setattr(namespace, self.dest, [])
self.appended = True
getattr(namespace, self.dest).append(values)


def _add_hook_type_option(parser: argparse.ArgumentParser) -> None:
parser.add_argument(
'-t', '--hook-type', choices=(
'pre-commit', 'pre-merge-commit', 'pre-push', 'prepare-commit-msg',
'commit-msg', 'post-commit', 'post-checkout', 'post-merge',
'post-rewrite',
),
action=AppendReplaceDefault,
default=['pre-commit'],
dest='hook_types',
'-t', '--hook-type',
choices=C.HOOK_TYPES, action='append', dest='hook_types',
)


Expand Down Expand Up @@ -399,7 +374,10 @@ def main(argv: Sequence[str] | None = None) -> int:
elif args.command == 'try-repo':
return try_repo(args)
elif args.command == 'uninstall':
return uninstall(hook_types=args.hook_types)
return uninstall(
config_file=args.config,
hook_types=args.hook_types,
)
else:
raise NotImplementedError(
f'Command {args.command} not implemented.',
Expand Down
54 changes: 49 additions & 5 deletions tests/commands/install_uninstall_test.py
Expand Up @@ -7,6 +7,7 @@

import pre_commit.constants as C
from pre_commit import git
from pre_commit.commands.install_uninstall import _hook_types
from pre_commit.commands.install_uninstall import CURRENT_HASH
from pre_commit.commands.install_uninstall import install
from pre_commit.commands.install_uninstall import install_hooks
Expand All @@ -27,6 +28,36 @@
from testing.util import git_commit


def test_hook_types_explicitly_listed():
assert _hook_types(os.devnull, ['pre-push']) == ['pre-push']


def test_hook_types_default_value_when_not_specified():
assert _hook_types(os.devnull, None) == ['pre-commit']


def test_hook_types_configured(tmpdir):
cfg = tmpdir.join('t.cfg')
cfg.write('default_install_hook_types: [pre-push]\nrepos: []\n')

assert _hook_types(str(cfg), None) == ['pre-push']


def test_hook_types_configured_nonsense(tmpdir):
cfg = tmpdir.join('t.cfg')
cfg.write('default_install_hook_types: []\nrepos: []\n')

# hopefully the user doesn't do this, but the code allows it!
assert _hook_types(str(cfg), None) == []


def test_hook_types_configuration_has_error(tmpdir):
cfg = tmpdir.join('t.cfg')
cfg.write('[')

assert _hook_types(str(cfg), None) == ['pre-commit']


def test_is_not_script():
assert is_our_script('setup.py') is False

Expand Down Expand Up @@ -61,7 +92,7 @@ def test_install_multiple_hooks_at_once(in_git_dir, store):
install(C.CONFIG_FILE, store, hook_types=['pre-commit', 'pre-push'])
assert in_git_dir.join('.git/hooks/pre-commit').exists()
assert in_git_dir.join('.git/hooks/pre-push').exists()
uninstall(hook_types=['pre-commit', 'pre-push'])
uninstall(C.CONFIG_FILE, hook_types=['pre-commit', 'pre-push'])
assert not in_git_dir.join('.git/hooks/pre-commit').exists()
assert not in_git_dir.join('.git/hooks/pre-push').exists()

Expand All @@ -79,14 +110,14 @@ def test_install_hooks_dead_symlink(in_git_dir, store):


def test_uninstall_does_not_blow_up_when_not_there(in_git_dir):
assert uninstall(hook_types=['pre-commit']) == 0
assert uninstall(C.CONFIG_FILE, hook_types=['pre-commit']) == 0


def test_uninstall(in_git_dir, store):
assert not in_git_dir.join('.git/hooks/pre-commit').exists()
install(C.CONFIG_FILE, store, hook_types=['pre-commit'])
assert in_git_dir.join('.git/hooks/pre-commit').exists()
uninstall(hook_types=['pre-commit'])
uninstall(C.CONFIG_FILE, hook_types=['pre-commit'])
assert not in_git_dir.join('.git/hooks/pre-commit').exists()


Expand Down Expand Up @@ -416,7 +447,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory, store):

# Now install and uninstall pre-commit
assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0
assert uninstall(hook_types=['pre-commit']) == 0
assert uninstall(C.CONFIG_FILE, hook_types=['pre-commit']) == 0

# Make sure we installed the "old" hook correctly
ret, output = _get_commit_output(tempdir_factory, touch_file='baz')
Expand Down Expand Up @@ -451,7 +482,7 @@ def test_uninstall_doesnt_remove_not_our_hooks(in_git_dir):
pre_commit.write('#!/usr/bin/env bash\necho 1\n')
make_executable(pre_commit.strpath)

assert uninstall(hook_types=['pre-commit']) == 0
assert uninstall(C.CONFIG_FILE, hook_types=['pre-commit']) == 0

assert pre_commit.exists()

Expand Down Expand Up @@ -1007,3 +1038,16 @@ def test_install_temporarily_allow_mising_config(tempdir_factory, store):
'Skipping `pre-commit`.'
)
assert expected in output


def test_install_uninstall_default_hook_types(in_git_dir, store):
cfg_src = 'default_install_hook_types: [pre-commit, pre-push]\nrepos: []\n'
in_git_dir.join(C.CONFIG_FILE).write(cfg_src)

assert not install(C.CONFIG_FILE, store, hook_types=None)
assert os.access(in_git_dir.join('.git/hooks/pre-commit').strpath, os.X_OK)
assert os.access(in_git_dir.join('.git/hooks/pre-push').strpath, os.X_OK)

assert not uninstall(C.CONFIG_FILE, hook_types=None)
assert not in_git_dir.join('.git/hooks/pre-commit').exists()
assert not in_git_dir.join('.git/hooks/pre-push').exists()
16 changes: 1 addition & 15 deletions tests/main_test.py
Expand Up @@ -14,20 +14,6 @@
from testing.util import cwd


@pytest.mark.parametrize(
('argv', 'expected'),
(
((), ['f']),
(('--f', 'x'), ['x']),
(('--f', 'x', '--f', 'y'), ['x', 'y']),
),
)
def test_append_replace_default(argv, expected):
parser = argparse.ArgumentParser()
parser.add_argument('--f', action=main.AppendReplaceDefault, default=['f'])
assert parser.parse_args(argv).f == expected


def _args(**kwargs):
kwargs.setdefault('command', 'help')
kwargs.setdefault('config', C.CONFIG_FILE)
Expand Down Expand Up @@ -172,7 +158,7 @@ def test_init_templatedir(mock_store_dir):

assert patch.call_count == 1
assert 'tdir' in patch.call_args[0]
assert patch.call_args[1]['hook_types'] == ['pre-commit']
assert patch.call_args[1]['hook_types'] is None
assert patch.call_args[1]['skip_on_missing_config'] is True


Expand Down