diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index aa237a5e..4fe852f7 100644 --- a/.pre-commit-config.yaml +++ b/.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: diff --git a/.travis.yml b/.travis.yml index e59e0717..32376b27 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,5 @@ language: python -dist: trusty -sudo: required +dist: xenial 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: diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index f40a7c55..f75a1924 100644 --- a/pre_commit/commands/autoupdate.py +++ b/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 diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 3e70b4c9..e27c5b2c 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/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): - repository.require_installed() + install_hook_envs(all_hooks(load_config(config_file), store), store) def uninstall(hook_type='pre-commit'): diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index d9280460..2b90e44e 100644 --- a/pre_commit/commands/run.py +++ b/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( - '[{}] '.format(hook['id']) if verbose else '', hook['name'], - ) + return '{}{}'.format('[{}] '.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): - include, exclude = hook['files'], hook['exclude'] +def _run_single_hook(filenames, hook, args, skips, cols): + 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( - hook, tuple(filenames) if hook['pass_filenames'] else (), + retcode, stdout, stderr = hook.run( + 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: - retval |= _run_single_hook(filenames, hook, repo, args, skips, cols) + for hook in hooks: + 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): - for _, hook in repo.hooks: - if ( - ( - not args.hook or - 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)) + hooks = [ + hook + for hook in all_hooks(config, store) + if not args.hook or hook.id == args.hook or hook.alias == args.hook + if not hook.stages or args.hook_stage in hook.stages + ] - 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}: - repo.require_installed() + install_hook_envs(hooks, store) - return _run_hooks(config, repo_hooks, args, environ) + return _run_hooks(config, hooks, args, environ) diff --git a/pre_commit/meta_hooks/check_hooks_apply.py b/pre_commit/meta_hooks/check_hooks_apply.py index 4c4719c8..a97830d2 100644 --- a/pre_commit/meta_hooks/check_hooks_apply.py +++ b/pre_commit/meta_hooks/check_hooks_apply.py @@ -5,25 +5,33 @@ 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_id, hook in repo.hooks: - if hook['always_run'] or hook['language'] == 'fail': - continue - include, exclude = hook['files'], hook['exclude'] - filtered = _filter_by_include_exclude(files, include, exclude) - 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)) - retv = 1 + for hook in all_hooks(load_config(config_file), Store()): + if hook.always_run or hook.language == 'fail': + continue + include, exclude = hook.files, hook.exclude + filtered = _filter_by_include_exclude(files, include, exclude) + 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)) + retv = 1 return retv diff --git a/pre_commit/meta_hooks/check_useless_excludes.py b/pre_commit/meta_hooks/check_useless_excludes.py index 18b9f163..7918eb31 100644 --- a/pre_commit/meta_hooks/check_useless_excludes.py +++ b/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): diff --git a/pre_commit/meta_hooks/helpers.py b/pre_commit/meta_hooks/helpers.py new file mode 100644 index 00000000..7ef74861 --- /dev/null +++ b/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) diff --git a/pre_commit/meta_hooks/identity.py b/pre_commit/meta_hooks/identity.py index ae7377b8..0cec32a0 100644 --- a/pre_commit/meta_hooks/identity.py +++ b/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): diff --git a/pre_commit/repository.py b/pre_commit/repository.py index e245a1a3..c9115dfd 100644 --- a/pre_commit/repository.py +++ b/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( - venv, '.install_state_v' + C.INSTALLED_STATE_VERSION, - ) + return prefix.path(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): - language = languages[language_name] - venv = environment_dir(language.ENVIRONMENT_DIR, language_version) - return ( - venv is None or ( - _read_state(prefix, venv) == _state(additional_deps) and - language.healthy(prefix, language_version) - ) - ) +_KEYS = tuple(item.key for item in MANIFEST_HOOK_DICT.items) -def _install_all(venvs, repo_url, store): - """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) +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), ) - 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), + def installed(self): + lang = languages[self.language] + venv = environment_dir(lang.ENVIRONMENT_DIR, self.language_version) + return ( + venv is None or ( + ( + _read_state(self.prefix, venv) == + _state(self.additional_dependencies) + ) and + lang.healthy(self.prefix, self.language_version) + ) ) + + def install(self): + logger.info('Installing environment for {}.'.format(self.src)) 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: - language = languages[language_name] - venv = environment_dir(language.ENVIRONMENT_DIR, version) + lang = languages[self.language] + venv = environment_dir(lang.ENVIRONMENT_DIR, self.language_version) - # There's potentially incomplete cleanup from previous runs - # Clean it up! - if prefix.exists(venv): - shutil.rmtree(prefix.path(venv)) + # There's potentially incomplete cleanup from previous runs + # Clean it up! + if self.prefix.exists(venv): + rmtree(self.prefix.path(venv)) - language.install_environment(prefix, version, deps) - # Write our state to indicate we're installed - state = _state(deps) - _write_state(prefix, venv, state) + lang.install_environment( + self.prefix, self.language_version, self.additional_dependencies, + ) + # Write our state to indicate we're installed + _write_state(self.prefix, venv, _state(self.additional_dependencies)) + + 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,169 +138,126 @@ 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 __init__(self, repo_config, store): - 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""" +def _local_repository_hooks(repo_config, store): + def _local_prefix(language_name, deps): 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 - def manifest(self): - raise NotImplementedError - - @cached_property - def hooks(self): - return tuple( - (hook['id'], _hook_from_manifest_dct(hook)) - for hook in self.repo_config['hooks'] + hook_dcts = [_hook_from_manifest_dct(h) for h in repo_config['hooks']] + return tuple( + Hook.create( + repo_config['repo'], + _local_prefix(hook['language'], hook['additional_dependencies']), + hook, ) + for hook in hook_dcts + ) -class MetaRepository(LocalRepository): - @cached_property - 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 _meta_repository_hooks(repo_config, store): + # 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) + for mod in (check_hooks_apply, check_useless_excludes, identity) + ] + by_id = {hook['id']: hook for hook in meta_hooks} - meta_hooks = [ - { - 'id': 'check-hooks-apply', - '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), - }, - ] + for hook in repo_config['hooks']: + if hook['id'] not in by_id: + logger.error( + '`{}` is not a valid meta hook. ' + 'Typo? Perhaps it is introduced in a newer version? ' + 'Often `pip install --upgrade pre-commit` fixes this.' + .format(hook['id']), + ) + exit(1) - return { - hook['id']: _hook_from_manifest_dct(hook) - 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? ' - 'Often `pip install --upgrade pre-commit` fixes this.' - .format(hook['id']), - ) - exit(1) - - return tuple( - (hook['id'], _hook(self.manifest_hooks[hook['id']], hook)) - for hook in self.repo_config['hooks'] + prefix = Prefix(os.getcwd()) + return tuple( + Hook.create( + repo_config['repo'], + prefix, + _hook(by_id[hook['id']], hook), ) + for hook in repo_config['hooks'] + ) -def repositories(config, store): - return tuple(Repository.create(x, store) for x in config['repos']) +def _cloned_repository_hooks(repo_config, store): + 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) + ) diff --git a/setup.py b/setup.py index edcd04ff..f6ea719c 100644 --- a/setup.py +++ b/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 diff --git a/testing/resources/ruby_versioned_hooks_repo/.pre-commit-hooks.yaml b/testing/resources/ruby_versioned_hooks_repo/.pre-commit-hooks.yaml index fcba780f..63e1dd4c 100644 --- a/testing/resources/ruby_versioned_hooks_repo/.pre-commit-hooks.yaml +++ b/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$ diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 28b6ab37..0345ea7d 100644 --- a/tests/commands/run_test.py +++ b/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), - ([{'id': 'a', 'name': 'a' * 51}], True, 85), + ([auto_namedtuple(id='a', name='a' * 51)], False, 81), + ([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, ), diff --git a/tests/repository_test.py b/tests/repository_test.py index 606bfe75..2092802b 100644 --- a/tests/repository_test.py +++ b/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) - hook_dict, = [ - hook for repo_hook_id, hook in repo.hooks if repo_hook_id == hook_id - ] - ret = repo.run_hook(hook_dict, args) + ret = _get_hook(config, store, hook_id).run(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( - path, hooks=[{'id': 'python3-hook', 'language_version': version}], - ) - 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, []) + config = make_config_from_repo(path) + config['hooks'][0]['language_version'] = version + ret = _get_hook(config, store, 'python3-hook').run([]) 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) - repo.require_installed() - (prefix, _, _, _), = repo._venvs() - binaries = os.listdir(prefix.path( + hook = _get_hook(config, store, 'rust-hook') + binaries = os.listdir(hook.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) - repo.require_installed() - (prefix, _, _, _), = repo._venvs() - binaries = os.listdir(prefix.path( + hook = _get_hook(config, store, 'rust-hook') + binaries = os.listdir(hook.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) - (_, hook), = repo.hooks - return repo, hook + return _get_hook(config, store, 'grep-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) - ret, out, _ = repo.run_hook(hook, ('f1', 'f2', 'f3')) + hook = _make_grep_repo(self.language, 'ello', store) + 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']) - ret, out, _ = repo.run_hook(hook, ('f1', 'f2', 'f3')) + hook = _make_grep_repo(self.language, 'ELLO', store, args=['-i']) + 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) - ret, out, _ = repo.run_hook(hook, ('f1', 'f2', 'f3')) + hook = _make_grep_repo(self.language, regex, store) + 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) - ret, out, _ = repo.run_hook(hook, (os.devnull,) * 15000 + ('f1',)) + hook = _make_grep_repo('pcre', 'ello', store) + 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: - return None - else: - return orig_find_executable(exe, **kwargs) + assert exe == pcre.GREP + return None with mock.patch.object(parse_shebang, 'find_executable', no_grep): - repo, hook = _make_grep_repo('pcre', 'ello', store) - ret, out, _ = repo.run_hook(hook, ('f1', 'f2', 'f3')) + hook = _make_grep_repo('pcre', 'ello', store) + 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) - repo1.require_installed() - (prefix1, _, version1, _), = repo1._venvs() - with python.in_env(prefix1, version1): + hook1 = _get_hook(config1, store, 'foo') + with python.in_env(hook1.prefix, hook1.language_version): 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) - repo2.require_installed() - (prefix2, _, version2, _), = repo2._venvs() - with python.in_env(prefix2, version2): + hook2 = _get_hook(config2, store, 'foo') + with python.in_env(hook2.prefix, hook2.language_version): 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'] - repo = Repository.create(config, store) - repo.require_installed() - (prefix, _, version, _), = repo._venvs() - with ruby.in_env(prefix, version): + config['hooks'][0]['additional_dependencies'] = ['tins'] + hook = _get_hook(config, store, 'ruby_hook') + with ruby.in_env(hook.prefix, hook.language_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) - repo.require_installed() - (prefix, _, version, _), = repo._venvs() - with node.in_env(prefix, version): + hook = _get_hook(config, store, 'foo') + with node.in_env(hook.prefix, hook.language_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) - repo.require_installed() - (prefix, _, _, _), = repo._venvs() - binaries = os.listdir(prefix.path( + hook = _get_hook(config, store, 'golang-hook') + binaries = os.listdir(hook.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) - (_, hook), = repo.hooks - ret = repo.run_hook(hook, ('filename',)) + ret = _get_hook(config, store, 'hello').run(()) 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) - (_, hook), = repo.hooks - ret = repo.run_hook(hook, ()) + ret = _get_hook(config, store, 'hello').run(()) 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/.*(? 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) - repo.require_installed() + _get_hook(config, store, 'foo') # 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) - repo.require_installed() + _get_hook(config, store, 'foo') 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) - hook = repo.hooks[0][1] + hooks = repository_hooks(config, store) 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() - envdir = 'py_env-{}'.format(version) - assert prefix.exists(envdir) + hook, = hooks + assert hook.prefix.exists( + 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() - (prefix, _, version, _), = repo._venvs() - libdir = prefix.path('py_env-{}'.format(version), 'lib', version) + libdir = hook.prefix.path( + helpers.environment_dir(python.ENVIRONMENT_DIR, hook.language_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) - hook = repo.hooks[0][1] - retv, stdout, stderr = repo.run_hook(hook, []) + retv, stdout, stderr = _get_hook(config, store, 'foo').run(()) 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) - repo.require_installed() + _get_hook(config, store, 'foo') 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) - assert repo.hooks[0][1]['files'] == '' + hook = _get_hook(config, store, 'bash_hook') + assert hook.files == '' # Set the file regex to something else config['hooks'][0]['files'] = '\\.sh$' - repo = Repository.create(config, store) - assert repo.hooks[0][1]['files'] == '\\.sh$' + hook = _get_hook(config, store, 'bash_hook') + 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) + cmd_output('git', 'tag', tag, cwd=path) 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) - git_dir_2 = _create_repo_with_tags( - tempdir_factory, 'script_hooks_repo', tag, - ) + git1 = _create_repo_with_tags(tempdir_factory, 'prints_cwd_repo', tag) + git2 = _create_repo_with_tags(tempdir_factory, 'script_hooks_repo', tag) - repo_1 = Repository.create( - make_config_from_repo(git_dir_1, rev=tag), store, - ) - ret = repo_1.run_hook(repo_1.hooks[0][1], ['-L']) - assert ret[0] == 0 - assert ret[1].strip() == _norm_pwd(in_tmpdir) + config1 = make_config_from_repo(git1, rev=tag) + ret1 = _get_hook(config1, store, 'prints_cwd').run(('-L',)) + assert ret1[0] == 0 + assert ret1[1].strip() == _norm_pwd(in_tmpdir) - repo_2 = Repository.create( - make_config_from_repo(git_dir_2, rev=tag), store, - ) - ret = repo_2.run_hook(repo_2.hooks[0][1], ['bar']) - 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 + config2 = make_config_from_repo(git2, rev=tag) + ret2 = _get_hook(config2, store, 'bash_hook').run(('bar',)) + assert ret2[0] == 0 + assert ret2[1] == b'bar\nHello World\n' 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), = repo.hooks + hook = _get_hook(config, store, 'foo') # language_version should have been adjusted to the interpreter version - assert hook['language_version'] != 'default' - ret = repo.run_hook(hook, ('filename',)) + assert hook.language_version != 'default' + 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'] == { - 'always_run': False, - 'additional_dependencies': [], - 'args': [], - 'description': '', - 'entry': 'bin/hook.sh', - 'exclude': '^$', - 'files': '', - 'id': 'bash_hook', - 'alias': '', - 'language': 'script', - 'language_version': 'default', - 'log_file': '', - 'minimum_pre_commit_version': '0', - 'name': 'Bash hook', - 'pass_filenames': True, - 'require_serial': False, - 'stages': [], - 'types': ['file'], - 'exclude_types': [], - 'verbose': False, - } + assert hook == Hook( + src='file://{}'.format(path), + prefix=Prefix(mock.ANY), + additional_dependencies=[], + alias='', + always_run=False, + args=[], + description='', + entry='bin/hook.sh', + exclude='^$', + exclude_types=[], + files='', + id='bash_hook', + language='script', + language_version='default', + log_file='', + minimum_pre_commit_version='0', + name='Bash hook', + pass_filenames=True, + require_serial=False, + stages=[], + types=['file'], + verbose=False, + )