Merge pull request #899 from pre-commit/repository_refactor

Refactor pre_commit.repository and factor out cached-property

.pre-commit-config.yaml

@@ -19,6 +19,10 @@ repos:
     rev: v1.11.2
     hooks:
     -   id: validate_manifest
+-   repo: https://github.com/asottile/pyupgrade
+    rev: v1.11.0
+    hooks:
+    -   id: pyupgrade
 -   repo: https://github.com/asottile/reorder_python_imports
     rev: v1.3.0
     hooks:

.travis.yml

@@ -1,6 +1,5 @@
 language: python
-dist: trusty
+dist: xenial
-sudo: required
 services:
     - docker
 matrix:
@@ -13,8 +12,6 @@ matrix:
             python: pypy2.7-5.10.0
         -   env: TOXENV=py37
             python: 3.7
-            sudo: required
-            dist: xenial
 install: pip install coveralls tox
 script: tox
 before_install:

pre_commit/commands/autoupdate.py

@@ -1,8 +1,8 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+import os.path
 import re
-from collections import OrderedDict
 
 import six
 from aspy.yaml import ordered_dump
@@ -16,8 +16,8 @@ from pre_commit.clientlib import InvalidManifestError
 from pre_commit.clientlib import is_local_repo
 from pre_commit.clientlib import is_meta_repo
 from pre_commit.clientlib import load_config
+from pre_commit.clientlib import load_manifest
 from pre_commit.commands.migrate_config import migrate_config
-from pre_commit.repository import Repository
 from pre_commit.util import CalledProcessError
 from pre_commit.util import cmd_output
 
@@ -52,24 +52,24 @@ def _update_repo(repo_config, store, tags_only):
     if rev == repo_config['rev']:
         return repo_config
 
-    # Construct a new config with the head rev
-    new_config = OrderedDict(repo_config)
-    new_config['rev'] = rev
-
     try:
-        new_hooks = Repository.create(new_config, store).manifest_hooks
+        path = store.clone(repo_config['repo'], rev)
+        manifest = load_manifest(os.path.join(path, C.MANIFEST_FILE))
     except InvalidManifestError as e:
         raise RepositoryCannotBeUpdatedError(six.text_type(e))
 
     # See if any of our hooks were deleted with the new commits
     hooks = {hook['id'] for hook in repo_config['hooks']}
-    hooks_missing = hooks - set(new_hooks)
+    hooks_missing = hooks - {hook['id'] for hook in manifest}
     if hooks_missing:
         raise RepositoryCannotBeUpdatedError(
             'Cannot update because the tip of master is missing these hooks:\n'
             '{}'.format(', '.join(sorted(hooks_missing))),
         )
 
+    # Construct a new config with the head rev
+    new_config = repo_config.copy()
+    new_config['rev'] = rev
     return new_config
 
 

pre_commit/commands/install_uninstall.py

@@ -10,7 +10,8 @@ from pre_commit import git
 from pre_commit import output
 from pre_commit.clientlib import load_config
 from pre_commit.languages import python
-from pre_commit.repository import repositories
+from pre_commit.repository import all_hooks
+from pre_commit.repository import install_hook_envs
 from pre_commit.util import cmd_output
 from pre_commit.util import make_executable
 from pre_commit.util import mkdirp
