Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion pre_commit/clientlib/validate_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@
from pre_commit.errors import FatalError


_LOCAL_HOOKS_MAGIC_REPO_STRING = 'local'


def is_local_hooks(repo_entry):
return repo_entry['repo'] == _LOCAL_HOOKS_MAGIC_REPO_STRING


class InvalidConfigError(FatalError):
pass

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

def validate_config_extra(config):
for repo in config:
if 'sha' not in repo:
if is_local_hooks(repo):
if 'sha' in repo:
raise InvalidConfigError(
'"sha" property provided for local hooks'
)
elif 'sha' not in repo:
raise InvalidConfigError(
'Missing "sha" field for repository {0}'.format(repo['repo'])
)
Expand Down
38 changes: 14 additions & 24 deletions pre_commit/commands/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@
logger = logging.getLogger('pre_commit')


class HookExecutor(object):
def __init__(self, hook, invoker):
self.hook = hook
self._invoker = invoker

def invoke(self, filenames):
return self._invoker(self.hook, filenames)


def _get_skips(environ):
skips = environ.get('SKIP', '')
return set(skip.strip() for skip in skips.split(',') if skip.strip())
Expand Down Expand Up @@ -80,8 +71,7 @@ def get_filenames(args, include_expr, exclude_expr):
return getter(include_expr, exclude_expr)


def _run_single_hook(hook_executor, args, write, skips=frozenset()):
hook = hook_executor.hook
def _run_single_hook(hook, repo, args, write, skips=frozenset()):
filenames = get_filenames(args, hook['files'], hook['exclude'])
if hook['id'] in skips:
_print_user_skipped(hook, write, args)
Expand All @@ -95,7 +85,7 @@ def _run_single_hook(hook_executor, args, write, skips=frozenset()):
write(get_hook_message(_hook_msg_start(hook, args.verbose), end_len=6))
sys.stdout.flush()

retcode, stdout, stderr = hook_executor.invoke(filenames)
retcode, stdout, stderr = repo.run_hook(hook, filenames)

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


def _run_hooks(hook_executors, args, write, environ):
def _run_hooks(repo_hooks, args, write, environ):
"""Actually run the hooks."""
skips = _get_skips(environ)
retval = 0
for hook_executor in hook_executors:
retval |= _run_single_hook(hook_executor, args, write, skips)
for repo, hook in repo_hooks:
retval |= _run_single_hook(hook, repo, args, write, skips)
return retval


def get_hook_executors(runner):
def get_repo_hooks(runner):
for repo in runner.repositories:
for _, repo_hook in repo.hooks:
yield HookExecutor(repo_hook, repo.run_hook)
for _, hook in repo.hooks:
yield (repo, hook)


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

