Skip to content

Commit b68261c

Browse files
committed
Adding support for locally-defined hooks
1 parent d97ea30 commit b68261c

File tree

8 files changed

+241
-40
lines changed

8 files changed

+241
-40
lines changed

pre_commit/clientlib/validate_config.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@
66
from pre_commit.errors import FatalError
77

88

9+
_LOCAL_HOOKS_MAGIC_REPO_STRING = 'local'
10+
11+
12+
def is_local_hooks(repo_entry):
13+
return repo_entry['repo'] == _LOCAL_HOOKS_MAGIC_REPO_STRING
14+
15+
916
class InvalidConfigError(FatalError):
1017
pass
1118

@@ -53,7 +60,12 @@ def try_regex(repo, hook, value, field_name):
5360

5461
def validate_config_extra(config):
5562
for repo in config:
56-
if 'sha' not in repo:
63+
if is_local_hooks(repo):
64+
if 'sha' in repo:
65+
raise InvalidConfigError(
66+
'"sha" property provided for local hooks'
67+
)
68+
elif 'sha' not in repo:
5769
raise InvalidConfigError(
5870
'Missing "sha" field for repository {0}'.format(repo['repo'])
5971
)

pre_commit/commands/run.py

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,6 @@
1818
logger = logging.getLogger('pre_commit')
1919

2020

21-
class HookExecutor(object):
22-
def __init__(self, hook, invoker):
23-
self.hook = hook
24-
self._invoker = invoker
25-
26-
def invoke(self, filenames):
27-
return self._invoker(self.hook, filenames)
28-
29-
3021
def _get_skips(environ):
3122
skips = environ.get('SKIP', '')
3223
return set(skip.strip() for skip in skips.split(',') if skip.strip())
@@ -80,8 +71,7 @@ def get_filenames(args, include_expr, exclude_expr):
8071
return getter(include_expr, exclude_expr)
8172

8273

83-
def _run_single_hook(hook_executor, args, write, skips=frozenset()):
84-
hook = hook_executor.hook
74+
def _run_single_hook(hook, repo, args, write, skips=frozenset()):
8575
filenames = get_filenames(args, hook['files'], hook['exclude'])
8676
if hook['id'] in skips:
8777
_print_user_skipped(hook, write, args)
@@ -95,7 +85,7 @@ def _run_single_hook(hook_executor, args, write, skips=frozenset()):
9585
write(get_hook_message(_hook_msg_start(hook, args.verbose), end_len=6))
9686
sys.stdout.flush()
9787

98-
retcode, stdout, stderr = hook_executor.invoke(filenames)
88+
retcode, stdout, stderr = repo.run_hook(hook, filenames)
9989

10090
if retcode != hook['expected_return_value']:
10191
retcode = 1
@@ -119,19 +109,19 @@ def _run_single_hook(hook_executor, args, write, skips=frozenset()):
119109
return retcode
120110

121111

122-
def _run_hooks(hook_executors, args, write, environ):
112+
def _run_hooks(repo_hooks, args, write, environ):
123113
"""Actually run the hooks."""
124114
skips = _get_skips(environ)
125115
retval = 0
126-
for hook_executor in hook_executors:
127-
retval |= _run_single_hook(hook_executor, args, write, skips)
116+
for repo, hook in repo_hooks:
117+
retval |= _run_single_hook(hook, repo, args, write, skips)
128118
return retval
129119

130120

131-
def get_hook_executors(runner):
121+
def get_repo_hooks(runner):
132122
for repo in runner.repositories:
133-
for _, repo_hook in repo.hooks:
134-
yield HookExecutor(repo_hook, repo.run_hook)
123+
for _, hook in repo.hooks:
124+
yield (repo, hook)
135125

136126

137127
def _has_unmerged_paths(runner):
@@ -159,13 +149,13 @@ def run(runner, args, write=sys_stdout_write_wrapper, environ=os.environ):
159149
ctx = staged_files_only(runner.cmd_runner)
160150