@@ -116,8 +117,7 @@ def install(
 
 
 def install_hooks(config_file, store):
-    for repository in repositories(load_config(config_file), store):
+    install_hook_envs(all_hooks(load_config(config_file), store), store)
-        repository.require_installed()
 
 
 def uninstall(hook_type='pre-commit'):

pre_commit/commands/run.py

@@ -13,7 +13,8 @@ from pre_commit import git
 from pre_commit import output
 from pre_commit.clientlib import load_config
 from pre_commit.output import get_hook_message
-from pre_commit.repository import repositories
+from pre_commit.repository import all_hooks
+from pre_commit.repository import install_hook_envs
 from pre_commit.staged_files_only import staged_files_only
 from pre_commit.util import cmd_output
 from pre_commit.util import memoize_by_cwd
@@ -32,9 +33,7 @@ def _get_skips(environ):
 
 
 def _hook_msg_start(hook, verbose):
-    return '{}{}'.format(
+    return '{}{}'.format('[{}] '.format(hook.id) if verbose else '', hook.name)
-        '[{}] '.format(hook['id']) if verbose else '', hook['name'],
-    )
 
 
 def _filter_by_include_exclude(filenames, include, exclude):
@@ -63,21 +62,21 @@ SKIPPED = 'Skipped'
 NO_FILES = '(no files to check)'
 
 
-def _run_single_hook(filenames, hook, repo, args, skips, cols):
+def _run_single_hook(filenames, hook, args, skips, cols):
-    include, exclude = hook['files'], hook['exclude']
+    include, exclude = hook.files, hook.exclude
     filenames = _filter_by_include_exclude(filenames, include, exclude)
-    types, exclude_types = hook['types'], hook['exclude_types']
+    types, exclude_types = hook.types, hook.exclude_types
     filenames = _filter_by_types(filenames, types, exclude_types)
 
-    if hook['language'] == 'pcre':
+    if hook.language == 'pcre':
         logger.warning(
             '`{}` (from {}) uses the deprecated pcre language.\n'
             'The pcre language is scheduled for removal in pre-commit 2.x.\n'
             'The pygrep language is a more portable (and usually drop-in) '
-            'replacement.'.format(hook['id'], repo.repo_config['repo']),
+            'replacement.'.format(hook.id, hook.src),
         )
 
-    if hook['id'] in skips or hook['alias'] in skips:
+    if hook.id in skips or hook.alias in skips:
         output.write(get_hook_message(
             _hook_msg_start(hook, args.verbose),
             end_msg=SKIPPED,
@@ -86,7 +85,7 @@ def _run_single_hook(filenames, hook, repo, args, skips, cols):
             cols=cols,
         ))
         return 0
-    elif not filenames and not hook['always_run']:
+    elif not filenames and not hook.always_run:
         output.write(get_hook_message(
             _hook_msg_start(hook, args.verbose),
             postfix=NO_FILES,
@@ -107,8 +106,8 @@ def _run_single_hook(filenames, hook, repo, args, skips, cols):
     diff_before = cmd_output(
         'git', 'diff', '--no-ext-diff', retcode=None, encoding=None,
     )
-    retcode, stdout, stderr = repo.run_hook(
+    retcode, stdout, stderr = hook.run(
-        hook, tuple(filenames) if hook['pass_filenames'] else (),
+        tuple(filenames) if hook.pass_filenames else (),
     )
     diff_after = cmd_output(
         'git', 'diff', '--no-ext-diff', retcode=None, encoding=None,
@@ -133,9 +132,9 @@ def _run_single_hook(filenames, hook, repo, args, skips, cols):
 
     if (
             (stdout or stderr or file_modifications) and
-            (retcode or args.verbose or hook['verbose'])
+            (retcode or args.verbose or hook.verbose)
     ):
-        output.write_line('hookid: {}\n'.format(hook['id']))
+        output.write_line('hookid: {}\n'.format(hook.id))
 
         # Print a message if failing due to file modifications
         if file_modifications:
@@ -149,7 +148,7 @@ def _run_single_hook(filenames, hook, repo, args, skips, cols):
         for out in (stdout, stderr):
             assert type(out) is bytes, type(out)
             if out.strip():
-                output.write_line(out.strip(), logfile_name=hook['log_file'])
+                output.write_line(out.strip(), logfile_name=hook.log_file)
         output.write_line()
 
     return retcode
@@ -189,15 +188,15 @@ def _all_filenames(args):
         return git.get_staged_files()
 
 
-def _run_hooks(config, repo_hooks, args, environ):
+def _run_hooks(config, hooks, args, environ):
     """Actually run the hooks."""
     skips = _get_skips(environ)
-    cols = _compute_cols([hook for _, hook in repo_hooks], args.verbose)
+    cols = _compute_cols(hooks, args.verbose)
     filenames = _all_filenames(args)
     filenames = _filter_by_include_exclude(filenames, '', config['exclude'])
     retval = 0
-    for repo, hook in repo_hooks:
+    for hook in hooks:
-        retval |= _run_single_hook(filenames, hook, repo, args, skips, cols)
+        retval |= _run_single_hook(filenames, hook, args, skips, cols)
         if retval and config['fail_fast']:
             break
     if (
@@ -252,28 +251,18 @@ def run(config_file, store, args, environ=os.environ):
         ctx = staged_files_only(store.directory)
 
     with ctx:
-        repo_hooks = []
         config = load_config(config_file)
-        for repo in repositories(config, store):
+        hooks = [
-            for _, hook in repo.hooks:
+            hook
-                if (
+            for hook in all_hooks(config, store)
-                        (
+            if not args.hook or hook.id == args.hook or hook.alias == args.hook
-                            not args.hook or
+            if not hook.stages or args.hook_stage in hook.stages
-                            hook['id'] == args.hook or
+        ]
-                            hook['alias'] == args.hook
-                        ) and
-                        (
-                            not hook['stages'] or
-                            args.hook_stage in hook['stages']
-                        )
-                ):
-                    repo_hooks.append((repo, hook))
 
-        if args.hook and not repo_hooks:
+        if args.hook and not hooks:
             output.write_line('No hook with id `{}`'.format(args.hook))
             return 1
 
-        for repo in {repo for repo, _ in repo_hooks}:
+        install_hook_envs(hooks, store)
-            repo.require_installed()
 
-        return _run_hooks(config, repo_hooks, args, environ)
+        return _run_hooks(config, hooks, args, environ)

pre_commit/meta_hooks/check_hooks_apply.py

@@ -5,24 +5,32 @@ from pre_commit import git
 from pre_commit.clientlib import load_config
 from pre_commit.commands.run import _filter_by_include_exclude
 from pre_commit.commands.run import _filter_by_types
-from pre_commit.repository import repositories
+from pre_commit.meta_hooks.helpers import make_meta_entry
+from pre_commit.repository import all_hooks
 from pre_commit.store import Store
 
+HOOK_DICT = {
+    'id': 'check-hooks-apply',
+    'name': 'Check hooks apply to the repository',
+    'files': C.CONFIG_FILE,
+    'language': 'system',
+    'entry': make_meta_entry(__name__),
+}
+
 
 def check_all_hooks_match_files(config_file):
     files = git.get_all_files()
     retv = 0
 
-    for repo in repositories(load_config(config_file), Store()):
+    for hook in all_hooks(load_config(config_file), Store()):
-        for hook_id, hook in repo.hooks:
+        if hook.always_run or hook.language == 'fail':
-            if hook['always_run'] or hook['language'] == 'fail':
             continue
-            include, exclude = hook['files'], hook['exclude']
+        include, exclude = hook.files, hook.exclude
         filtered = _filter_by_include_exclude(files, include, exclude)
-            types, exclude_types = hook['types'], hook['exclude_types']
+        types, exclude_types = hook.types, hook.exclude_types
         filtered = _filter_by_types(filtered, types, exclude_types)
         if not filtered:
-                print('{} does not apply to this repository'.format(hook_id))
+            print('{} does not apply to this repository'.format(hook.id))
             retv = 1
 
     return retv

pre_commit/meta_hooks/check_useless_excludes.py

@@ -10,6 +10,15 @@ from pre_commit import git
 from pre_commit.clientlib import load_config
 from pre_commit.clientlib import MANIFEST_HOOK_DICT
 from pre_commit.commands.run import _filter_by_types
+from pre_commit.meta_hooks.helpers import make_meta_entry
+
+HOOK_DICT = {
+    'id': 'check-useless-excludes',
+    'name': 'Check for useless excludes',
+    'files': C.CONFIG_FILE,
+    'language': 'system',
+    'entry': make_meta_entry(__name__),
+}
 
 
 def exclude_matches_any(filenames, include, exclude):

pre_commit/meta_hooks/helpers.py

@@ -0,0 +1,10 @@
+import pipes
+import sys
+
+
+def make_meta_entry(modname):
+    """the hook `entry` is passed through `shlex.split()` by the command
+    runner, so to prevent issues with spaces and backslashes (on Windows)
+    it must be quoted here.
+    """
+    return '{} -m {}'.format(pipes.quote(sys.executable), modname)

pre_commit/meta_hooks/identity.py

@@ -1,6 +1,15 @@
 import sys
 
 from pre_commit import output
+from pre_commit.meta_hooks.helpers import make_meta_entry
+
+HOOK_DICT = {
+    'id': 'identity',
+    'name': 'identity',
+    'language': 'system',
+    'verbose': True,
+    'entry': make_meta_entry(__name__),
+}
 
 
 def main(argv=None):

pre_commit/repository.py

@@ -1,20 +1,16 @@
 from __future__ import unicode_literals
 
+import collections
 import io
 import json
 import logging
 import os
-import pipes
-import shutil
-import sys
 
-from cached_property import cached_property
 from cfgv import apply_defaults
 from cfgv import validate
 
 import pre_commit.constants as C
 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
@@ -23,6 +19,7 @@ from pre_commit.languages.all import languages
 from pre_commit.languages.helpers import environment_dir
 from pre_commit.prefix import Prefix
 from pre_commit.util import parse_version
+from pre_commit.util import rmtree
 
 
 logger = logging.getLogger('pre_commit')
@@ -33,9 +30,7 @@ def _state(additional_deps):
 
 
 def _state_filename(prefix, venv):
-    return prefix.path(
+    return prefix.path(venv, '.install_state_v' + C.INSTALLED_STATE_VERSION)
-        venv, '.install_state_v' + C.INSTALLED_STATE_VERSION,
-    )
 
 
 def _read_state(prefix, venv):
@@ -44,7 +39,7 @@ def _read_state(prefix, venv):
         return None
     else:
         with io.open(filename) as f:
-            return json.loads(f.read())
+            return json.load(f)
 
 
 def _write_state(prefix, venv, state):
@@ -56,53 +51,67 @@ def _write_state(prefix, venv, state):
     os.rename(staging, state_filename)
 
 
-def _installed(prefix, language_name, language_version, additional_deps):
+_KEYS = tuple(item.key for item in MANIFEST_HOOK_DICT.items)
-    language = languages[language_name]
+
-    venv = environment_dir(language.ENVIRONMENT_DIR, language_version)
+
+class Hook(collections.namedtuple('Hook', ('src', 'prefix') + _KEYS)):
+    __slots__ = ()
+
+    @property
+    def install_key(self):
+        return (
+            self.prefix,
+            self.language,
+            self.language_version,
+            tuple(self.additional_dependencies),
+        )
+
+    def installed(self):
+        lang = languages[self.language]
+        venv = environment_dir(lang.ENVIRONMENT_DIR, self.language_version)
         return (
             venv is None or (
-            _read_state(prefix, venv) == _state(additional_deps) and
+                (
-            language.healthy(prefix, language_version)
+                    _read_state(self.prefix, venv) ==
+                    _state(self.additional_dependencies)
+                ) and
+                lang.healthy(self.prefix, self.language_version)
             )
         )
 
-
+    def install(self):
-def _install_all(venvs, repo_url, store):
+        logger.info('Installing environment for {}.'.format(self.src))
-    """Tuple of (prefix, language, version, deps)"""
-    def _need_installed():
-        return tuple(
-            (prefix, language_name, version, deps)
-            for prefix, language_name, version, deps in venvs
-            if not _installed(prefix, language_name, version, deps)
-        )
-
-    if not _need_installed():
-        return
-    with store.exclusive_lock():
-        # Another process may have already completed this work
-        need_installed = _need_installed()
-        if not need_installed:  # pragma: no cover (race)
-            return
-
-        logger.info(
-            'Installing environment for {}.'.format(repo_url),
-        )
         logger.info('Once installed this environment will be reused.')
         logger.info('This may take a few minutes...')
 
-        for prefix, language_name, version, deps in need_installed:
+        lang = languages[self.language]
-            language = languages[language_name]
+        venv = environment_dir(lang.ENVIRONMENT_DIR, self.language_version)
-            venv = environment_dir(language.ENVIRONMENT_DIR, version)
 
         # There's potentially incomplete cleanup from previous runs
         # Clean it up!
-            if prefix.exists(venv):
+        if self.prefix.exists(venv):
-                shutil.rmtree(prefix.path(venv))
+            rmtree(self.prefix.path(venv))
 
-            language.install_environment(prefix, version, deps)
+        lang.install_environment(
+            self.prefix, self.language_version, self.additional_dependencies,
+        )
         # Write our state to indicate we're installed
-            state = _state(deps)
+        _write_state(self.prefix, venv, _state(self.additional_dependencies))
-            _write_state(prefix, venv, state)
+
+    def run(self, file_args):
+        lang = languages[self.language]
+        return lang.run_hook(self.prefix, self._asdict(), file_args)
+
+    @classmethod
+    def create(cls, src, prefix, dct):
+        # TODO: have cfgv do this (?)
+        extra_keys = set(dct) - set(_KEYS)
+        if extra_keys:
+            logger.warning(
+                'Unexpected keys present on {} => {}: '
+                '{}'.format(src, dct['id'], ', '.join(sorted(extra_keys))),
+            )
+        return cls(src=src, prefix=prefix, **{k: dct[k] for k in _KEYS})
 
 
 def _hook(*hook_dicts):
@@ -129,156 +138,47 @@ def _hook(*hook_dicts):
 
 
 def _hook_from_manifest_dct(dct):
-    dct = validate(apply_defaults(dct, MANIFEST_HOOK_DICT), MANIFEST_HOOK_DICT)
+    dct = apply_defaults(dct, MANIFEST_HOOK_DICT)
+    dct = validate(dct, MANIFEST_HOOK_DICT)
     dct = _hook(dct)
     return dct
 
 
-class Repository(object):
+def _local_repository_hooks(repo_config, store):
-    def __init__(self, repo_config, store):
+    def _local_prefix(language_name, deps):
-        self.repo_config = repo_config
-        self.store = store
-        self.__installed = False
-
-    @classmethod
-    def create(cls, config, store):
-        if is_local_repo(config):
-            return LocalRepository(config, store)
-        elif is_meta_repo(config):
-            return MetaRepository(config, store)
-        else:
-            return cls(config, store)
-
-    @cached_property
-    def manifest_hooks(self):
-        repo, rev = self.repo_config['repo'], self.repo_config['rev']
-        repo_path = self.store.clone(repo, rev)
-        manifest_path = os.path.join(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:
-                logger.error(
-                    '`{}` is not present in repository {}.  '
-                    'Typo? Perhaps it is introduced in a newer version?  '
-                    'Often `pre-commit autoupdate` fixes this.'.format(
-                        hook['id'], self.repo_config['repo'],
-                    ),
-                )
-                exit(1)
-
-        return tuple(
-            (hook['id'], _hook(self.manifest_hooks[hook['id']], hook))
-            for hook in self.repo_config['hooks']
-        )
-
-    def _prefix_from_deps(self, language_name, deps):
-        repo, rev = self.repo_config['repo'], self.repo_config['rev']
-        return Prefix(self.store.clone(repo, rev, deps))
-
-    def _venvs(self):
-        ret = set()
-        for _, hook in self.hooks:
-            language = hook['language']
-            version = hook['language_version']
-            deps = tuple(hook['additional_dependencies'])
-            ret.add((
-                self._prefix_from_deps(language, deps),
-                language, version, deps,
-            ))
-        return tuple(ret)
-
-    def require_installed(self):
-        if not self.__installed:
-            _install_all(self._venvs(), self.repo_config['repo'], self.store)
-            self.__installed = True
-
-    def run_hook(self, hook, file_args):
-        """Run a hook.
-
-        :param dict hook:
-        :param tuple file_args: all the files to run the hook on
-        """
-        self.require_installed()
-        language_name = hook['language']
-        deps = hook['additional_dependencies']
-        prefix = self._prefix_from_deps(language_name, deps)
-        return languages[language_name].run_hook(prefix, hook, file_args)
-
-
-class LocalRepository(Repository):
-    def _prefix_from_deps(self, language_name, deps):
-        """local repositories have a prefix per hook"""
         language = languages[language_name]
         # pcre / pygrep / script / system / docker_image do not have
         # environments so they work out of the current directory
         if language.ENVIRONMENT_DIR is None:
-            return Prefix(git.get_root())
+            return Prefix(os.getcwd())
         else:
-            return Prefix(self.store.make_local(deps))
+            return Prefix(store.make_local(deps))
 
-    @property
+    hook_dcts = [_hook_from_manifest_dct(h) for h in repo_config['hooks']]
-    def manifest(self):
-        raise NotImplementedError
-
-    @cached_property
-    def hooks(self):
     return tuple(
-            (hook['id'], _hook_from_manifest_dct(hook))
+        Hook.create(
-            for hook in self.repo_config['hooks']
+            repo_config['repo'],
+            _local_prefix(hook['language'], hook['additional_dependencies']),
+            hook,
+        )
+        for hook in hook_dcts
     )
 
 
-class MetaRepository(LocalRepository):
+def _meta_repository_hooks(repo_config, store):
-    @cached_property
+    # imported here to prevent circular imports.
-    def manifest_hooks(self):
-        # The hooks are imported here to prevent circular imports.
     from pre_commit.meta_hooks import check_hooks_apply
     from pre_commit.meta_hooks import check_useless_excludes
     from pre_commit.meta_hooks import identity
 
-        def _make_entry(mod):
-            """the hook `entry` is passed through `shlex.split()` by the
-            command runner, so to prevent issues with spaces and backslashes
-            (on Windows) it must be quoted here.
-            """
-            return '{} -m {}'.format(pipes.quote(sys.executable), mod.__name__)
-
     meta_hooks = [
-            {
+        _hook_from_manifest_dct(mod.HOOK_DICT)
-                'id': 'check-hooks-apply',
+        for mod in (check_hooks_apply, check_useless_excludes, identity)
-                'name': 'Check hooks apply to the repository',
-                'files': C.CONFIG_FILE,
-                'language': 'system',
-                'entry': _make_entry(check_hooks_apply),
-            },
-            {
-                'id': 'check-useless-excludes',
-                'name': 'Check for useless excludes',
-                'files': C.CONFIG_FILE,
-                'language': 'system',
-                'entry': _make_entry(check_useless_excludes),
-            },
-            {
-                'id': 'identity',
-                'name': 'identity',
-                'language': 'system',
-                'verbose': True,
-                'entry': _make_entry(identity),
-            },
     ]
+    by_id = {hook['id']: hook for hook in meta_hooks}
 
-        return {
+    for hook in repo_config['hooks']:
-            hook['id']: _hook_from_manifest_dct(hook)
+        if hook['id'] not in by_id:
-            for hook in meta_hooks
-        }
-
-    @cached_property
-    def hooks(self):
-        for hook in self.repo_config['hooks']:
-            if hook['id'] not in self.manifest_hooks:
             logger.error(
                 '`{}` is not a valid meta hook.  '
                 'Typo? Perhaps it is introduced in a newer version?  '
@@ -287,11 +187,77 @@ class MetaRepository(LocalRepository):
             )
             exit(1)
 
+    prefix = Prefix(os.getcwd())
     return tuple(
-            (hook['id'], _hook(self.manifest_hooks[hook['id']], hook))
+        Hook.create(
-            for hook in self.repo_config['hooks']
+            repo_config['repo'],
+            prefix,
+            _hook(by_id[hook['id']], hook),
+        )
+        for hook in repo_config['hooks']
     )
 
 
-def repositories(config, store):
+def _cloned_repository_hooks(repo_config, store):
-    return tuple(Repository.create(x, store) for x in config['repos'])
+    repo, rev = repo_config['repo'], repo_config['rev']
+    manifest_path = os.path.join(store.clone(repo, rev), C.MANIFEST_FILE)
+    by_id = {hook['id']: hook for hook in load_manifest(manifest_path)}
+
+    for hook in repo_config['hooks']:
+        if hook['id'] not in by_id:
+            logger.error(
+                '`{}` is not present in repository {}.  '
+                'Typo? Perhaps it is introduced in a newer version?  '
+                'Often `pre-commit autoupdate` fixes this.'
+                .format(hook['id'], repo),
+            )
+            exit(1)
+
+    hook_dcts = [_hook(by_id[h['id']], h) for h in repo_config['hooks']]
+    return tuple(
+        Hook.create(
+            repo_config['repo'],
+            Prefix(store.clone(repo, rev, hook['additional_dependencies'])),
+            hook,
+        )
+        for hook in hook_dcts
+    )
+
+
+def repository_hooks(repo_config, store):
+    if is_local_repo(repo_config):
+        return _local_repository_hooks(repo_config, store)
+    elif is_meta_repo(repo_config):
+        return _meta_repository_hooks(repo_config, store)
+    else:
+        return _cloned_repository_hooks(repo_config, store)
+
+
+def install_hook_envs(hooks, store):
+    def _need_installed():
+        seen = set()
+        ret = []
+        for hook in hooks:
+            if hook.install_key not in seen and not hook.installed():
+                ret.append(hook)
+            seen.add(hook.install_key)
+        return ret
+
+    if not _need_installed():
+        return
+    with store.exclusive_lock():
+        # Another process may have already completed this work
+        need_installed = _need_installed()
+        if not need_installed:  # pragma: no cover (race)
+            return
+
+        for hook in need_installed:
+            hook.install()
+
+
+def all_hooks(config, store):
+    return tuple(
+        hook
+        for repo in config['repos']
+        for hook in repository_hooks(repo, store)
+    )

setup.py

@@ -36,7 +36,6 @@ setup(
     },
     install_requires=[
         'aspy.yaml',
-        'cached-property',
         'cfgv>=1.0.0',
         'identify>=1.0.0',
         # if this makes it into python3.8 move to extras_require

testing/resources/ruby_versioned_hooks_repo/.pre-commit-hooks.yaml

@@ -2,5 +2,5 @@
     name: Ruby Hook
     entry: ruby_hook
     language: ruby
-    language_version: 2.1.5
+    language_version: 2.5.1
     files: \.rb$

tests/commands/run_test.py

@@ -18,6 +18,7 @@ from pre_commit.commands.run import _has_unmerged_paths
 from pre_commit.commands.run import run
 from pre_commit.util import cmd_output
 from pre_commit.util import make_executable
+from testing.auto_namedtuple import auto_namedtuple
 from testing.fixtures import add_config_to_repo
 from testing.fixtures import make_consuming_repo
 from testing.fixtures import modify_config
@@ -362,10 +363,13 @@ def test_merge_conflict_resolved(cap_out, store, in_merge_conflict):
     ('hooks', 'verbose', 'expected'),
     (
         ([], True, 80),
-        ([{'id': 'a', 'name': 'a' * 51}], False, 81),
+        ([auto_namedtuple(id='a', name='a' * 51)], False, 81),
-        ([{'id': 'a', 'name': 'a' * 51}], True, 85),
+        ([auto_namedtuple(id='a', name='a' * 51)], True, 85),
         (
-            [{'id': 'a', 'name': 'a' * 51}, {'id': 'b', 'name': 'b' * 52}],
+            [
+                auto_namedtuple(id='a', name='a' * 51),
+                auto_namedtuple(id='b', name='b' * 52),
+            ],
             False,
             82,
         ),

tests/repository_test.py

@@ -20,9 +20,11 @@ from pre_commit.languages import pcre
 from pre_commit.languages import python
 from pre_commit.languages import ruby
 from pre_commit.languages import rust
-from pre_commit.repository import Repository
+from pre_commit.prefix import Prefix
+from pre_commit.repository import Hook
+from pre_commit.repository import install_hook_envs
+from pre_commit.repository import repository_hooks
 from pre_commit.util import cmd_output
-from testing.fixtures import config_with_local_hooks
 from testing.fixtures import make_config_from_repo
 from testing.fixtures import make_repo
 from testing.fixtures import modify_manifest
@@ -40,6 +42,13 @@ def _norm_out(b):
     return b.replace(b'\r\n', b'\n')
 
 
+def _get_hook(config, store, hook_id):
+    hooks = repository_hooks(config, store)
+    install_hook_envs(hooks, store)
+    hook, = [hook for hook in hooks if hook.id == hook_id]
+    return hook
+
+
 def _test_hook_repo(
         tempdir_factory,
         store,
@@ -52,11 +61,7 @@ def _test_hook_repo(
 ):
     path = make_repo(tempdir_factory, repo_path)
     config = make_config_from_repo(path, **(config_kwargs or {}))
-    repo = Repository.create(config, store)
+    ret = _get_hook(config, store, hook_id).run(args)
-    hook_dict, = [
-        hook for repo_hook_id, hook in repo.hooks if repo_hook_id == hook_id
-    ]
-    ret = repo.run_hook(hook_dict, args)
     assert ret[0] == expected_return_code
     assert _norm_out(ret[1]) == expected
 
@@ -118,16 +123,9 @@ def test_switch_language_versions_doesnt_clobber(tempdir_factory, store):
     path = make_repo(tempdir_factory, 'python3_hooks_repo')
 
     def run_on_version(version, expected_output):
-        config = make_config_from_repo(
+        config = make_config_from_repo(path)
-            path, hooks=[{'id': 'python3-hook', 'language_version': version}],
+        config['hooks'][0]['language_version'] = version
-        )
+        ret = _get_hook(config, store, 'python3-hook').run([])
-        repo = Repository.create(config, store)
-        hook_dict, = [
-            hook
-            for repo_hook_id, hook in repo.hooks
-            if repo_hook_id == 'python3-hook'
-        ]
-        ret = repo.run_hook(hook_dict, [])
         assert ret[0] == 0
         assert _norm_out(ret[1]) == expected_output
 
@@ -212,7 +210,7 @@ def test_run_versioned_ruby_hook(tempdir_factory, store):
         tempdir_factory, store, 'ruby_versioned_hooks_repo',
         'ruby_hook',
         [os.devnull],
-        b'2.1.5\nHello world from a ruby hook\n',
+        b'2.5.1\nHello world from a ruby hook\n',
     )
 
 
@@ -234,7 +232,7 @@ def test_run_ruby_hook_with_disable_shared_gems(
             tempdir_factory, store, 'ruby_versioned_hooks_repo',
             'ruby_hook',
             [os.devnull],
-            b'2.1.5\nHello world from a ruby hook\n',
+            b'2.5.1\nHello world from a ruby hook\n',
         )
 
 
@@ -275,10 +273,8 @@ def test_additional_rust_cli_dependencies_installed(
     config = make_config_from_repo(path)
     # A small rust package with no dependencies.
     config['hooks'][0]['additional_dependencies'] = [dep]
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'rust-hook')
-    repo.require_installed()
+    binaries = os.listdir(hook.prefix.path(
-    (prefix, _, _, _), = repo._venvs()
-    binaries = os.listdir(prefix.path(
         helpers.environment_dir(rust.ENVIRONMENT_DIR, 'default'), 'bin',
     ))
     # normalize for windows
@@ -294,10 +290,8 @@ def test_additional_rust_lib_dependencies_installed(
     # A small rust package with no dependencies.
     deps = ['shellharden:3.1.0']
     config['hooks'][0]['additional_dependencies'] = deps
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'rust-hook')
-    repo.require_installed()
+    binaries = os.listdir(hook.prefix.path(
-    (prefix, _, _, _), = repo._venvs()
-    binaries = os.listdir(prefix.path(
         helpers.environment_dir(rust.ENVIRONMENT_DIR, 'default'), 'bin',
     ))
     # normalize for windows
@@ -362,9 +356,7 @@ def _make_grep_repo(language, entry, store, args=()):
             ],
         ),
     ))
-    repo = Repository.create(config, store)
+    return _get_hook(config, store, 'grep-hook')
-    (_, hook), = repo.hooks
-    return repo, hook
 
 
 @pytest.fixture
@@ -381,21 +373,21 @@ class TestPygrep(object):
     language = 'pygrep'
 
     def test_grep_hook_matching(self, greppable_files, store):
-        repo, hook = _make_grep_repo(self.language, 'ello', store)
+        hook = _make_grep_repo(self.language, 'ello', store)
-        ret, out, _ = repo.run_hook(hook, ('f1', 'f2', 'f3'))
+        ret, out, _ = hook.run(('f1', 'f2', 'f3'))
         assert ret == 1
         assert _norm_out(out) == b"f1:1:hello'hi\n"
 
     def test_grep_hook_case_insensitive(self, greppable_files, store):
-        repo, hook = _make_grep_repo(self.language, 'ELLO', store, args=['-i'])
+        hook = _make_grep_repo(self.language, 'ELLO', store, args=['-i'])
-        ret, out, _ = repo.run_hook(hook, ('f1', 'f2', 'f3'))
+        ret, out, _ = hook.run(('f1', 'f2', 'f3'))
         assert ret == 1
         assert _norm_out(out) == b"f1:1:hello'hi\n"
 
     @pytest.mark.parametrize('regex', ('nope', "foo'bar", r'^\[INFO\]'))
     def test_grep_hook_not_matching(self, regex, greppable_files, store):
-        repo, hook = _make_grep_repo(self.language, regex, store)
+        hook = _make_grep_repo(self.language, regex, store)
-        ret, out, _ = repo.run_hook(hook, ('f1', 'f2', 'f3'))
+        ret, out, _ = hook.run(('f1', 'f2', 'f3'))
         assert (ret, out) == (0, b'')
 
 
@@ -408,23 +400,19 @@ class TestPCRE(TestPygrep):
         # This is intended to simulate lots of passing files and one failing
         # file to make sure it still fails.  This is not the case when naively
         # using a system hook with `grep -H -n '...'`
-        repo, hook = _make_grep_repo('pcre', 'ello', store)
+        hook = _make_grep_repo('pcre', 'ello', store)
-        ret, out, _ = repo.run_hook(hook, (os.devnull,) * 15000 + ('f1',))
+        ret, out, _ = hook.run((os.devnull,) * 15000 + ('f1',))
         assert ret == 1
         assert _norm_out(out) == b"f1:1:hello'hi\n"
 
     def test_missing_pcre_support(self, greppable_files, store):
-        orig_find_executable = parse_shebang.find_executable
-
         def no_grep(exe, **kwargs):
-            if exe == pcre.GREP:
+            assert exe == pcre.GREP
             return None
-            else:
-                return orig_find_executable(exe, **kwargs)
 
         with mock.patch.object(parse_shebang, 'find_executable', no_grep):
-            repo, hook = _make_grep_repo('pcre', 'ello', store)
+            hook = _make_grep_repo('pcre', 'ello', store)
-            ret, out, _ = repo.run_hook(hook, ('f1', 'f2', 'f3'))
+            ret, out, _ = hook.run(('f1', 'f2', 'f3'))
             assert ret == 1
             expected = 'Executable `{}` not found'.format(pcre.GREP).encode()
             assert out == expected
@@ -454,44 +442,23 @@ def test_lots_of_files(tempdir_factory, store):
     )
 
 
-def test_venvs(tempdir_factory, store):
-    path = make_repo(tempdir_factory, 'python_hooks_repo')
-    config = make_config_from_repo(path)
-    repo = Repository.create(config, store)
-    venv, = repo._venvs()
-    assert venv == (mock.ANY, 'python', python.get_default_version(), ())
-
-
-def test_additional_dependencies(tempdir_factory, store):
-    path = make_repo(tempdir_factory, 'python_hooks_repo')
-    config = make_config_from_repo(path)
-    config['hooks'][0]['additional_dependencies'] = ['pep8']
-    repo = Repository.create(config, store)
-    env, = repo._venvs()
-    assert env == (mock.ANY, 'python', python.get_default_version(), ('pep8',))
-
-
 def test_additional_dependencies_roll_forward(tempdir_factory, store):
     path = make_repo(tempdir_factory, 'python_hooks_repo')
 
     config1 = make_config_from_repo(path)
-    repo1 = Repository.create(config1, store)
+    hook1 = _get_hook(config1, store, 'foo')
-    repo1.require_installed()
+    with python.in_env(hook1.prefix, hook1.language_version):
-    (prefix1, _, version1, _), = repo1._venvs()
-    with python.in_env(prefix1, version1):
         assert 'mccabe' not in cmd_output('pip', 'freeze', '-l')[1]
 
     # Make another repo with additional dependencies
     config2 = make_config_from_repo(path)
     config2['hooks'][0]['additional_dependencies'] = ['mccabe']
-    repo2 = Repository.create(config2, store)
+    hook2 = _get_hook(config2, store, 'foo')
-    repo2.require_installed()
+    with python.in_env(hook2.prefix, hook2.language_version):
-    (prefix2, _, version2, _), = repo2._venvs()
-    with python.in_env(prefix2, version2):
         assert 'mccabe' in cmd_output('pip', 'freeze', '-l')[1]
 
     # should not have affected original
-    with python.in_env(prefix1, version1):
+    with python.in_env(hook1.prefix, hook1.language_version):
         assert 'mccabe' not in cmd_output('pip', 'freeze', '-l')[1]
 
 
@@ -499,13 +466,10 @@ def test_additional_dependencies_roll_forward(tempdir_factory, store):
 def test_additional_ruby_dependencies_installed(tempdir_factory, store):
     path = make_repo(tempdir_factory, 'ruby_hooks_repo')
     config = make_config_from_repo(path)
-    config['hooks'][0]['additional_dependencies'] = ['thread_safe', 'tins']
+    config['hooks'][0]['additional_dependencies'] = ['tins']
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'ruby_hook')
-    repo.require_installed()
+    with ruby.in_env(hook.prefix, hook.language_version):
-    (prefix, _, version, _), = repo._venvs()
-    with ruby.in_env(prefix, version):
         output = cmd_output('gem', 'list', '--local')[1]
-        assert 'thread_safe' in output
         assert 'tins' in output
 
 
@@ -515,10 +479,8 @@ def test_additional_node_dependencies_installed(tempdir_factory, store):
     config = make_config_from_repo(path)
     # Careful to choose a small package that's not depped by npm
     config['hooks'][0]['additional_dependencies'] = ['lodash']
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'foo')
-    repo.require_installed()
+    with node.in_env(hook.prefix, hook.language_version):
-    (prefix, _, version, _), = repo._venvs()
-    with node.in_env(prefix, version):
         output = cmd_output('npm', 'ls', '-g')[1]
         assert 'lodash' in output
 
@@ -531,10 +493,8 @@ def test_additional_golang_dependencies_installed(
     # A small go package
     deps = ['github.com/golang/example/hello']
     config['hooks'][0]['additional_dependencies'] = deps
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'golang-hook')
-    repo.require_installed()
+    binaries = os.listdir(hook.prefix.path(
-    (prefix, _, _, _), = repo._venvs()
-    binaries = os.listdir(prefix.path(
         helpers.environment_dir(golang.ENVIRONMENT_DIR, 'default'), 'bin',
     ))
     # normalize for windows
@@ -553,9 +513,7 @@ def test_local_golang_additional_dependencies(store):
             'additional_dependencies': ['github.com/golang/example/hello'],
         }],
     }
-    repo = Repository.create(config, store)
+    ret = _get_hook(config, store, 'hello').run(())
-    (_, hook), = repo.hooks
-    ret = repo.run_hook(hook, ('filename',))
     assert ret[0] == 0
     assert _norm_out(ret[1]) == b"Hello, Go examples!\n"
 
@@ -571,9 +529,7 @@ def test_local_rust_additional_dependencies(store):
             'additional_dependencies': ['cli:hello-cli:0.2.2'],
         }],
     }
-    repo = Repository.create(config, store)
+    ret = _get_hook(config, store, 'hello').run(())
-    (_, hook), = repo.hooks
-    ret = repo.run_hook(hook, ())
     assert ret[0] == 0
     assert _norm_out(ret[1]) == b"Hello World!\n"
 
@@ -589,9 +545,8 @@ def test_fail_hooks(store):
             'files': r'changelog/.*(?<!\.rst)$',
         }],
     }
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'fail')
-    (_, hook), = repo.hooks
+    ret = hook.run(('changelog/1234.bugfix', 'changelog/wat'))
-    ret = repo.run_hook(hook, ('changelog/1234.bugfix', 'changelog/wat'))
     assert ret[0] == 1
     assert ret[1] == (
         b'make sure to name changelogs as .rst!\n'
@@ -601,20 +556,32 @@ def test_fail_hooks(store):
     )
 
 