with ctx:
hook_executors = list(get_hook_executors(runner))
repo_hooks = list(get_repo_hooks(runner))
if args.hook:
hook_executors = [
he for he in hook_executors
if he.hook['id'] == args.hook
repo_hooks = [
(repo, hook) for repo, hook in repo_hooks
if hook['id'] == args.hook
]
if not hook_executors:
if not repo_hooks:
write('No hook with id `{0}`\n'.format(args.hook))
return 1
return _run_hooks(hook_executors, args, write, environ)
return _run_hooks(repo_hooks, args, write, environ)
40 changes: 36 additions & 4 deletions pre_commit/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

from cached_property import cached_property

from pre_commit import git
from pre_commit.clientlib.validate_config import is_local_hooks
from pre_commit.clientlib.validate_manifest import MANIFEST_JSON_SCHEMA
from pre_commit.jsonschema_extensions import apply_defaults
from pre_commit.languages.all import languages
from pre_commit.manifest import Manifest
from pre_commit.prefixed_command_runner import PrefixedCommandRunner
Expand All @@ -21,10 +25,13 @@ def __init__(self, repo_config, repo_path_getter):

@classmethod
def create(cls, config, store):
repo_path_getter = store.get_repo_path_getter(
config['repo'], config['sha']
)
return cls(config, repo_path_getter)
if is_local_hooks(config):
return LocalRepository(config)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make this if...: ... else: ... so it's slightly more explicit

else:
repo_path_getter = store.get_repo_path_getter(
config['repo'], config['sha']
)
return cls(config, repo_path_getter)

@cached_property
def repo_url(self):
Expand Down Expand Up @@ -111,3 +118,28 @@ def run_hook(self, hook, file_args):
return languages[hook['language']].run_hook(
self.cmd_runner, hook, file_args,
)


class LocalRepository(Repository):
def __init__(self, repo_config, repo_path_getter=None):
repo_path_getter = None
super(LocalRepository, self).__init__(repo_config, repo_path_getter)

@cached_property
def hooks(self):
return tuple(
(hook['id'], apply_defaults(hook, MANIFEST_JSON_SCHEMA['items']))
for hook in self.repo_config['hooks']
)

@cached_property
def cmd_runner(self):
return PrefixedCommandRunner(git.get_root())

@cached_property
def sha(self):
raise NotImplementedError

@cached_property
def manifest(self):
raise NotImplementedError
12 changes: 8 additions & 4 deletions testing/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,16 @@ def write_config(directory, config):
config_file.write(ordered_dump([config], **C.YAML_DUMP_KWARGS))


def make_consuming_repo(tmpdir_factory, repo_source):
path = make_repo(tmpdir_factory, repo_source)
config = make_config_from_repo(path)
git_path = git_dir(tmpdir_factory)
def add_config_to_repo(git_path, config):
write_config(git_path, config)
with cwd(git_path):
cmd_output('git', 'add', C.CONFIG_FILE)
cmd_output('git', 'commit', '-m', 'Add hooks config')
return git_path


def make_consuming_repo(tmpdir_factory, repo_source):
path = make_repo(tmpdir_factory, repo_source)
config = make_config_from_repo(path)
git_path = git_dir(tmpdir_factory)
return add_config_to_repo(git_path, config)
59 changes: 56 additions & 3 deletions tests/clientlib/validate_config_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import unicode_literals

import jsonschema
import pytest

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


@pytest.mark.parametrize(('manifest_obj', 'expected'), (
@pytest.mark.parametrize(('config_obj', 'expected'), (
([], False),
(
[{
Expand Down Expand Up @@ -66,8 +67,8 @@ def test_run(input, expected_output):
False,
),
))
def test_is_valid_according_to_schema(manifest_obj, expected):
ret = is_valid_according_to_schema(manifest_obj, CONFIG_JSON_SCHEMA)
def test_is_valid_according_to_schema(config_obj, expected):
ret = is_valid_according_to_schema(config_obj, CONFIG_JSON_SCHEMA)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops my bad! ++

assert ret is expected


Expand Down Expand Up @@ -121,3 +122,55 @@ def test_config_with_ok_exclude_regex_passes():
CONFIG_JSON_SCHEMA,
)
validate_config_extra(config)


@pytest.mark.parametrize('config_obj', (
[{
'repo': 'local',
'sha': 'foo',
'hooks': [{
'id': 'do_not_commit',
'name': 'Block if "DO NOT COMMIT" is found',
'entry': 'DO NOT COMMIT',
'language': 'pcre',
'files': '^(.*)$',
}],
}],
))
def test_config_with_local_hooks_definition_fails(config_obj):
with pytest.raises((
jsonschema.exceptions.ValidationError, InvalidConfigError
)):
jsonschema.validate(config_obj, CONFIG_JSON_SCHEMA)
config = apply_defaults(config_obj, CONFIG_JSON_SCHEMA)
validate_config_extra(config)


@pytest.mark.parametrize('config_obj', (
[{
'repo': 'local',
'hooks': [{
'id': 'arg-per-line',
'name': 'Args per line hook',
'entry': 'bin/hook.sh',
'language': 'script',
'files': '',
'args': ['hello', 'world'],
}],
}],
[{
'repo': 'local',
'hooks': [{
'id': 'arg-per-line',
'name': 'Args per line hook',
'entry': 'bin/hook.sh',
'language': 'script',
'files': '',
'args': ['hello', 'world'],
}]
}],
))
def test_config_with_local_hooks_definition_passes(config_obj):
jsonschema.validate(config_obj, CONFIG_JSON_SCHEMA)
config = apply_defaults(config_obj, CONFIG_JSON_SCHEMA)
validate_config_extra(config)
71 changes: 67 additions & 4 deletions tests/commands/run_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
from pre_commit.commands.run import _has_unmerged_paths
from pre_commit.commands.run import get_changed_files
from pre_commit.commands.run import run
from pre_commit.ordereddict import OrderedDict
from pre_commit.runner import Runner
from pre_commit.util import cmd_output
from pre_commit.util import cwd
from testing.auto_namedtuple import auto_namedtuple
from testing.fixtures import add_config_to_repo
from testing.fixtures import make_consuming_repo


Expand Down Expand Up @@ -81,7 +83,7 @@ def _test_run(repo, options, expected_outputs, expected_ret, stage):
stage_a_file()
args = _get_opts(**options)
ret, printed = _do_run(repo, args)
assert ret == expected_ret
assert ret == expected_ret, (ret, expected_ret, printed)
for expected_output_part in expected_outputs:
assert expected_output_part in printed

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

# Write a crap ton of files
Expand All @@ -334,3 +334,66 @@ def test_lots_of_files(mock_out_store_directory, tmpdir_factory):
stderr=subprocess.STDOUT,
env=env,
)


def test_local_hook_passes(
repo_with_passing_hook, mock_out_store_directory,
):
config = OrderedDict((
('repo', 'local'),
('hooks', (OrderedDict((
('id', 'pylint'),
('name', 'PyLint'),
('entry', 'python -m pylint.__main__'),
('language', 'system'),
('files', r'\.py$'),
)), OrderedDict((
('id', 'do_not_commit'),
('name', 'Block if "DO NOT COMMIT" is found'),
('entry', 'DO NOT COMMIT'),
('language', 'pcre'),
('files', '^(.*)$'),
))))
))
add_config_to_repo(repo_with_passing_hook, config)

with io.open('dummy.py', 'w') as staged_file:
staged_file.write('"""TODO: something"""\n')
cmd_output('git', 'add', 'dummy.py')

_test_run(
repo_with_passing_hook,
options={},
expected_outputs=[''],
expected_ret=0,
stage=False
)


def test_local_hook_fails(
repo_with_passing_hook, mock_out_store_directory,
):
config = OrderedDict((
('repo', 'local'),
('hooks', [OrderedDict((
('id', 'no-todo'),
('name', 'No TODO'),
('entry', 'grep -iI todo'),
('expected_return_value', 1),
('language', 'system'),
('files', ''),
))])
))
add_config_to_repo(repo_with_passing_hook, config)

with io.open('dummy.py', 'w') as staged_file:
staged_file.write('"""TODO: something"""\n')
cmd_output('git', 'add', 'dummy.py')

_test_run(
repo_with_passing_hook,
options={},
expected_outputs=[''],
expected_ret=1,
stage=False
)
Loading