161151
with ctx:
162-
hook_executors = list(get_hook_executors(runner))
152+
repo_hooks = list(get_repo_hooks(runner))
163153
if args.hook:
164-
hook_executors = [
165-
he for he in hook_executors
166-
if he.hook['id'] == args.hook
154+
repo_hooks = [
155+
(repo, hook) for repo, hook in repo_hooks
156+
if hook['id'] == args.hook
167157
]
168-
if not hook_executors:
158+
if not repo_hooks:
169159
write('No hook with id `{0}`\n'.format(args.hook))
170160
return 1
171-
return _run_hooks(hook_executors, args, write, environ)
161+
return _run_hooks(repo_hooks, args, write, environ)

pre_commit/repository.py

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55

66
from cached_property import cached_property
77

8+
from pre_commit import git
9+
from pre_commit.clientlib.validate_config import is_local_hooks
10+
from pre_commit.clientlib.validate_manifest import MANIFEST_JSON_SCHEMA
11+
from pre_commit.jsonschema_extensions import apply_defaults
812
from pre_commit.languages.all import languages
913
from pre_commit.manifest import Manifest
1014
from pre_commit.prefixed_command_runner import PrefixedCommandRunner
@@ -21,10 +25,13 @@ def __init__(self, repo_config, repo_path_getter):
2125

2226
@classmethod
2327
def create(cls, config, store):
24-
repo_path_getter = store.get_repo_path_getter(
25-
config['repo'], config['sha']
26-
)
27-
return cls(config, repo_path_getter)
28+
if is_local_hooks(config):
29+
return LocalRepository(config)
30+
else:
31+
repo_path_getter = store.get_repo_path_getter(
32+
config['repo'], config['sha']
33+
)
34+
return cls(config, repo_path_getter)
2835

2936
@cached_property
3037
def repo_url(self):
@@ -111,3 +118,28 @@ def run_hook(self, hook, file_args):
111118
return languages[hook['language']].run_hook(
112119
self.cmd_runner, hook, file_args,
113120
)
121+
122+
123+
class LocalRepository(Repository):
124+
def __init__(self, repo_config, repo_path_getter=None):
125+
repo_path_getter = None
126+
super(LocalRepository, self).__init__(repo_config, repo_path_getter)
127+
128+
@cached_property
129+
def hooks(self):
130+
return tuple(
131+
(hook['id'], apply_defaults(hook, MANIFEST_JSON_SCHEMA['items']))
132+
for hook in self.repo_config['hooks']
133+
)
134+
135+
@cached_property
136+
def cmd_runner(self):
137+
return PrefixedCommandRunner(git.get_root())
138+
139+
@cached_property
140+
def sha(self):
141+
raise NotImplementedError
142+
143+
@cached_property
144+
def manifest(self):
145+
raise NotImplementedError

testing/fixtures.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,16 @@ def write_config(directory, config):
6060
config_file.write(ordered_dump([config], **C.YAML_DUMP_KWARGS))
6161

6262

63-
def make_consuming_repo(tmpdir_factory, repo_source):
64-
path = make_repo(tmpdir_factory, repo_source)
65-
config = make_config_from_repo(path)
66-
git_path = git_dir(tmpdir_factory)
63+
def add_config_to_repo(git_path, config):
6764
write_config(git_path, config)
6865
with cwd(git_path):
6966
cmd_output('git', 'add', C.CONFIG_FILE)
7067
cmd_output('git', 'commit', '-m', 'Add hooks config')
7168
return git_path
69+
70+
71+
def make_consuming_repo(tmpdir_factory, repo_source):
72+
path = make_repo(tmpdir_factory, repo_source)
73+
config = make_config_from_repo(path)
74+
git_path = git_dir(tmpdir_factory)
75+
return add_config_to_repo(git_path, config)

tests/clientlib/validate_config_test.py

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import unicode_literals
22

3+
import jsonschema
34
import pytest
45

56
from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA
@@ -25,7 +26,7 @@ def test_run(input, expected_output):
2526
assert run(input) == expected_output
2627

2728

28-
@pytest.mark.parametrize(('manifest_obj', 'expected'), (
29+
@pytest.mark.parametrize(('config_obj', 'expected'), (
2930
([], False),
3031
(
3132
[{
@@ -66,8 +67,8 @@ def test_run(input, expected_output):
6667
False,
6768
),
6869
))
69-
def test_is_valid_according_to_schema(manifest_obj, expected):
70-
ret = is_valid_according_to_schema(manifest_obj, CONFIG_JSON_SCHEMA)
70+
def test_is_valid_according_to_schema(config_obj, expected):
71+
ret = is_valid_according_to_schema(config_obj, CONFIG_JSON_SCHEMA)
7172
assert ret is expected
7273

7374

@@ -121,3 +122,55 @@ def test_config_with_ok_exclude_regex_passes():
121122
CONFIG_JSON_SCHEMA,
122123
)
123124
validate_config_extra(config)
125+
126+
127+
@pytest.mark.parametrize('config_obj', (
128+
[{
129+
'repo': 'local',
130+
'sha': 'foo',
131+
'hooks': [{
132+
'id': 'do_not_commit',
133+
'name': 'Block if "DO NOT COMMIT" is found',
134+
'entry': 'DO NOT COMMIT',
135+
'language': 'pcre',
136+
'files': '^(.*)$',
137+
}],
138+
}],
139+
))
140+
def test_config_with_local_hooks_definition_fails(config_obj):
141+
with pytest.raises((
142+
jsonschema.exceptions.ValidationError, InvalidConfigError
143+
)):
144+
jsonschema.validate(config_obj, CONFIG_JSON_SCHEMA)
145+
config = apply_defaults(config_obj, CONFIG_JSON_SCHEMA)
146+
validate_config_extra(config)
147+
148+
149+
@pytest.mark.parametrize('config_obj', (
150+
[{
151+
'repo': 'local',
152+
'hooks': [{
153+
'id': 'arg-per-line',
154+
'name': 'Args per line hook',
155+
'entry': 'bin/hook.sh',
156+
'language': 'script',
157+
'files': '',
158+
'args': ['hello', 'world'],
159+
}],
160+
}],
161+
[{
162+
'repo': 'local',
163+
'hooks': [{
164+
'id': 'arg-per-line',
165+
'name': 'Args per line hook',
166+
'entry': 'bin/hook.sh',
167+
'language': 'script',
168+
'files': '',
169+
'args': ['hello', 'world'],
170+
}]
171+
}],
172+
))
173+
def test_config_with_local_hooks_definition_passes(config_obj):
174+
jsonschema.validate(config_obj, CONFIG_JSON_SCHEMA)
175+
config = apply_defaults(config_obj, CONFIG_JSON_SCHEMA)
176+
validate_config_extra(config)

tests/commands/run_test.py

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
from pre_commit.commands.run import _has_unmerged_paths
1515
from pre_commit.commands.run import get_changed_files
1616
from pre_commit.commands.run import run
17+
from pre_commit.ordereddict import OrderedDict
1718
from pre_commit.runner import Runner
1819
from pre_commit.util import cmd_output
1920
from pre_commit.util import cwd
2021
from testing.auto_namedtuple import auto_namedtuple
22+
from testing.fixtures import add_config_to_repo
2123
from testing.fixtures import make_consuming_repo
2224

2325

@@ -81,7 +83,7 @@ def _test_run(repo, options, expected_outputs, expected_ret, stage):
8183
stage_a_file()
8284
args = _get_opts(**options)
8385
ret, printed = _do_run(repo, args)
84-
assert ret == expected_ret
86+
assert ret == expected_ret, (ret, expected_ret, printed)
8587
for expected_output_part in expected_outputs:
8688
assert expected_output_part in printed
8789

@@ -313,9 +315,7 @@ def test_lots_of_files(mock_out_store_directory, tmpdir_factory):
313315
git_path = make_consuming_repo(tmpdir_factory, 'python_hooks_repo')
314316
with cwd(git_path):
315317
# Override files so we run against them
316-
with io.open(
317-
'.pre-commit-config.yaml', 'a+',
318-
) as config_file:
318+
with io.open('.pre-commit-config.yaml', 'a+') as config_file:
319319
config_file.write(' files: ""\n')
320320

321321
# Write a crap ton of files
@@ -334,3 +334,66 @@ def test_lots_of_files(mock_out_store_directory, tmpdir_factory):
334334
stderr=subprocess.STDOUT,
335335
env=env,
336336
)
337+
338+
339+
def test_local_hook_passes(
340+
repo_with_passing_hook, mock_out_store_directory,
341+
):
342+
config = OrderedDict((
343+
('repo', 'local'),
344+
('hooks', (OrderedDict((
345+
('id', 'pylint'),
346+
('name', 'PyLint'),
347+
('entry', 'python -m pylint.__main__'),
348+
('language', 'system'),
349+
('files', r'\.py$'),
350+
)), OrderedDict((
351+
('id', 'do_not_commit'),
352+
('name', 'Block if "DO NOT COMMIT" is found'),
353+
('entry', 'DO NOT COMMIT'),
354+
('language', 'pcre'),
355+
('files', '^(.*)$'),
356+
))))
357+
))
358+
add_config_to_repo(repo_with_passing_hook, config)
359+
360+
with io.open('dummy.py', 'w') as staged_file:
361+
staged_file.write('"""TODO: something"""\n')
362+
cmd_output('git', 'add', 'dummy.py')
363+
364+
_test_run(
365+
repo_with_passing_hook,
366+
options={},
367+
expected_outputs=[''],
368+
expected_ret=0,
369+
stage=False
370+
)
371+
372+
373+
def test_local_hook_fails(
374+
repo_with_passing_hook, mock_out_store_directory,
375+
):
376+
config = OrderedDict((
377+
('repo', 'local'),
378+
('hooks', [OrderedDict((
379+
('id', 'no-todo'),
380+
('name', 'No TODO'),
381+
('entry', 'grep -iI todo'),
382+
('expected_return_value', 1),
383+
('language', 'system'),
384+
('files', ''),
385+
))])
386+
))
387+
add_config_to_repo(repo_with_passing_hook, config)
388+
389+
with io.open('dummy.py', 'w') as staged_file:
390+
staged_file.write('"""TODO: something"""\n')
391+
cmd_output('git', 'add', 'dummy.py')
392+
393+
_test_run(
394+
repo_with_passing_hook,
395+
options={},
396+
expected_outputs=[''],
397+
expected_ret=1,
398+
stage=False
399+
)

0 commit comments

Comments
 (0)