+def test_unknown_keys(store, fake_log_handler):
+    config = {
+        'repo': 'local',
+        'hooks': [{
+            'id': 'too-much',
+            'name': 'too much',
+            'hello': 'world',
+            'foo': 'bar',
+            'language': 'system',
+            'entry': 'true',
+        }],
+    }
+    _get_hook(config, store, 'too-much')
+    expected = 'Unexpected keys present on local => too-much: foo, hello'
+    assert fake_log_handler.handle.call_args[0][0].msg == expected
+
+
 def test_reinstall(tempdir_factory, store, log_info_mock):
     path = make_repo(tempdir_factory, 'python_hooks_repo')
     config = make_config_from_repo(path)
-    repo = Repository.create(config, store)
+    _get_hook(config, store, 'foo')
-    repo.require_installed()
     # We print some logging during clone (1) + install (3)
     assert log_info_mock.call_count == 4
     log_info_mock.reset_mock()
-    # Reinstall with same repo should not trigger another install
-    repo.require_installed()
-    assert log_info_mock.call_count == 0
     # Reinstall on another run should not trigger another install
-    repo = Repository.create(config, store)
+    _get_hook(config, store, 'foo')
-    repo.require_installed()
     assert log_info_mock.call_count == 0
 
 
@@ -622,8 +589,7 @@ def test_control_c_control_c_on_install(tempdir_factory, store):
     """Regression test for #186."""
     path = make_repo(tempdir_factory, 'python_hooks_repo')
     config = make_config_from_repo(path)
