diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index 4dce674f..a80c6b40 100644 --- a/pre_commit/commands/autoupdate.py +++ b/pre_commit/commands/autoupdate.py @@ -57,7 +57,7 @@ def _update_repo(repo_config, runner, tags_only): # See if any of our hooks were deleted with the new commits hooks = {hook['id'] for hook in repo.repo_config['hooks']} - hooks_missing = hooks - (hooks & set(new_repo.manifest.hooks)) + hooks_missing = hooks - (hooks & set(new_repo.manifest_hooks)) if hooks_missing: raise RepositoryCannotBeUpdatedError( 'Cannot update because the tip of master is missing these hooks:\n' diff --git a/pre_commit/commands/try_repo.py b/pre_commit/commands/try_repo.py index 2e1933d8..4c825823 100644 --- a/pre_commit/commands/try_repo.py +++ b/pre_commit/commands/try_repo.py @@ -9,8 +9,8 @@ from aspy.yaml import ordered_dump import pre_commit.constants as C from pre_commit import git from pre_commit import output +from pre_commit.clientlib import load_manifest from pre_commit.commands.run import run -from pre_commit.manifest import Manifest from pre_commit.runner import Runner from pre_commit.store import Store from pre_commit.util import tmpdir @@ -23,8 +23,10 @@ def try_repo(args): if args.hook: hooks = [{'id': args.hook}] else: - manifest = Manifest(Store(tempdir).clone(args.repo, ref)) - hooks = [{'id': hook_id} for hook_id in sorted(manifest.hooks)] + repo_path = Store(tempdir).clone(args.repo, ref) + manifest = load_manifest(os.path.join(repo_path, C.MANIFEST_FILE)) + manifest = sorted(manifest, key=lambda hook: hook['id']) + hooks = [{'id': hook['id']} for hook in manifest] items = (('repo', args.repo), ('sha', ref), ('hooks', hooks)) config = {'repos': [collections.OrderedDict(items)]} diff --git a/pre_commit/manifest.py b/pre_commit/manifest.py deleted file mode 100644 index 10e312fb..00000000 --- a/pre_commit/manifest.py +++ /dev/null @@ -1,28 +0,0 @@ -from __future__ import unicode_literals - -import os.path - -from cached_property import cached_property - -import pre_commit.constants as C -from pre_commit.clientlib import load_manifest -from pre_commit.languages.all import languages - - -class Manifest(object): - def __init__(self, repo_path): - self.repo_path = repo_path - - @cached_property - def manifest_contents(self): - return load_manifest(os.path.join(self.repo_path, C.MANIFEST_FILE)) - - @cached_property - def hooks(self): - ret = {} - for hook in self.manifest_contents: - if hook['language_version'] == 'default': - language = languages[hook['language']] - hook['language_version'] = language.get_default_version() - ret[hook['id']] = hook - return ret diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 45389cb4..0afb5004 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -17,10 +17,10 @@ from pre_commit import five from pre_commit import git from pre_commit.clientlib import is_local_repo from pre_commit.clientlib import is_meta_repo +from pre_commit.clientlib import load_manifest from pre_commit.clientlib import MANIFEST_HOOK_DICT from pre_commit.languages.all import languages from pre_commit.languages.helpers import environment_dir -from pre_commit.manifest import Manifest from pre_commit.prefixed_command_runner import PrefixedCommandRunner from pre_commit.schema import apply_defaults from pre_commit.schema import validate @@ -105,20 +105,27 @@ def _install_all(venvs, repo_url, store): _write_state(cmd_runner, venv, state) -def _validate_minimum_version(hook): - hook_version = pkg_resources.parse_version( - hook['minimum_pre_commit_version'], - ) - if hook_version > C.VERSION_PARSED: +def _hook(*hook_dicts): + ret, rest = dict(hook_dicts[0]), hook_dicts[1:] + for dct in rest: + ret.update(dct) + + version = pkg_resources.parse_version(ret['minimum_pre_commit_version']) + if version > C.VERSION_PARSED: logger.error( - 'The hook `{}` requires pre-commit version {} but ' - 'version {} is installed. ' + 'The hook `{}` requires pre-commit version {} but version {} ' + 'is installed. ' 'Perhaps run `pip install --upgrade pre-commit`.'.format( - hook['id'], hook_version, C.VERSION_PARSED, + ret['id'], version, C.VERSION_PARSED, ), ) exit(1) - return hook + + if ret['language_version'] == 'default': + language = languages[ret['language']] + ret['language_version'] = language.get_default_version() + + return ret class Repository(object): @@ -150,13 +157,14 @@ class Repository(object): return self._cmd_runner @cached_property - def manifest(self): - return Manifest(self._repo_path) + def manifest_hooks(self): + manifest_path = os.path.join(self._repo_path, C.MANIFEST_FILE) + return {hook['id']: hook for hook in load_manifest(manifest_path)} @cached_property def hooks(self): for hook in self.repo_config['hooks']: - if hook['id'] not in self.manifest.hooks: + if hook['id'] not in self.manifest_hooks: logger.error( '`{}` is not present in repository {}. ' 'Typo? Perhaps it is introduced in a newer version? ' @@ -166,10 +174,8 @@ class Repository(object): ) exit(1) - _validate_minimum_version(self.manifest.hooks[hook['id']]) - return tuple( - (hook['id'], dict(self.manifest.hooks[hook['id']], **hook)) + (hook['id'], _hook(self.manifest_hooks[hook['id']], hook)) for hook in self.repo_config['hooks'] ) @@ -220,16 +226,14 @@ class LocalRepository(Repository): @cached_property def hooks(self): + def _from_manifest_dct(dct): + dct = validate(dct, MANIFEST_HOOK_DICT) + dct = apply_defaults(dct, MANIFEST_HOOK_DICT) + dct = _hook(dct) + return dct + return tuple( - ( - hook['id'], - _validate_minimum_version( - apply_defaults( - validate(hook, MANIFEST_HOOK_DICT), - MANIFEST_HOOK_DICT, - ), - ), - ) + (hook['id'], _from_manifest_dct(hook)) for hook in self.repo_config['hooks'] ) diff --git a/tests/manifest_test.py b/tests/manifest_test.py deleted file mode 100644 index b7603a0d..00000000 --- a/tests/manifest_test.py +++ /dev/null @@ -1,70 +0,0 @@ -from __future__ import absolute_import -from __future__ import unicode_literals - -import pytest - -from pre_commit import git -from pre_commit.manifest import Manifest -from testing.fixtures import make_repo - - -@pytest.yield_fixture -def manifest(store, tempdir_factory): - path = make_repo(tempdir_factory, 'script_hooks_repo') - repo_path = store.clone(path, git.head_sha(path)) - yield Manifest(repo_path) - - -def test_manifest_contents(manifest): - # Should just retrieve the manifest contents - assert manifest.manifest_contents == [{ - 'always_run': False, - 'additional_dependencies': [], - 'args': [], - 'description': '', - 'entry': 'bin/hook.sh', - 'exclude': '^$', - 'files': '', - 'id': 'bash_hook', - 'language': 'script', - 'language_version': 'default', - 'log_file': '', - 'minimum_pre_commit_version': '0', - 'name': 'Bash hook', - 'pass_filenames': True, - 'stages': [], - 'types': ['file'], - 'exclude_types': [], - }] - - -def test_hooks(manifest): - assert manifest.hooks['bash_hook'] == { - 'always_run': False, - 'additional_dependencies': [], - 'args': [], - 'description': '', - 'entry': 'bin/hook.sh', - 'exclude': '^$', - 'files': '', - 'id': 'bash_hook', - 'language': 'script', - 'language_version': 'default', - 'log_file': '', - 'minimum_pre_commit_version': '0', - 'name': 'Bash hook', - 'pass_filenames': True, - 'stages': [], - 'types': ['file'], - 'exclude_types': [], - } - - -def test_default_python_language_version(store, tempdir_factory): - path = make_repo(tempdir_factory, 'python_hooks_repo') - repo_path = store.clone(path, git.head_sha(path)) - manifest = Manifest(repo_path) - - # This assertion is difficult as it is version dependent, just assert - # that it is *something* - assert manifest.hooks['foo']['language_version'] != 'default' diff --git a/tests/repository_test.py b/tests/repository_test.py index 263ce1ea..80489406 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -71,6 +71,16 @@ def test_python_hook(tempdir_factory, store): ) +@pytest.mark.integration +def test_python_hook_default_version(tempdir_factory, store): + # make sure that this continues to work for platforms where default + # language detection does not work + with mock.patch.object( + python, 'get_default_version', return_value='default', + ): + test_python_hook(tempdir_factory, store) + + @pytest.mark.integration def test_python_hook_args_with_spaces(tempdir_factory, store): _test_hook_repo( @@ -690,6 +700,8 @@ def test_local_python_repo(store): config = {'repo': 'local', 'hooks': hooks} repo = Repository.create(config, store) (_, hook), = repo.hooks + # language_version should have been adjusted to the interpreter version + assert hook['language_version'] != 'default' ret = repo.run_hook(hook, ('filename',)) assert ret[0] == 0 assert _norm_out(ret[1]) == b"['filename']\nHello World\n" @@ -746,3 +758,29 @@ def test_versions_ok(tempdir_factory, store, version): config = make_config_from_repo(path) # Should succeed Repository.create(config, store).require_installed() + + +def test_manifest_hooks(tempdir_factory, store): + path = make_repo(tempdir_factory, 'script_hooks_repo') + config = make_config_from_repo(path) + repo = Repository.create(config, store) + + assert repo.manifest_hooks['bash_hook'] == { + 'always_run': False, + 'additional_dependencies': [], + 'args': [], + 'description': '', + 'entry': 'bin/hook.sh', + 'exclude': '^$', + 'files': '', + 'id': 'bash_hook', + 'language': 'script', + 'language_version': 'default', + 'log_file': '', + 'minimum_pre_commit_version': '0', + 'name': 'Bash hook', + 'pass_filenames': True, + 'stages': [], + 'types': ['file'], + 'exclude_types': [], + }