diff options
| -rw-r--r-- | .pre-commit-config.yaml | 10 | ||||
| -rw-r--r-- | CHANGELOG.md | 26 | ||||
| -rw-r--r-- | pre_commit/clientlib.py | 51 | ||||
| -rw-r--r-- | pre_commit/commands/hook_impl.py | 7 | ||||
| -rw-r--r-- | pre_commit/commands/run.py | 3 | ||||
| -rw-r--r-- | pre_commit/file_lock.py | 4 | ||||
| -rw-r--r-- | pre_commit/git.py | 2 | ||||
| -rw-r--r-- | pre_commit/main.py | 3 | ||||
| -rw-r--r-- | pre_commit/repository.py | 18 | ||||
| -rw-r--r-- | setup.cfg | 2 | ||||
| -rw-r--r-- | testing/util.py | 2 | ||||
| -rw-r--r-- | tests/clientlib_test.py | 97 | ||||
| -rw-r--r-- | tests/commands/run_test.py | 1 | ||||
| -rw-r--r-- | tests/conftest.py | 9 | ||||
| -rw-r--r-- | tests/git_test.py | 20 | ||||
| -rw-r--r-- | tests/main_test.py | 5 | ||||
| -rw-r--r-- | tests/repository_test.py | 65 |
17 files changed, 286 insertions, 39 deletions
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d42bb1b..321e839 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: v3.3.0 + rev: v3.4.0 hooks: - id: trailing-whitespace - id: end-of-file-fixer @@ -21,7 +21,7 @@ repos: hooks: - id: autopep8 - repo: https://github.com/pre-commit/pre-commit - rev: v2.9.3 + rev: v2.10.0 hooks: - id: validate_manifest - repo: https://github.com/asottile/pyupgrade @@ -35,16 +35,16 @@ repos: - id: reorder-python-imports args: [--py3-plus] - repo: https://github.com/asottile/add-trailing-comma - rev: v2.0.1 + rev: v2.1.0 hooks: - id: add-trailing-comma args: [--py36-plus] - repo: https://github.com/asottile/setup-cfg-fmt - rev: v1.15.1 + rev: v1.16.0 hooks: - id: setup-cfg-fmt - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.790 + rev: v0.800 hooks: - id: mypy exclude: ^testing/resources/ diff --git a/CHANGELOG.md b/CHANGELOG.md index ef36dec..1ba7ffa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,27 @@ +2.10.0 - 2021-01-27 +=================== + +### Features +- Allow `ci` as a top-level map for configuration for https://pre-commit.ci + - #1735 PR by @asottile. +- Add warning for mutable `rev` in configuration + - #1715 PR by @paulhfischer. + - #974 issue by @asottile. +- Add warning for `/*` in top-level `files` / `exclude` regexes + - #1750 PR by @paulhfischer. + - #1702 issue by @asottile. +- Expose `PRE_COMMIT_REMOTE_BRANCH` environment variable during `pre-push` + hooks + - #1770 PR by @surafelabebe. +- Produce error message for `language` / `language_version` for non-installable + languages + - #1771 PR by @asottile. + +### Fixes +- Fix execution in worktrees in subdirectories of bare repositories + - #1778 PR by @asottile. + - #1777 issue by @s0undt3ch. + 2.9.3 - 2020-12-07 ================== @@ -8,7 +32,7 @@ - Fix cleanup code on docker volumes for go - #1725 PR by @fsouza. - Fix working directory detection on SUBST drives on windows - - #1727 PR by mrogaski. + - #1727 PR by @mrogaski. - #1610 issue by @jcameron73. 2.9.2 - 2020-11-25 diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 20d4492..8f35057 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -1,6 +1,7 @@ import argparse import functools import logging +import re import shlex import sys from typing import Any @@ -112,7 +113,26 @@ LOCAL = 'local' META = 'meta' -class OptionalSensibleRegex(cfgv.OptionalNoDefault): +# should inherit from cfgv.Conditional if sha support is dropped +class WarnMutableRev(cfgv.ConditionalOptional): + def check(self, dct: Dict[str, Any]) -> None: + super().check(dct) + + if self.key in dct: + rev = dct[self.key] + + if '.' not in rev and not re.match(r'^[a-fA-F0-9]+$', rev): + logger.warning( + f'The {self.key!r} field of repo {dct["repo"]!r} ' + f'appears to be a mutable reference ' + f'(moving tag / branch). Mutable references are never ' + f'updated after first install and are not supported. ' + f'See https://pre-commit.com/#using-the-latest-version-for-a-repository ' # noqa: E501 + f'for more details.', + ) + + +class OptionalSensibleRegexAtHook(cfgv.OptionalNoDefault): def check(self, dct: Dict[str, Any]) -> None: super().check(dct) @@ -124,6 +144,17 @@ class OptionalSensibleRegex(cfgv.OptionalNoDefault): ) +class OptionalSensibleRegexAtTop(cfgv.OptionalNoDefault): + def check(self, dct: Dict[str, Any]) -> None: + super().check(dct) + + if '/*' in dct.get(self.key, ''): + logger.warning( + f'The top-level {self.key!r} field is a regex, not a glob -- ' + f"matching '/*' probably isn't what you want here", + ) + + class MigrateShaToRev: key = 'rev' @@ -239,8 +270,8 @@ CONFIG_HOOK_DICT = cfgv.Map( for item in MANIFEST_HOOK_DICT.items if item.key != 'id' ), - OptionalSensibleRegex('files', cfgv.check_string), - OptionalSensibleRegex('exclude', cfgv.check_string), + OptionalSensibleRegexAtHook('files', cfgv.check_string), + OptionalSensibleRegexAtHook('exclude', cfgv.check_string), ) CONFIG_REPO_DICT = cfgv.Map( 'Repository', 'repo', @@ -261,6 +292,14 @@ CONFIG_REPO_DICT = cfgv.Map( ), MigrateShaToRev(), + WarnMutableRev( + 'rev', + cfgv.check_string, + '', + 'repo', + cfgv.NotIn(LOCAL, META), + True, + ), cfgv.WarnAdditionalKeys(('repo', 'rev', 'hooks'), warn_unknown_keys_repo), ) DEFAULT_LANGUAGE_VERSION = cfgv.Map( @@ -297,9 +336,15 @@ CONFIG_SCHEMA = cfgv.Map( 'exclude', 'fail_fast', 'minimum_pre_commit_version', + 'ci', ), warn_unknown_keys_root, ), + OptionalSensibleRegexAtTop('files', cfgv.check_string), + OptionalSensibleRegexAtTop('exclude', cfgv.check_string), + + # do not warn about configuration for pre-commit.ci + cfgv.OptionalNoDefault('ci', cfgv.check_type(dict)), ) diff --git a/pre_commit/commands/hook_impl.py b/pre_commit/commands/hook_impl.py index d0e226f..25c5fdf 100644 --- a/pre_commit/commands/hook_impl.py +++ b/pre_commit/commands/hook_impl.py @@ -69,6 +69,7 @@ def _ns( color: bool, *, all_files: bool = False, + remote_branch: Optional[str] = None, from_ref: Optional[str] = None, to_ref: Optional[str] = None, remote_name: Optional[str] = None, @@ -79,6 +80,7 @@ def _ns( return argparse.Namespace( color=color, hook_stage=hook_type.replace('pre-', ''), + remote_branch=remote_branch, from_ref=from_ref, to_ref=to_ref, remote_name=remote_name, @@ -106,13 +108,14 @@ def _pre_push_ns( remote_url = args[1] for line in stdin.decode().splitlines(): - _, local_sha, _, remote_sha = line.split() + _, local_sha, remote_branch, remote_sha = line.split() if local_sha == Z40: continue elif remote_sha != Z40 and _rev_exists(remote_sha): return _ns( 'pre-push', color, from_ref=remote_sha, to_ref=local_sha, + remote_branch=remote_branch, remote_name=remote_name, remote_url=remote_url, ) else: @@ -133,6 +136,7 @@ def _pre_push_ns( 'pre-push', color, all_files=True, remote_name=remote_name, remote_url=remote_url, + remote_branch=remote_branch, ) else: rev_cmd = ('git', 'rev-parse', f'{first_ancestor}^') @@ -141,6 +145,7 @@ def _pre_push_ns( 'pre-push', color, from_ref=source, to_ref=local_sha, remote_name=remote_name, remote_url=remote_url, + remote_branch=remote_branch, ) # nothing to push diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 1e8fad2..891488d 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -371,7 +371,8 @@ def run( environ['PRE_COMMIT_FROM_REF'] = args.from_ref environ['PRE_COMMIT_TO_REF'] = args.to_ref - if args.remote_name and args.remote_url: + if args.remote_name and args.remote_url and args.remote_branch: + environ['PRE_COMMIT_REMOTE_BRANCH'] = args.remote_branch environ['PRE_COMMIT_REMOTE_NAME'] = args.remote_name environ['PRE_COMMIT_REMOTE_URL'] = args.remote_url diff --git a/pre_commit/file_lock.py b/pre_commit/file_lock.py index 5e7a058..55a8eb2 100644 --- a/pre_commit/file_lock.py +++ b/pre_commit/file_lock.py @@ -1,11 +1,11 @@ import contextlib import errno -import os +import sys from typing import Callable from typing import Generator -if os.name == 'nt': # pragma: no cover (windows) +if sys.platform == 'win32': # pragma: no cover (windows) import msvcrt # https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/locking diff --git a/pre_commit/git.py b/pre_commit/git.py index 5096274..bec816c 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -61,7 +61,7 @@ def get_root() -> str: 'git failed. Is it installed, and are you in a Git repository ' 'directory?', ) - if os.path.commonpath((root, git_dir)) == git_dir: + if os.path.samefile(root, git_dir): raise FatalError( 'git toplevel unexpectedly empty! make sure you are not ' 'inside the `.git` directory of your repository.', diff --git a/pre_commit/main.py b/pre_commit/main.py index c1eb104..ce850c4 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -97,6 +97,9 @@ def _add_run_options(parser: argparse.ArgumentParser) -> None: help='The stage during which the hook is fired. One of %(choices)s', ) parser.add_argument( + '--remote-branch', help='Remote branch ref used by `git push`.', + ) + parser.add_argument( '--from-ref', '--source', '-s', help=( '(for usage with `--from-ref`) -- this option represents the ' diff --git a/pre_commit/repository.py b/pre_commit/repository.py index 46e96c1..15827dd 100644 --- a/pre_commit/repository.py +++ b/pre_commit/repository.py @@ -118,6 +118,24 @@ def _hook( if not ret['stages']: ret['stages'] = root_config['default_stages'] + if languages[lang].ENVIRONMENT_DIR is None: + if ret['language_version'] != C.DEFAULT: + logger.error( + f'The hook `{ret["id"]}` specifies `language_version` but is ' + f'using language `{lang}` which does not install an ' + f'environment. ' + f'Perhaps you meant to use a specific language?', + ) + exit(1) + if ret['additional_dependencies']: + logger.error( + f'The hook `{ret["id"]}` specifies `additional_dependencies` ' + f'but is using language `{lang}` which does not install an ' + f'environment. ' + f'Perhaps you meant to use a specific language?', + ) + exit(1) + return ret @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 2.9.3 +version = 2.10.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/testing/util.py b/testing/util.py index 18cd734..1f8cb35 100644 --- a/testing/util.py +++ b/testing/util.py @@ -61,6 +61,7 @@ def run_opts( color=False, verbose=False, hook=None, + remote_branch='', from_ref='', to_ref='', remote_name='', @@ -78,6 +79,7 @@ def run_opts( color=color, verbose=verbose, hook=hook, + remote_branch=remote_branch, from_ref=from_ref, to_ref=to_ref, remote_name=remote_name, diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index bfb754b..6bdb0d6 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -166,7 +166,85 @@ def test_validate_warn_on_unknown_keys_at_top_level(tmpdir, caplog): ] -def test_validate_optional_sensible_regex(caplog): +def test_ci_map_key_allowed_at_top_level(caplog): + cfg = { + 'ci': {'skip': ['foo']}, + 'repos': [{'repo': 'meta', 'hooks': [{'id': 'identity'}]}], + } + cfgv.validate(cfg, CONFIG_SCHEMA) + assert not caplog.record_tuples + + +def test_ci_key_must_be_map(): + with pytest.raises(cfgv.ValidationError): + cfgv.validate({'ci': 'invalid', 'repos': []}, CONFIG_SCHEMA) + + +@pytest.mark.parametrize( + 'rev', + ( + 'v0.12.4', + 'b27f281', + 'b27f281eb9398fc8504415d7fbdabf119ea8c5e1', + '19.10b0', + '4.3.21-2', + ), +) +def test_warn_mutable_rev_ok(caplog, rev): + config_obj = { + 'repo': 'https://gitlab.com/pycqa/flake8', + 'rev': rev, + 'hooks': [{'id': 'flake8'}], + } + cfgv.validate(config_obj, CONFIG_REPO_DICT) + + assert caplog.record_tuples == [] + + +@pytest.mark.parametrize( + 'rev', + ( + '', + 'HEAD', + 'stable', + 'master', + 'some_branch_name', + ), +) +def test_warn_mutable_rev_invalid(caplog, rev): + config_obj = { + 'repo': 'https://gitlab.com/pycqa/flake8', + 'rev': rev, + 'hooks': [{'id': 'flake8'}], + } + cfgv.validate(config_obj, CONFIG_REPO_DICT) + + assert caplog.record_tuples == [ + ( + 'pre_commit', + logging.WARNING, + "The 'rev' field of repo 'https://gitlab.com/pycqa/flake8' " + 'appears to be a mutable reference (moving tag / branch). ' + 'Mutable references are never updated after first install and are ' + 'not supported. ' + 'See https://pre-commit.com/#using-the-latest-version-for-a-repository ' # noqa: E501 + 'for more details.', + ), + ] + + +def test_warn_mutable_rev_conditional(): + config_obj = { + 'repo': 'meta', + 'rev': '3.7.7', + 'hooks': [{'id': 'flake8'}], + } + + with pytest.raises(cfgv.ValidationError): + cfgv.validate(config_obj, CONFIG_REPO_DICT) + + +def test_validate_optional_sensible_regex_at_hook_level(caplog): config_obj = { 'id': 'flake8', 'files': 'dir/*.py', @@ -183,6 +261,23 @@ def test_validate_optional_sensible_regex(caplog): ] +def test_validate_optional_sensible_regex_at_top_level(caplog): + config_obj = { + 'files': 'dir/*.py', + 'repos': [], + } + cfgv.validate(config_obj, CONFIG_SCHEMA) + + assert caplog.record_tuples == [ + ( + 'pre_commit', + logging.WARNING, + "The top-level 'files' field is a regex, not a glob -- matching " + "'/*' probably isn't what you want here", + ), + ] + + @pytest.mark.parametrize('fn', (validate_config_main, validate_manifest_main)) def test_mains_not_ok(tmpdir, fn): not_yaml = tmpdir.join('f.notyaml') diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 914d567..eaea813 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -486,6 +486,7 @@ def test_from_ref_to_ref_error_msg_error( def test_all_push_options_ok(cap_out, store, repo_with_passing_hook): args = run_opts( from_ref='master', to_ref='master', + remote_branch='master', remote_name='origin', remote_url='https://example.com/repo', ) ret, printed = _do_run(cap_out, store, repo_with_passing_hook, args) diff --git a/tests/conftest.py b/tests/conftest.py index 335d261..b36ce5a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -261,15 +261,6 @@ def cap_out(): yield Fixture(stream) -@pytest.fixture -def fake_log_handler(): - handler = mock.Mock(level=logging.INFO) - logger = logging.getLogger('pre_commit') - logger.addHandler(handler) - yield handler - logger.removeHandler(handler) - - @pytest.fixture(scope='session', autouse=True) def set_git_templatedir(tmpdir_factory): tdir = str(tmpdir_factory.mktemp('git_template_dir')) diff --git a/tests/git_test.py b/tests/git_test.py index fafd4a6..69fd206 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -3,6 +3,7 @@ import os.path import pytest from pre_commit import git +from pre_commit.error_handler import FatalError from pre_commit.util import cmd_output from testing.util import git_commit @@ -18,6 +19,25 @@ def test_get_root_deeper(in_git_dir): assert os.path.normcase(git.get_root()) == expected +def test_in_exactly_dot_git(in_git_dir): + with in_git_dir.join('.git').as_cwd(), pytest.raises(FatalError): + git.get_root() + + +def test_get_root_bare_worktree(tmpdir): + src = tmpdir.join('src').ensure_dir() + cmd_output('git', 'init', str(src)) + git_commit(cwd=str(src)) + + bare = tmpdir.join('bare.git').ensure_dir() + cmd_output('git', 'clone', '--bare', str(src), str(bare)) + + cmd_output('git', 'worktree', 'add', 'foo', 'HEAD', cwd=bare) + + with bare.join('foo').as_cwd(): + assert git.get_root() == os.path.abspath('.') + + def test_get_staged_files_deleted(in_git_dir): in_git_dir.join('test').ensure() cmd_output('git', 'add', 'test') diff --git a/tests/main_test.py b/tests/main_test.py index 6738df6..2460bd8 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -35,11 +35,6 @@ def test_adjust_args_and_chdir_not_in_git_dir(in_tmpdir): main._adjust_args_and_chdir(_args()) -def test_adjust_args_and_chdir_in_dot_git_dir(in_git_dir): - with in_git_dir.join('.git').as_cwd(), pytest.raises(FatalError): - main._adjust_args_and_chdir(_args()) - - def test_adjust_args_and_chdir_noop(in_git_dir): args = _args(command='run', files=['f1', 'f2']) main._adjust_args_and_chdir(args) diff --git a/tests/repository_test.py b/tests/repository_test.py index d513cb7..8540db3 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -569,7 +569,7 @@ def test_additional_golang_dependencies_installed( path = make_repo(tempdir_factory, 'golang_hooks_repo') config = make_config_from_repo(path) # A small go package - deps = ['github.com/golang/example/hello'] + deps = ['golang.org/x/example/hello'] config['hooks'][0]['additional_dependencies'] = deps hook = _get_hook(config, store, 'golang-hook') binaries = os.listdir( @@ -590,7 +590,7 @@ def test_local_golang_additional_dependencies(store): 'name': 'hello', 'entry': 'hello', 'language': 'golang', - 'additional_dependencies': ['github.com/golang/example/hello'], + 'additional_dependencies': ['golang.org/x/example/hello'], }], } hook = _get_hook(config, store, 'hello') @@ -640,7 +640,7 @@ def test_fail_hooks(store): ) -def test_unknown_keys(store, fake_log_handler): +def test_unknown_keys(store, caplog): config = { 'repo': 'local', 'hooks': [{ @@ -653,8 +653,8 @@ def test_unknown_keys(store, fake_log_handler): }], } _get_hook(config, store, 'too-much') - expected = 'Unexpected key(s) present on local => too-much: foo, hello' - assert fake_log_handler.handle.call_args[0][0].msg == expected + msg, = caplog.messages + assert msg == 'Unexpected key(s) present on local => too-much: foo, hello' def test_reinstall(tempdir_factory, store, log_info_mock): @@ -832,27 +832,28 @@ def test_default_stages(store, local_python_config): assert hook.stages == ['push'] -def test_hook_id_not_present(tempdir_factory, store, fake_log_handler): +def test_hook_id_not_present(tempdir_factory, store, caplog): path = make_repo(tempdir_factory, 'script_hooks_repo') config = make_config_from_repo(path) config['hooks'][0]['id'] = 'i-dont-exist' with pytest.raises(SystemExit): _get_hook(config, store, 'i-dont-exist') - assert fake_log_handler.handle.call_args[0][0].msg == ( + _, msg = caplog.messages + assert msg == ( f'`i-dont-exist` is not present in repository file://{path}. ' f'Typo? Perhaps it is introduced in a newer version? ' f'Often `pre-commit autoupdate` fixes this.' ) -def test_too_new_version(tempdir_factory, store, fake_log_handler): +def test_too_new_version(tempdir_factory, store, caplog): path = make_repo(tempdir_factory, 'script_hooks_repo') with modify_manifest(path) as manifest: manifest[0]['minimum_pre_commit_version'] = '999.0.0' config = make_config_from_repo(path) with pytest.raises(SystemExit): _get_hook(config, store, 'bash_hook') - msg = fake_log_handler.handle.call_args[0][0].msg + _, msg = caplog.messages pattern = re_assert.Matches( r'^The hook `bash_hook` requires pre-commit version 999\.0\.0 but ' r'version \d+\.\d+\.\d+ is installed. ' @@ -942,3 +943,49 @@ def test_dotnet_hook(tempdir_factory, store, repo): tempdir_factory, store, repo, 'dotnet example hook', [], b'Hello from dotnet!\n', ) + + +def test_non_installable_hook_error_for_language_version(store, caplog): + config = { + 'repo': 'local', + 'hooks': [{ + 'id': 'system-hook', + 'name': 'system-hook', + 'language': 'system', + 'entry': 'python3 -c "import sys; print(sys.version)"', + 'language_version': 'python3.10', + }], + } + with pytest.raises(SystemExit) as excinfo: + _get_hook(config, store, 'system-hook') + assert excinfo.value.code == 1 + + msg, = caplog.messages + assert msg == ( + 'The hook `system-hook` specifies `language_version` but is using ' + 'language `system` which does not install an environment. ' + 'Perhaps you meant to use a specific language?' + ) + + +def test_non_installable_hook_error_for_additional_dependencies(store, caplog): + config = { + 'repo': 'local', + 'hooks': [{ + 'id': 'system-hook', + 'name': 'system-hook', + 'language': 'system', + 'entry': 'python3 -c "import sys; print(sys.version)"', + 'additional_dependencies': ['astpretty'], + }], + } + with pytest.raises(SystemExit) as excinfo: + _get_hook(config, store, 'system-hook') + assert excinfo.value.code == 1 + + msg, = caplog.messages + assert msg == ( + 'The hook `system-hook` specifies `additional_dependencies` but is ' + 'using language `system` which does not install an environment. ' + 'Perhaps you meant to use a specific language?' + ) |