-    repo = Repository.create(config, store)
+    hooks = repository_hooks(config, store)
-    hook = repo.hooks[0][1]
 
     class MyKeyboardInterrupt(KeyboardInterrupt):
         pass
@@ -638,16 +604,18 @@ def test_control_c_control_c_on_install(tempdir_factory, store):
             with mock.patch.object(
                 shutil, 'rmtree', side_effect=MyKeyboardInterrupt,
             ):
-                repo.run_hook(hook, [])
+                install_hook_envs(hooks, store)
 
     # Should have made an environment, however this environment is broken!
-    (prefix, _, version, _), = repo._venvs()
+    hook, = hooks
-    envdir = 'py_env-{}'.format(version)
+    assert hook.prefix.exists(
-    assert prefix.exists(envdir)
+        helpers.environment_dir(python.ENVIRONMENT_DIR, hook.language_version),
+    )
 
     # However, it should be perfectly runnable (reinstall after botched
     # install)
-    retv, stdout, stderr = repo.run_hook(hook, [])
+    install_hook_envs(hooks, store)
+    retv, stdout, stderr = hook.run(())
     assert retv == 0
 
 
@@ -656,21 +624,20 @@ def test_invalidated_virtualenv(tempdir_factory, store):
     # This should not cause every hook in that virtualenv to fail.
     path = make_repo(tempdir_factory, 'python_hooks_repo')
     config = make_config_from_repo(path)
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'foo')
 
     # Simulate breaking of the virtualenv
