diff options
| -rw-r--r-- | .pre-commit-config.yaml | 12 | ||||
| -rw-r--r-- | CHANGELOG.md | 17 | ||||
| -rw-r--r-- | pre_commit/commands/run.py | 54 | ||||
| -rw-r--r-- | pre_commit/languages/node.py | 2 | ||||
| -rw-r--r-- | pre_commit/meta_hooks/check_hooks_apply.py | 2 | ||||
| -rw-r--r-- | pre_commit/meta_hooks/check_useless_excludes.py | 14 | ||||
| -rw-r--r-- | setup.cfg | 2 | ||||
| -rw-r--r-- | tests/commands/run_test.py | 18 | ||||
| -rw-r--r-- | tests/commands/try_repo_test.py | 2 | ||||
| -rw-r--r-- | tests/lang_base_test.py | 62 | ||||
| -rw-r--r-- | tests/languages/golang_test.py | 4 | ||||
| -rw-r--r-- | tests/languages/node_test.py | 2 | ||||
| -rw-r--r-- | tests/parse_shebang_test.py | 12 | ||||
| -rw-r--r-- | tests/xargs_test.py | 35 |
14 files changed, 131 insertions, 107 deletions
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5c6f62b..5381cd6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,6 +1,6 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.4.0 + rev: v4.5.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer @@ -10,25 +10,25 @@ repos: - id: name-tests-test - id: requirements-txt-fixer - repo: https://github.com/asottile/setup-cfg-fmt - rev: v2.4.0 + rev: v2.5.0 hooks: - id: setup-cfg-fmt - repo: https://github.com/asottile/reorder-python-imports - rev: v3.10.0 + rev: v3.12.0 hooks: - id: reorder-python-imports exclude: ^(pre_commit/resources/|testing/resources/python3_hooks_repo/) args: [--py38-plus, --add-import, 'from __future__ import annotations'] - repo: https://github.com/asottile/add-trailing-comma - rev: v3.0.1 + rev: v3.1.0 hooks: - id: add-trailing-comma - repo: https://github.com/asottile/pyupgrade - rev: v3.10.1 + rev: v3.15.0 hooks: - id: pyupgrade args: [--py38-plus] -- repo: https://github.com/pre-commit/mirrors-autopep8 +- repo: https://github.com/hhatto/autopep8 rev: v2.0.4 hooks: - id: autopep8 diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e2ef0d..7a1b61a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,20 @@ +3.5.0 - 2023-10-13 +================== + +### Features +- Improve performance of `check-hooks-apply` and `check-useless-excludes`. + - #2998 PR by @mxr. + - #2935 issue by @mxr. + +### Fixes +- Use `time.monotonic()` for more accurate hook timing. + - #3024 PR by @adamchainz. + +### Migrating +- Require npm 6.x+ for `language: node` hooks. + - #2996 PR by @RoelAdriaans. + - #1983 issue by @henryiii. + 3.4.0 - 2023-09-02 ================== diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index c867799..41ba4ec 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -10,7 +10,8 @@ import subprocess import time import unicodedata from typing import Any -from typing import Collection +from typing import Generator +from typing import Iterable from typing import MutableMapping from typing import Sequence @@ -57,20 +58,20 @@ def _full_msg( def filter_by_include_exclude( - names: Collection[str], + names: Iterable[str], include: str, exclude: str, -) -> list[str]: +) -> Generator[str, None, None]: include_re, exclude_re = re.compile(include), re.compile(exclude) - return [ + return ( filename for filename in names if include_re.search(filename) if not exclude_re.search(filename) - ] + ) class Classifier: - def __init__(self, filenames: Collection[str]) -> None: + def __init__(self, filenames: Iterable[str]) -> None: self.filenames = [f for f in filenames if os.path.lexists(f)] @functools.lru_cache(maxsize=None) @@ -79,15 +80,14 @@ class Classifier: def by_types( self, - names: Sequence[str], - types: Collection[str], - types_or: Collection[str], - exclude_types: Collection[str], - ) -> list[str]: + names: Iterable[str], + types: Iterable[str], + types_or: Iterable[str], + exclude_types: Iterable[str], + ) -> Generator[str, None, None]: types = frozenset(types) types_or = frozenset(types_or) exclude_types = frozenset(exclude_types) - ret = [] for filename in names: tags = self._types_for_file(filename) if ( @@ -95,24 +95,24 @@ class Classifier: (not types_or or tags & types_or) and not tags & exclude_types ): - ret.append(filename) - return ret - - def filenames_for_hook(self, hook: Hook) -> tuple[str, ...]: - names = self.filenames - names = filter_by_include_exclude(names, hook.files, hook.exclude) - names = self.by_types( - names, + yield filename + + def filenames_for_hook(self, hook: Hook) -> Generator[str, None, None]: + return self.by_types( + filter_by_include_exclude( + self.filenames, + hook.files, + hook.exclude, + ), hook.types, hook.types_or, hook.exclude_types, ) - return tuple(names) @classmethod def from_config( cls, - filenames: Collection[str], + filenames: Iterable[str], include: str, exclude: str, ) -> Classifier: @@ -121,7 +121,7 @@ class Classifier: # this also makes improperly quoted shell-based hooks work better # see #1173 if os.altsep == '/' and os.sep == '\\': - filenames = [f.replace(os.sep, os.altsep) for f in filenames] + filenames = (f.replace(os.sep, os.altsep) for f in filenames) filenames = filter_by_include_exclude(filenames, include, exclude) return Classifier(filenames) @@ -148,7 +148,7 @@ def _run_single_hook( verbose: bool, use_color: bool, ) -> tuple[bool, bytes]: - filenames = classifier.filenames_for_hook(hook) + filenames = tuple(classifier.filenames_for_hook(hook)) if hook.id in skips or hook.alias in skips: output.write( @@ -187,7 +187,7 @@ def _run_single_hook( if not hook.pass_filenames: filenames = () - time_before = time.time() + time_before = time.monotonic() language = languages[hook.language] with language.in_env(hook.prefix, hook.language_version): retcode, out = language.run_hook( @@ -199,7 +199,7 @@ def _run_single_hook( require_serial=hook.require_serial, color=use_color, ) - duration = round(time.time() - time_before, 2) or 0 + duration = round(time.monotonic() - time_before, 2) or 0 diff_after = _get_diff() # if the hook makes changes, fail the commit @@ -250,7 +250,7 @@ def _compute_cols(hooks: Sequence[Hook]) -> int: return max(cols, 80) -def _all_filenames(args: argparse.Namespace) -> Collection[str]: +def _all_filenames(args: argparse.Namespace) -> Iterable[str]: # these hooks do not operate on files if args.hook_stage in { 'post-checkout', 'post-commit', 'post-merge', 'post-rewrite', diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py index 66d6136..3e22dc7 100644 --- a/pre_commit/languages/node.py +++ b/pre_commit/languages/node.py @@ -93,7 +93,7 @@ def install_environment( # install as if we installed from git local_install_cmd = ( - 'npm', 'install', '--dev', '--prod', + 'npm', 'install', '--include=dev', '--include=prod', '--ignore-prepublish', '--no-progress', '--no-save', ) lang_base.setup_cmd(prefix, local_install_cmd) diff --git a/pre_commit/meta_hooks/check_hooks_apply.py b/pre_commit/meta_hooks/check_hooks_apply.py index b05a705..7f491a2 100644 --- a/pre_commit/meta_hooks/check_hooks_apply.py +++ b/pre_commit/meta_hooks/check_hooks_apply.py @@ -21,7 +21,7 @@ def check_all_hooks_match_files(config_file: str) -> int: for hook in all_hooks(config, Store()): if hook.always_run or hook.language == 'fail': continue - elif not classifier.filenames_for_hook(hook): + elif not any(classifier.filenames_for_hook(hook)): print(f'{hook.id} does not apply to this repository') retv = 1 diff --git a/pre_commit/meta_hooks/check_useless_excludes.py b/pre_commit/meta_hooks/check_useless_excludes.py index 0a8249b..8b0c106 100644 --- a/pre_commit/meta_hooks/check_useless_excludes.py +++ b/pre_commit/meta_hooks/check_useless_excludes.py @@ -2,6 +2,7 @@ from __future__ import annotations import argparse import re +from typing import Iterable from typing import Sequence from cfgv import apply_defaults @@ -14,7 +15,7 @@ from pre_commit.commands.run import Classifier def exclude_matches_any( - filenames: Sequence[str], + filenames: Iterable[str], include: str, exclude: str, ) -> bool: @@ -50,11 +51,12 @@ def check_useless_excludes(config_file: str) -> int: # Not actually a manifest dict, but this more accurately reflects # the defaults applied during runtime hook = apply_defaults(hook, MANIFEST_HOOK_DICT) - names = classifier.filenames - types = hook['types'] - types_or = hook['types_or'] - exclude_types = hook['exclude_types'] - names = classifier.by_types(names, types, types_or, exclude_types) + names = classifier.by_types( + classifier.filenames, + hook['types'], + hook['types_or'], + hook['exclude_types'], + ) include, exclude = hook['files'], hook['exclude'] if not exclude_matches_any(names, include, exclude): print( @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 3.4.0 +version = 3.5.0 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index dd15b94..6a0cd85 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -293,7 +293,7 @@ def test_verbose_duration(cap_out, store, in_git_dir, t1, t2, expected): write_config('.', {'repo': 'meta', 'hooks': [{'id': 'identity'}]}) cmd_output('git', 'add', '.') opts = run_opts(verbose=True) - with mock.patch.object(time, 'time', side_effect=(t1, t2)): + with mock.patch.object(time, 'monotonic', side_effect=(t1, t2)): ret, printed = _do_run(cap_out, store, str(in_git_dir), opts) assert ret == 0 assert expected in printed @@ -1127,8 +1127,8 @@ def test_classifier_empty_types_or(tmpdir): types_or=[], exclude_types=[], ) - assert for_symlink == ['foo'] - assert for_file == ['bar'] + assert tuple(for_symlink) == ('foo',) + assert tuple(for_file) == ('bar',) @pytest.fixture @@ -1142,33 +1142,33 @@ def some_filenames(): def test_include_exclude_base_case(some_filenames): ret = filter_by_include_exclude(some_filenames, '', '^$') - assert ret == [ + assert tuple(ret) == ( '.pre-commit-hooks.yaml', 'pre_commit/git.py', 'pre_commit/main.py', - ] + ) def test_matches_broken_symlink(tmpdir): with tmpdir.as_cwd(): os.symlink('does-not-exist', 'link') ret = filter_by_include_exclude({'link'}, '', '^$') - assert ret == ['link'] + assert tuple(ret) == ('link',) def test_include_exclude_total_match(some_filenames): ret = filter_by_include_exclude(some_filenames, r'^.*\.py$', '^$') - assert ret == ['pre_commit/git.py', 'pre_commit/main.py'] + assert tuple(ret) == ('pre_commit/git.py', 'pre_commit/main.py') def test_include_exclude_does_search_instead_of_match(some_filenames): ret = filter_by_include_exclude(some_filenames, r'\.yaml$', '^$') - assert ret == ['.pre-commit-hooks.yaml'] + assert tuple(ret) == ('.pre-commit-hooks.yaml',) def test_include_exclude_exclude_removes_files(some_filenames): ret = filter_by_include_exclude(some_filenames, '', r'\.py$') - assert ret == ['.pre-commit-hooks.yaml'] + assert tuple(ret) == ('.pre-commit-hooks.yaml',) def test_args_hook_only(cap_out, store, repo_with_passing_hook): diff --git a/tests/commands/try_repo_test.py b/tests/commands/try_repo_test.py index 0b2db7e..c5f891e 100644 --- a/tests/commands/try_repo_test.py +++ b/tests/commands/try_repo_test.py @@ -43,7 +43,7 @@ def _run_try_repo(tempdir_factory, **kwargs): def test_try_repo_repo_only(cap_out, tempdir_factory): - with mock.patch.object(time, 'time', return_value=0.0): + with mock.patch.object(time, 'monotonic', return_value=0.0): _run_try_repo(tempdir_factory, verbose=True) start, config, rest = _get_out(cap_out) assert start == '' diff --git a/tests/lang_base_test.py b/tests/lang_base_test.py index 1cffa0e..da289ae 100644 --- a/tests/lang_base_test.py +++ b/tests/lang_base_test.py @@ -1,6 +1,5 @@ from __future__ import annotations -import multiprocessing import os.path import sys from unittest import mock @@ -10,6 +9,7 @@ import pytest import pre_commit.constants as C from pre_commit import lang_base from pre_commit import parse_shebang +from pre_commit import xargs from pre_commit.prefix import Prefix from pre_commit.util import CalledProcessError @@ -30,19 +30,6 @@ def homedir_mck(): yield -@pytest.fixture -def no_sched_getaffinity(): - # Simulates an OS without os.sched_getaffinity available (mac/windows) - # https://docs.python.org/3/library/os.html#interface-to-the-scheduler - with mock.patch.object( - os, - 'sched_getaffinity', - create=True, - side_effect=AttributeError, - ): - yield - - def test_exe_exists_does_not_exist(find_exe_mck, homedir_mck): find_exe_mck.return_value = None assert lang_base.exe_exists('ruby') is False @@ -129,40 +116,23 @@ def test_no_env_noop(tmp_path): assert before == inside == after -def test_target_concurrency_sched_getaffinity(no_sched_getaffinity): - with mock.patch.object( - os, - 'sched_getaffinity', - return_value=set(range(345)), - ): - with mock.patch.dict(os.environ, clear=True): - assert lang_base.target_concurrency() == 345 - - -def test_target_concurrency_without_sched_getaffinity(no_sched_getaffinity): - with mock.patch.object(multiprocessing, 'cpu_count', return_value=123): - with mock.patch.dict(os.environ, {}, clear=True): - assert lang_base.target_concurrency() == 123 - - -def test_target_concurrency_testing_env_var(): - with mock.patch.dict( - os.environ, {'PRE_COMMIT_NO_CONCURRENCY': '1'}, clear=True, - ): - assert lang_base.target_concurrency() == 1 - - -def test_target_concurrency_on_travis(): - with mock.patch.dict(os.environ, {'TRAVIS': '1'}, clear=True): - assert lang_base.target_concurrency() == 2 +@pytest.fixture +def cpu_count_mck(): + with mock.patch.object(xargs, 'cpu_count', return_value=4): + yield -def test_target_concurrency_cpu_count_not_implemented(no_sched_getaffinity): - with mock.patch.object( - multiprocessing, 'cpu_count', side_effect=NotImplementedError, - ): - with mock.patch.dict(os.environ, {}, clear=True): - assert lang_base.target_concurrency() == 1 +@pytest.mark.parametrize( + ('var', 'expected'), + ( + ('PRE_COMMIT_NO_CONCURRENCY', 1), + ('TRAVIS', 2), + (None, 4), + ), +) +def test_target_concurrency(cpu_count_mck, var, expected): + with mock.patch.dict(os.environ, {var: '1'} if var else {}, clear=True): + assert lang_base.target_concurrency() == expected def test_shuffled_is_deterministic(): diff --git a/tests/languages/golang_test.py b/tests/languages/golang_test.py index 6406267..19e9f62 100644 --- a/tests/languages/golang_test.py +++ b/tests/languages/golang_test.py @@ -111,11 +111,11 @@ def test_golang_versioned(tmp_path): tmp_path, golang, 'go version', - version='1.18.4', + version='1.21.1', ) assert ret == 0 - assert out.startswith(b'go version go1.18.4') + assert out.startswith(b'go version go1.21.1') def test_local_golang_additional_deps(tmp_path): diff --git a/tests/languages/node_test.py b/tests/languages/node_test.py index cba0228..055cb1e 100644 --- a/tests/languages/node_test.py +++ b/tests/languages/node_test.py @@ -139,7 +139,7 @@ def test_node_with_user_config_set(tmp_path): test_node_hook_system(tmp_path) -@pytest.mark.parametrize('version', (C.DEFAULT, '18.13.0')) +@pytest.mark.parametrize('version', (C.DEFAULT, '18.14.0')) def test_node_hook_versions(tmp_path, version): _make_hello_world(tmp_path) ret = run_language(tmp_path, node, 'node-hello', version=version) diff --git a/tests/parse_shebang_test.py b/tests/parse_shebang_test.py index dd97ca5..bd4384d 100644 --- a/tests/parse_shebang_test.py +++ b/tests/parse_shebang_test.py @@ -133,17 +133,17 @@ def test_normalize_cmd_PATH(): def test_normalize_cmd_shebang(in_tmpdir): - echo = _echo_exe().replace(os.sep, '/') - path = write_executable(echo) - assert parse_shebang.normalize_cmd((path,)) == (echo, path) + us = sys.executable.replace(os.sep, '/') + path = write_executable(us) + assert parse_shebang.normalize_cmd((path,)) == (us, path) def test_normalize_cmd_PATH_shebang_full_path(in_tmpdir): - echo = _echo_exe().replace(os.sep, '/') - path = write_executable(echo) + us = sys.executable.replace(os.sep, '/') + path = write_executable(us) with bin_on_path(): ret = parse_shebang.normalize_cmd(('run',)) - assert ret == (echo, os.path.abspath(path)) + assert ret == (us, os.path.abspath(path)) def test_normalize_cmd_PATH_shebang_PATH(in_tmpdir): diff --git a/tests/xargs_test.py b/tests/xargs_test.py index b0a8e0d..e8000b2 100644 --- a/tests/xargs_test.py +++ b/tests/xargs_test.py @@ -1,6 +1,7 @@ from __future__ import annotations import concurrent.futures +import multiprocessing import os import sys import time @@ -12,6 +13,40 @@ from pre_commit import parse_shebang from pre_commit import xargs +def test_cpu_count_sched_getaffinity_exists(): + with mock.patch.object( + os, 'sched_getaffinity', create=True, return_value=set(range(345)), + ): + assert xargs.cpu_count() == 345 + + +@pytest.fixture +def no_sched_getaffinity(): + # Simulates an OS without os.sched_getaffinity available (mac/windows) + # https://docs.python.org/3/library/os.html#interface-to-the-scheduler + with mock.patch.object( + os, + 'sched_getaffinity', + create=True, + side_effect=AttributeError, + ): + yield + + +def test_cpu_count_multiprocessing_cpu_count_implemented(no_sched_getaffinity): + with mock.patch.object(multiprocessing, 'cpu_count', return_value=123): + assert xargs.cpu_count() == 123 + + +def test_cpu_count_multiprocessing_cpu_count_not_implemented( + no_sched_getaffinity, +): + with mock.patch.object( + multiprocessing, 'cpu_count', side_effect=NotImplementedError, + ): + assert xargs.cpu_count() == 1 + + @pytest.mark.parametrize( ('env', 'expected'), ( |