-    repo.require_installed()
+    libdir = hook.prefix.path(
-    (prefix, _, version, _), = repo._venvs()
+        helpers.environment_dir(python.ENVIRONMENT_DIR, hook.language_version),
-    libdir = prefix.path('py_env-{}'.format(version), 'lib', version)
+        'lib', hook.language_version,
+    )
     paths = [
         os.path.join(libdir, p) for p in ('site.py', 'site.pyc', '__pycache__')
     ]
     cmd_output('rm', '-rf', *paths)
 
     # pre-commit should rebuild the virtualenv and it should be runnable
-    repo = Repository.create(config, store)
+    retv, stdout, stderr = _get_hook(config, store, 'foo').run(())
-    hook = repo.hooks[0][1]
-    retv, stdout, stderr = repo.run_hook(hook, [])
     assert retv == 0
 
 
@@ -683,57 +650,41 @@ def test_really_long_file_paths(tempdir_factory, store):
     config = make_config_from_repo(path)
 
     with cwd(really_long_path):
-        repo = Repository.create(config, store)
+        _get_hook(config, store, 'foo')
-        repo.require_installed()
 
 
 def test_config_overrides_repo_specifics(tempdir_factory, store):
     path = make_repo(tempdir_factory, 'script_hooks_repo')
     config = make_config_from_repo(path)
 
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'bash_hook')
-    assert repo.hooks[0][1]['files'] == ''
+    assert hook.files == ''
     # Set the file regex to something else
     config['hooks'][0]['files'] = '\\.sh$'
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'bash_hook')
-    assert repo.hooks[0][1]['files'] == '\\.sh$'
+    assert hook.files == '\\.sh$'
 
 
 def _create_repo_with_tags(tempdir_factory, src, tag):
     path = make_repo(tempdir_factory, src)
-    with cwd(path):
+    cmd_output('git', 'tag', tag, cwd=path)
-        cmd_output('git', 'tag', tag)
     return path
 
 
 def test_tags_on_repositories(in_tmpdir, tempdir_factory, store):
     tag = 'v1.1'
-    git_dir_1 = _create_repo_with_tags(tempdir_factory, 'prints_cwd_repo', tag)
+    git1 = _create_repo_with_tags(tempdir_factory, 'prints_cwd_repo', tag)
-    git_dir_2 = _create_repo_with_tags(
+    git2 = _create_repo_with_tags(tempdir_factory, 'script_hooks_repo', tag)
-        tempdir_factory, 'script_hooks_repo', tag,
-    )
 
-    repo_1 = Repository.create(
+    config1 = make_config_from_repo(git1, rev=tag)
-        make_config_from_repo(git_dir_1, rev=tag), store,
+    ret1 = _get_hook(config1, store, 'prints_cwd').run(('-L',))
-    )
+    assert ret1[0] == 0
-    ret = repo_1.run_hook(repo_1.hooks[0][1], ['-L'])
+    assert ret1[1].strip() == _norm_pwd(in_tmpdir)
-    assert ret[0] == 0
-    assert ret[1].strip() == _norm_pwd(in_tmpdir)
 
-    repo_2 = Repository.create(
+    config2 = make_config_from_repo(git2, rev=tag)
-        make_config_from_repo(git_dir_2, rev=tag), store,
+    ret2 = _get_hook(config2, store, 'bash_hook').run(('bar',))
-    )
+    assert ret2[0] == 0
-    ret = repo_2.run_hook(repo_2.hooks[0][1], ['bar'])
+    assert ret2[1] == b'bar\nHello World\n'
-    assert ret[0] == 0
-    assert ret[1] == b'bar\nHello World\n'
-
-
-def test_local_repository():
-    config = config_with_local_hooks()
-    local_repo = Repository.create(config, 'dummy')
-    with pytest.raises(NotImplementedError):
-        local_repo.manifest
-    assert len(local_repo.hooks) == 1
 
 
 def test_local_python_repo(store):
@@ -744,11 +695,10 @@ def test_local_python_repo(store):
         dict(hook, additional_dependencies=[repo_path]) for hook in manifest
     ]
     config = {'repo': 'local', 'hooks': hooks}
-    repo = Repository.create(config, store)
+    hook = _get_hook(config, store, 'foo')
-    (_, hook), = repo.hooks
     # language_version should have been adjusted to the interpreter version
-    assert hook['language_version'] != 'default'
+    assert hook.language_version != 'default'
-    ret = repo.run_hook(hook, ('filename',))
+    ret = hook.run(('filename',))
     assert ret[0] == 0
     assert _norm_out(ret[1]) == b"['filename']\nHello World\n"
 
@@ -757,9 +707,8 @@ def test_hook_id_not_present(tempdir_factory, store, fake_log_handler):
     path = make_repo(tempdir_factory, 'script_hooks_repo')
     config = make_config_from_repo(path)
     config['hooks'][0]['id'] = 'i-dont-exist'
-    repo = Repository.create(config, store)
     with pytest.raises(SystemExit):
-        repo.require_installed()
+        _get_hook(config, store, 'i-dont-exist')
     assert fake_log_handler.handle.call_args[0][0].msg == (
         '`i-dont-exist` is not present in repository file://{}.  '
         'Typo? Perhaps it is introduced in a newer version?  '
@@ -769,9 +718,8 @@ def test_hook_id_not_present(tempdir_factory, store, fake_log_handler):
 
 def test_meta_hook_not_present(store, fake_log_handler):
     config = {'repo': 'meta', 'hooks': [{'id': 'i-dont-exist'}]}
-    repo = Repository.create(config, store)
     with pytest.raises(SystemExit):
-        repo.require_installed()
+        _get_hook(config, store, 'i-dont-exist')
     assert fake_log_handler.handle.call_args[0][0].msg == (
         '`i-dont-exist` is not a valid meta hook.  '
         'Typo? Perhaps it is introduced in a newer version?  '
@@ -784,9 +732,8 @@ def test_too_new_version(tempdir_factory, store, fake_log_handler):
     with modify_manifest(path) as manifest:
         manifest[0]['minimum_pre_commit_version'] = '999.0.0'
     config = make_config_from_repo(path)
-    repo = Repository.create(config, store)
     with pytest.raises(SystemExit):
-        repo.require_installed()
+        _get_hook(config, store, 'bash_hook')
     msg = fake_log_handler.handle.call_args[0][0].msg
     assert re.match(
         r'^The hook `bash_hook` requires pre-commit version 999\.0\.0 but '
@@ -803,33 +750,35 @@ def test_versions_ok(tempdir_factory, store, version):
         manifest[0]['minimum_pre_commit_version'] = version
     config = make_config_from_repo(path)
     # Should succeed
-    Repository.create(config, store).require_installed()
+    _get_hook(config, store, 'bash_hook')
 
 
 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)
+    hook = _get_hook(config, store, 'bash_hook')
 
-    assert repo.manifest_hooks['bash_hook'] == {
+    assert hook == Hook(
-        'always_run': False,
+        src='file://{}'.format(path),
-        'additional_dependencies': [],
+        prefix=Prefix(mock.ANY),
-        'args': [],
+        additional_dependencies=[],
-        'description': '',
+        alias='',
-        'entry': 'bin/hook.sh',
+        always_run=False,
-        'exclude': '^$',
+        args=[],
-        'files': '',
+        description='',
-        'id': 'bash_hook',
+        entry='bin/hook.sh',
-        'alias': '',
+        exclude='^$',
-        'language': 'script',
+        exclude_types=[],
-        'language_version': 'default',
+        files='',
-        'log_file': '',
+        id='bash_hook',
-        'minimum_pre_commit_version': '0',
+        language='script',
-        'name': 'Bash hook',
+        language_version='default',
-        'pass_filenames': True,
+        log_file='',
-        'require_serial': False,
+        minimum_pre_commit_version='0',
-        'stages': [],
+        name='Bash hook',
-        'types': ['file'],
+        pass_filenames=True,
-        'exclude_types': [],
+        require_serial=False,
-        'verbose': False,
+        stages=[],
-    }
+        types=['file'],
+        verbose=False,
+    )