From 5ce4a549d3e0ee441698a13e431cf207bc3b611f Mon Sep 17 00:00:00 2001 From: marsha <46257533+m-rsha@users.noreply.github.com> Date: Fri, 3 Mar 2023 20:16:09 -0600 Subject: [PATCH 01/14] prefer `sys.platform` over `os.name` when checking for windows OS --- pre_commit/languages/conda.py | 3 ++- pre_commit/languages/python.py | 4 ++-- pre_commit/util.py | 2 +- testing/util.py | 3 ++- tests/languages/python_test.py | 4 ++-- tests/parse_shebang_test.py | 2 +- tests/repository_test.py | 3 ++- tests/xargs_test.py | 2 +- 8 files changed, 13 insertions(+), 10 deletions(-) diff --git a/pre_commit/languages/conda.py b/pre_commit/languages/conda.py index 05f1d2919..41c355e77 100644 --- a/pre_commit/languages/conda.py +++ b/pre_commit/languages/conda.py @@ -2,6 +2,7 @@ import contextlib import os +import sys from typing import Generator from typing import Sequence @@ -26,7 +27,7 @@ def get_env_patch(env: str) -> PatchesT: # $CONDA_PREFIX/Scripts and $CONDA_PREFIX. Whereas the latter only # seems to be used for python.exe. path: SubstitutionT = (os.path.join(env, 'bin'), os.pathsep, Var('PATH')) - if os.name == 'nt': # pragma: no cover (platform specific) + if sys.platform == 'win32': # pragma: win32 cover path = (env, os.pathsep, *path) path = (os.path.join(env, 'Scripts'), os.pathsep, *path) path = (os.path.join(env, 'Library', 'bin'), os.pathsep, *path) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 976674e2b..3ef343608 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -48,7 +48,7 @@ def _read_pyvenv_cfg(filename: str) -> dict[str, str]: def bin_dir(venv: str) -> str: """On windows there's a different directory for the virtualenv""" - bin_part = 'Scripts' if os.name == 'nt' else 'bin' + bin_part = 'Scripts' if sys.platform == 'win32' else 'bin' return os.path.join(venv, bin_part) @@ -137,7 +137,7 @@ def norm_version(version: str) -> str | None: elif _sys_executable_matches(version): # virtualenv defaults to our exe return None - if os.name == 'nt': # pragma: no cover (windows) + if sys.platform == 'win32': # pragma: no cover (windows) version_exec = _find_by_py_launcher(version) if version_exec: return version_exec diff --git a/pre_commit/util.py b/pre_commit/util.py index ea0d4f525..4f8e8357d 100644 --- a/pre_commit/util.py +++ b/pre_commit/util.py @@ -119,7 +119,7 @@ def cmd_output(*cmd: str, **kwargs: Any) -> tuple[int, str, str | None]: return returncode, stdout, stderr -if os.name != 'nt': # pragma: win32 no cover +if sys.platform != 'win32': # pragma: win32 no cover from os import openpty import termios diff --git a/testing/util.py b/testing/util.py index 7c68d0eee..0fee28265 100644 --- a/testing/util.py +++ b/testing/util.py @@ -3,6 +3,7 @@ import contextlib import os.path import subprocess +import sys import pytest @@ -30,7 +31,7 @@ def cmd_output_mocked_pre_commit_home( return ret, out.replace('\r\n', '\n'), None -xfailif_windows = pytest.mark.xfail(os.name == 'nt', reason='windows') +xfailif_windows = pytest.mark.xfail(sys.platform == 'win32', reason='windows') def run_opts( diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index 8bb284eb9..a4000b416 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -36,10 +36,10 @@ def test_read_pyvenv_cfg_non_utf8(tmpdir): def test_norm_version_expanduser(): home = os.path.expanduser('~') - if os.name == 'nt': # pragma: nt cover + if sys.platform == 'win32': # pragma: win32 cover path = r'~\python343' expected_path = fr'{home}\python343' - else: # pragma: nt no cover + else: # pragma: win32 no cover path = '~/.pyenv/versions/3.4.3/bin/python' expected_path = f'{home}/.pyenv/versions/3.4.3/bin/python' result = python.norm_version(path) diff --git a/tests/parse_shebang_test.py b/tests/parse_shebang_test.py index 2fcb29ee7..dd97ca5d8 100644 --- a/tests/parse_shebang_test.py +++ b/tests/parse_shebang_test.py @@ -94,7 +94,7 @@ def test_normexe_does_not_exist_sep(): assert excinfo.value.args == ('Executable `./i-dont-exist-lol` not found',) -@pytest.mark.xfail(os.name == 'nt', reason='posix only') +@pytest.mark.xfail(sys.platform == 'win32', reason='posix only') def test_normexe_not_executable(tmpdir): # pragma: win32 no cover tmpdir.join('exe').ensure() with tmpdir.as_cwd(), pytest.raises(OSError) as excinfo: diff --git a/tests/repository_test.py b/tests/repository_test.py index 9e5d9d62f..8fe6e02bb 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -2,6 +2,7 @@ import os.path import shutil +import sys from typing import Any from unittest import mock @@ -198,7 +199,7 @@ def test_intermixed_stdout_stderr(tempdir_factory, store): ) -@pytest.mark.xfail(os.name == 'nt', reason='ptys are posix-only') +@pytest.mark.xfail(sys.platform == 'win32', reason='ptys are posix-only') def test_output_isatty(tempdir_factory, store): _test_hook_repo( tempdir_factory, store, 'stdout_stderr_repo', diff --git a/tests/xargs_test.py b/tests/xargs_test.py index 0530e50d1..7c41f98cd 100644 --- a/tests/xargs_test.py +++ b/tests/xargs_test.py @@ -187,7 +187,7 @@ def test_xargs_propagate_kwargs_to_cmd(): assert b'Pre commit is awesome' in stdout -@pytest.mark.xfail(os.name == 'nt', reason='posix only') +@pytest.mark.xfail(sys.platform == 'win32', reason='posix only') def test_xargs_color_true_makes_tty(): retcode, out = xargs.xargs( (sys.executable, '-c', 'import sys; print(sys.stdout.isatty())'), From 0616c0abf75d45d2bd793ced4b3bddc42b478662 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 7 Mar 2023 02:52:32 +0000 Subject: [PATCH 02/14] [pre-commit.ci] pre-commit autoupdate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-autopep8: v2.0.1 → v2.0.2](https://github.com/pre-commit/mirrors-autopep8/compare/v2.0.1...v2.0.2) --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ad8ffba73..0aa2e9ea2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,7 +30,7 @@ repos: - id: pyupgrade args: [--py38-plus] - repo: https://github.com/pre-commit/mirrors-autopep8 - rev: v2.0.1 + rev: v2.0.2 hooks: - id: autopep8 - repo: https://github.com/PyCQA/flake8 From 63a180a935dc0096d23a65aa48b84498b57b8760 Mon Sep 17 00:00:00 2001 From: marsha <46257533+m-rsha@users.noreply.github.com> Date: Sun, 5 Mar 2023 05:16:00 -0600 Subject: [PATCH 03/14] rewrite `args with spaces` test to not require python --- tests/repository_test.py | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/tests/repository_test.py b/tests/repository_test.py index 8fe6e02bb..a6c58bc7d 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -1,6 +1,7 @@ from __future__ import annotations import os.path +import shlex import shutil import sys from typing import Any @@ -17,6 +18,7 @@ from pre_commit.clientlib import load_manifest from pre_commit.hook import Hook from pre_commit.languages import python +from pre_commit.languages import system from pre_commit.prefix import Prefix from pre_commit.repository import _hook_installed from pre_commit.repository import all_hooks @@ -99,22 +101,6 @@ def test_python_hook_default_version(tempdir_factory, store): test_python_hook(tempdir_factory, store) -def test_python_hook_args_with_spaces(tempdir_factory, store): - _test_hook_repo( - tempdir_factory, store, 'python_hooks_repo', - 'foo', - [], - b"['i have spaces', 'and\"\\'quotes', '$and !this']\n" - b'Hello World\n', - config_kwargs={ - 'hooks': [{ - 'id': 'foo', - 'args': ['i have spaces', 'and"\'quotes', '$and !this'], - }], - }, - ) - - def test_python_hook_weird_setup_cfg(in_git_dir, tempdir_factory, store): in_git_dir.join('setup.cfg').write('[install]\ninstall_scripts=/usr/sbin') @@ -583,3 +569,14 @@ def test_non_installable_hook_error_for_additional_dependencies(store, caplog): 'using language `system` which does not install an environment. ' 'Perhaps you meant to use a specific language?' ) + + +def test_args_with_spaces_and_quotes(tmp_path): + ret = run_language( + tmp_path, system, + f"{shlex.quote(sys.executable)} -c 'import sys; print(sys.argv[1:])'", + ('i have spaces', 'and"\'quotes', '$and !this'), + ) + + expected = b"['i have spaces', 'and\"\\'quotes', '$and !this']\n" + assert ret == (0, expected) From 8ab9747b339df5bfbf0b7ebb7ebd1885ad6baabd Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Thu, 9 Mar 2023 11:00:31 -0500 Subject: [PATCH 04/14] show 20 slowest durations in CI --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 602679a60..609c2fe18 100644 --- a/tox.ini +++ b/tox.ini @@ -6,7 +6,7 @@ deps = -rrequirements-dev.txt passenv = * commands = coverage erase - coverage run -m pytest {posargs:tests} --ignore=tests/languages + coverage run -m pytest {posargs:tests} --ignore=tests/languages --durations=20 coverage report --omit=pre_commit/languages/*,tests/languages/* [testenv:pre-commit] From e3e17a1617b90c081e043db32cb046ed010f2310 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 11 Mar 2023 14:15:49 -0500 Subject: [PATCH 05/14] make --hook-type and stages match --- pre_commit/clientlib.py | 67 ++++++++++++++++++++++++++++---- pre_commit/commands/hook_impl.py | 2 +- pre_commit/constants.py | 13 ------- pre_commit/main.py | 8 +++- testing/util.py | 2 +- tests/clientlib_test.py | 48 +++++++++++++++++++++++ tests/commands/hook_impl_test.py | 4 +- tests/commands/run_test.py | 14 +++---- tests/main_test.py | 6 +++ tests/repository_test.py | 25 +++++++----- 10 files changed, 147 insertions(+), 42 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 9ff38c6a2..cb7778bb2 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -6,6 +6,7 @@ import shlex import sys from typing import Any +from typing import NamedTuple from typing import Sequence import cfgv @@ -20,6 +21,20 @@ check_string_regex = cfgv.check_and(cfgv.check_string, cfgv.check_regex) +HOOK_TYPES = ( + 'commit-msg', + 'post-checkout', + 'post-commit', + 'post-merge', + 'post-rewrite', + 'pre-commit', + 'pre-merge-commit', + 'pre-push', + 'prepare-commit-msg', +) +# `manual` is not invoked by any installed git hook. See #719 +STAGES = (*HOOK_TYPES, 'manual') + def check_type_tag(tag: str) -> None: if tag not in ALL_TAGS: @@ -43,6 +58,46 @@ def check_min_version(version: str) -> None: ) +_STAGES = { + 'commit': 'pre-commit', + 'merge-commit': 'pre-merge-commit', + 'push': 'pre-push', +} + + +def transform_stage(stage: str) -> str: + return _STAGES.get(stage, stage) + + +class StagesMigrationNoDefault(NamedTuple): + key: str + default: Sequence[str] + + def check(self, dct: dict[str, Any]) -> None: + if self.key not in dct: + return + + val = dct[self.key] + cfgv.check_array(cfgv.check_any)(val) + + val = [transform_stage(v) for v in val] + cfgv.check_array(cfgv.check_one_of(STAGES))(val) + + def apply_default(self, dct: dict[str, Any]) -> None: + if self.key not in dct: + return + dct[self.key] = [transform_stage(v) for v in dct[self.key]] + + def remove_default(self, dct: dict[str, Any]) -> None: + raise NotImplementedError + + +class StagesMigration(StagesMigrationNoDefault): + def apply_default(self, dct: dict[str, Any]) -> None: + dct.setdefault(self.key, self.default) + super().apply_default(dct) + + MANIFEST_HOOK_DICT = cfgv.Map( 'Hook', 'id', @@ -70,7 +125,7 @@ def check_min_version(version: str) -> None: cfgv.Optional('log_file', cfgv.check_string, ''), cfgv.Optional('minimum_pre_commit_version', cfgv.check_string, '0'), cfgv.Optional('require_serial', cfgv.check_bool, False), - cfgv.Optional('stages', cfgv.check_array(cfgv.check_one_of(C.STAGES)), []), + StagesMigration('stages', []), cfgv.Optional('verbose', cfgv.check_bool, False), ) MANIFEST_SCHEMA = cfgv.Array(MANIFEST_HOOK_DICT) @@ -241,7 +296,9 @@ def check(self, dct: dict[str, Any]) -> None: cfgv.OptionalNoDefault(item.key, item.check_fn) for item in MANIFEST_HOOK_DICT.items if item.key != 'id' + if item.key != 'stages' ), + StagesMigrationNoDefault('stages', []), OptionalSensibleRegexAtHook('files', cfgv.check_string), OptionalSensibleRegexAtHook('exclude', cfgv.check_string), ) @@ -290,17 +347,13 @@ def check(self, dct: dict[str, Any]) -> None: cfgv.RequiredRecurse('repos', cfgv.Array(CONFIG_REPO_DICT)), cfgv.Optional( 'default_install_hook_types', - cfgv.check_array(cfgv.check_one_of(C.HOOK_TYPES)), + cfgv.check_array(cfgv.check_one_of(HOOK_TYPES)), ['pre-commit'], ), cfgv.OptionalRecurse( 'default_language_version', DEFAULT_LANGUAGE_VERSION, {}, ), - cfgv.Optional( - 'default_stages', - cfgv.check_array(cfgv.check_one_of(C.STAGES)), - C.STAGES, - ), + StagesMigration('default_stages', STAGES), cfgv.Optional('files', check_string_regex, ''), cfgv.Optional('exclude', check_string_regex, '^$'), cfgv.Optional('fail_fast', cfgv.check_bool, False), diff --git a/pre_commit/commands/hook_impl.py b/pre_commit/commands/hook_impl.py index f5995e9ad..25d99c297 100644 --- a/pre_commit/commands/hook_impl.py +++ b/pre_commit/commands/hook_impl.py @@ -84,7 +84,7 @@ def _ns( ) -> argparse.Namespace: return argparse.Namespace( color=color, - hook_stage=hook_type.replace('pre-', ''), + hook_stage=hook_type, remote_branch=remote_branch, local_branch=local_branch, from_ref=from_ref, diff --git a/pre_commit/constants.py b/pre_commit/constants.py index 3f03ceed9..79a9bb692 100644 --- a/pre_commit/constants.py +++ b/pre_commit/constants.py @@ -10,17 +10,4 @@ VERSION = importlib.metadata.version('pre_commit') -# `manual` is not invoked by any installed git hook. See #719 -STAGES = ( - 'commit', 'merge-commit', 'prepare-commit-msg', 'commit-msg', - 'post-commit', 'manual', 'post-checkout', 'push', 'post-merge', - 'post-rewrite', -) - -HOOK_TYPES = ( - 'pre-commit', 'pre-merge-commit', 'pre-push', 'prepare-commit-msg', - 'commit-msg', 'post-commit', 'post-checkout', 'post-merge', - 'post-rewrite', -) - DEFAULT = 'default' diff --git a/pre_commit/main.py b/pre_commit/main.py index 3915993ff..62d171e66 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -7,6 +7,7 @@ from typing import Sequence import pre_commit.constants as C +from pre_commit import clientlib from pre_commit import git from pre_commit.color import add_color_option from pre_commit.commands.autoupdate import autoupdate @@ -52,7 +53,7 @@ def _add_config_option(parser: argparse.ArgumentParser) -> None: def _add_hook_type_option(parser: argparse.ArgumentParser) -> None: parser.add_argument( '-t', '--hook-type', - choices=C.HOOK_TYPES, action='append', dest='hook_types', + choices=clientlib.HOOK_TYPES, action='append', dest='hook_types', ) @@ -73,7 +74,10 @@ def _add_run_options(parser: argparse.ArgumentParser) -> None: help='When hooks fail, run `git diff` directly afterward.', ) parser.add_argument( - '--hook-stage', choices=C.STAGES, default='commit', + '--hook-stage', + choices=clientlib.STAGES, + type=clientlib.transform_stage, + default='pre-commit', help='The stage during which the hook is fired. One of %(choices)s', ) parser.add_argument( diff --git a/testing/util.py b/testing/util.py index 0fee28265..8e3934cf2 100644 --- a/testing/util.py +++ b/testing/util.py @@ -46,7 +46,7 @@ def run_opts( to_ref='', remote_name='', remote_url='', - hook_stage='commit', + hook_stage='pre-commit', show_diff_on_failure=False, commit_msg_filename='', prepare_commit_message_source='', diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index efb2aa84a..568b2e974 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -12,6 +12,7 @@ from pre_commit.clientlib import CONFIG_REPO_DICT from pre_commit.clientlib import CONFIG_SCHEMA from pre_commit.clientlib import DEFAULT_LANGUAGE_VERSION +from pre_commit.clientlib import MANIFEST_HOOK_DICT from pre_commit.clientlib import MANIFEST_SCHEMA from pre_commit.clientlib import META_HOOK_DICT from pre_commit.clientlib import OptionalSensibleRegexAtHook @@ -416,3 +417,50 @@ def test_warn_additional(schema): x for x in schema.items if isinstance(x, cfgv.WarnAdditionalKeys) ) assert allowed_keys == set(warn_additional.keys) + + +def test_stages_migration_for_default_stages(): + cfg = { + 'default_stages': ['commit-msg', 'push', 'commit', 'merge-commit'], + 'repos': [], + } + cfgv.validate(cfg, CONFIG_SCHEMA) + cfg = cfgv.apply_defaults(cfg, CONFIG_SCHEMA) + assert cfg['default_stages'] == [ + 'commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit', + ] + + +def test_manifest_stages_defaulting(): + dct = { + 'id': 'fake-hook', + 'name': 'fake-hook', + 'entry': 'fake-hook', + 'language': 'system', + 'stages': ['commit-msg', 'push', 'commit', 'merge-commit'], + } + cfgv.validate(dct, MANIFEST_HOOK_DICT) + dct = cfgv.apply_defaults(dct, MANIFEST_HOOK_DICT) + assert dct['stages'] == [ + 'commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit', + ] + + +def test_config_hook_stages_defaulting_missing(): + dct = {'id': 'fake-hook'} + cfgv.validate(dct, CONFIG_HOOK_DICT) + dct = cfgv.apply_defaults(dct, CONFIG_HOOK_DICT) + assert dct == {'id': 'fake-hook'} + + +def test_config_hook_stages_defaulting(): + dct = { + 'id': 'fake-hook', + 'stages': ['commit-msg', 'push', 'commit', 'merge-commit'], + } + cfgv.validate(dct, CONFIG_HOOK_DICT) + dct = cfgv.apply_defaults(dct, CONFIG_HOOK_DICT) + assert dct == { + 'id': 'fake-hook', + 'stages': ['commit-msg', 'pre-push', 'pre-commit', 'pre-merge-commit'], + } diff --git a/tests/commands/hook_impl_test.py b/tests/commands/hook_impl_test.py index aa321dabc..169e1414b 100644 --- a/tests/commands/hook_impl_test.py +++ b/tests/commands/hook_impl_test.py @@ -142,7 +142,7 @@ def test_check_args_length_prepare_commit_msg_error(): def test_run_ns_pre_commit(): ns = hook_impl._run_ns('pre-commit', True, (), b'') assert ns is not None - assert ns.hook_stage == 'commit' + assert ns.hook_stage == 'pre-commit' assert ns.color is True @@ -245,7 +245,7 @@ def test_run_ns_pre_push_updating_branch(push_example): ns = hook_impl._run_ns('pre-push', False, args, stdin) assert ns is not None - assert ns.hook_stage == 'push' + assert ns.hook_stage == 'pre-push' assert ns.color is False assert ns.remote_name == 'origin' assert ns.remote_url == src diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index f1085d9bb..885b78d61 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -354,13 +354,13 @@ def test_show_diff_on_failure( ({'hook': 'bash_hook'}, (b'Bash hook', b'Passed'), 0, True), ( {'hook': 'nope'}, - (b'No hook with id `nope` in stage `commit`',), + (b'No hook with id `nope` in stage `pre-commit`',), 1, True, ), ( - {'hook': 'nope', 'hook_stage': 'push'}, - (b'No hook with id `nope` in stage `push`',), + {'hook': 'nope', 'hook_stage': 'pre-push'}, + (b'No hook with id `nope` in stage `pre-push`',), 1, True, ), @@ -818,7 +818,7 @@ def test_stages(cap_out, store, repo_with_passing_hook): 'language': 'pygrep', 'stages': [stage], } - for i, stage in enumerate(('commit', 'push', 'manual'), 1) + for i, stage in enumerate(('pre-commit', 'pre-push', 'manual'), 1) ], } add_config_to_repo(repo_with_passing_hook, config) @@ -833,8 +833,8 @@ def _run_for_stage(stage): assert printed.count(b'hook ') == 1 return printed - assert _run_for_stage('commit').startswith(b'hook 1...') - assert _run_for_stage('push').startswith(b'hook 2...') + assert _run_for_stage('pre-commit').startswith(b'hook 1...') + assert _run_for_stage('pre-push').startswith(b'hook 2...') assert _run_for_stage('manual').startswith(b'hook 3...') @@ -1173,7 +1173,7 @@ def test_args_hook_only(cap_out, store, repo_with_passing_hook): ), 'language': 'system', 'files': r'\.py$', - 'stages': ['commit'], + 'stages': ['pre-commit'], }, { 'id': 'do_not_commit', diff --git a/tests/main_test.py b/tests/main_test.py index 511592622..945349fa4 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -216,3 +216,9 @@ def test_expected_fatal_error_no_git_repo(in_tmpdir, cap_out, mock_store_dir): 'Is it installed, and are you in a Git repository directory?' ) assert cap_out_lines[-1] == f'Check the log at {log_file}' + + +def test_hook_stage_migration(mock_store_dir): + with mock.patch.object(main, 'run') as mck: + main.main(('run', '--hook-stage', 'commit')) + assert mck.call_args[0][2].hook_stage == 'pre-commit' diff --git a/tests/repository_test.py b/tests/repository_test.py index a6c58bc7d..903574ce3 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -417,7 +417,7 @@ def test_local_python_repo(store, local_python_config): def test_default_language_version(store, local_python_config): config: dict[str, Any] = { 'default_language_version': {'python': 'fake'}, - 'default_stages': ['commit'], + 'default_stages': ['pre-commit'], 'repos': [local_python_config], } @@ -434,18 +434,18 @@ def test_default_language_version(store, local_python_config): def test_default_stages(store, local_python_config): config: dict[str, Any] = { 'default_language_version': {'python': C.DEFAULT}, - 'default_stages': ['commit'], + 'default_stages': ['pre-commit'], 'repos': [local_python_config], } # `stages` was not set, should default hook, = all_hooks(config, store) - assert hook.stages == ['commit'] + assert hook.stages == ['pre-commit'] # `stages` is set, should not default - config['repos'][0]['hooks'][0]['stages'] = ['push'] + config['repos'][0]['hooks'][0]['stages'] = ['pre-push'] hook, = all_hooks(config, store) - assert hook.stages == ['push'] + assert hook.stages == ['pre-push'] def test_hook_id_not_present(tempdir_factory, store, caplog): @@ -513,11 +513,18 @@ def test_manifest_hooks(tempdir_factory, store): name='Bash hook', pass_filenames=True, require_serial=False, - stages=( - 'commit', 'merge-commit', 'prepare-commit-msg', 'commit-msg', - 'post-commit', 'manual', 'post-checkout', 'push', 'post-merge', + stages=[ + 'commit-msg', + 'post-checkout', + 'post-commit', + 'post-merge', 'post-rewrite', - ), + 'pre-commit', + 'pre-merge-commit', + 'pre-push', + 'prepare-commit-msg', + 'manual', + ], types=['file'], types_or=[], verbose=False, From f39154f69f864457595b21f00e81f0e989d05ddf Mon Sep 17 00:00:00 2001 From: Marcelo Galigniana Date: Fri, 27 Jan 2023 16:18:06 -0300 Subject: [PATCH 06/14] Add pre-rebase hook support --- pre_commit/clientlib.py | 1 + pre_commit/commands/hook_impl.py | 17 ++++++++++ pre_commit/commands/run.py | 5 +++ pre_commit/main.py | 11 +++++++ testing/util.py | 4 +++ tests/commands/hook_impl_test.py | 25 +++++++++++++++ tests/commands/install_uninstall_test.py | 40 ++++++++++++++++++++++++ tests/commands/run_test.py | 10 ++++++ tests/repository_test.py | 1 + 9 files changed, 114 insertions(+) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index cb7778bb2..d0651cae2 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -30,6 +30,7 @@ 'pre-commit', 'pre-merge-commit', 'pre-push', + 'pre-rebase', 'prepare-commit-msg', ) # `manual` is not invoked by any installed git hook. See #719 diff --git a/pre_commit/commands/hook_impl.py b/pre_commit/commands/hook_impl.py index 25d99c297..dab2135d4 100644 --- a/pre_commit/commands/hook_impl.py +++ b/pre_commit/commands/hook_impl.py @@ -73,6 +73,8 @@ def _ns( local_branch: str | None = None, from_ref: str | None = None, to_ref: str | None = None, + pre_rebase_upstream: str | None = None, + pre_rebase_branch: str | None = None, remote_name: str | None = None, remote_url: str | None = None, commit_msg_filename: str | None = None, @@ -89,6 +91,8 @@ def _ns( local_branch=local_branch, from_ref=from_ref, to_ref=to_ref, + pre_rebase_upstream=pre_rebase_upstream, + pre_rebase_branch=pre_rebase_branch, remote_name=remote_name, remote_url=remote_url, commit_msg_filename=commit_msg_filename, @@ -185,6 +189,12 @@ def _check_args_length(hook_type: str, args: Sequence[str]) -> None: f'hook-impl for {hook_type} expected 1, 2, or 3 arguments ' f'but got {len(args)}: {args}', ) + elif hook_type == 'pre-rebase': + if len(args) < 1 or len(args) > 2: + raise SystemExit( + f'hook-impl for {hook_type} expected 1 or 2 arguments ' + f'but got {len(args)}: {args}', + ) elif hook_type in _EXPECTED_ARG_LENGTH_BY_HOOK: expected = _EXPECTED_ARG_LENGTH_BY_HOOK[hook_type] if len(args) != expected: @@ -231,6 +241,13 @@ def _run_ns( return _ns(hook_type, color, is_squash_merge=args[0]) elif hook_type == 'post-rewrite': return _ns(hook_type, color, rewrite_command=args[0]) + elif hook_type == 'pre-rebase' and len(args) == 1: + return _ns(hook_type, color, pre_rebase_upstream=args[0]) + elif hook_type == 'pre-rebase' and len(args) == 2: + return _ns( + hook_type, color, pre_rebase_upstream=args[0], + pre_rebase_branch=args[1], + ) else: raise AssertionError(f'unexpected hook type: {hook_type}') diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index c9bc55b42..c867799e8 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -254,6 +254,7 @@ def _all_filenames(args: argparse.Namespace) -> Collection[str]: # these hooks do not operate on files if args.hook_stage in { 'post-checkout', 'post-commit', 'post-merge', 'post-rewrite', + 'pre-rebase', }: return () elif args.hook_stage in {'prepare-commit-msg', 'commit-msg'}: @@ -389,6 +390,10 @@ def run( environ['PRE_COMMIT_FROM_REF'] = args.from_ref environ['PRE_COMMIT_TO_REF'] = args.to_ref + if args.pre_rebase_upstream and args.pre_rebase_branch: + environ['PRE_COMMIT_PRE_REBASE_UPSTREAM'] = args.pre_rebase_upstream + environ['PRE_COMMIT_PRE_REBASE_BRANCH'] = args.pre_rebase_branch + if ( args.remote_name and args.remote_url and args.remote_branch and args.local_branch diff --git a/pre_commit/main.py b/pre_commit/main.py index 62d171e66..9615c5e14 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -107,6 +107,17 @@ def _add_run_options(parser: argparse.ArgumentParser) -> None: 'now checked out.' ), ) + parser.add_argument( + '--pre-rebase-upstream', help=( + 'The upstream from which the series was forked.' + ), + ) + parser.add_argument( + '--pre-rebase-branch', help=( + 'The branch being rebased, and is not set when ' + 'rebasing the current branch.' + ), + ) parser.add_argument( '--commit-msg-filename', help='Filename to check when running during `commit-msg`', diff --git a/testing/util.py b/testing/util.py index 8e3934cf2..08d52cbc3 100644 --- a/testing/util.py +++ b/testing/util.py @@ -44,6 +44,8 @@ def run_opts( local_branch='', from_ref='', to_ref='', + pre_rebase_upstream='', + pre_rebase_branch='', remote_name='', remote_url='', hook_stage='pre-commit', @@ -67,6 +69,8 @@ def run_opts( local_branch=local_branch, from_ref=from_ref, to_ref=to_ref, + pre_rebase_upstream=pre_rebase_upstream, + pre_rebase_branch=pre_rebase_branch, remote_name=remote_name, remote_url=remote_url, hook_stage=hook_stage, diff --git a/tests/commands/hook_impl_test.py b/tests/commands/hook_impl_test.py index 169e1414b..d757e85c0 100644 --- a/tests/commands/hook_impl_test.py +++ b/tests/commands/hook_impl_test.py @@ -100,6 +100,8 @@ def call(*_, **__): ('commit-msg', ['.git/COMMIT_EDITMSG']), ('post-commit', []), ('post-merge', ['1']), + ('pre-rebase', ['main', 'topic']), + ('pre-rebase', ['main']), ('post-checkout', ['old_head', 'new_head', '1']), ('post-rewrite', ['amend']), # multiple choices for commit-editmsg @@ -139,6 +141,13 @@ def test_check_args_length_prepare_commit_msg_error(): ) +def test_check_args_length_pre_rebase_error(): + with pytest.raises(SystemExit) as excinfo: + hook_impl._check_args_length('pre-rebase', []) + msg, = excinfo.value.args + assert msg == 'hook-impl for pre-rebase expected 1 or 2 arguments but got 0: []' # noqa: E501 + + def test_run_ns_pre_commit(): ns = hook_impl._run_ns('pre-commit', True, (), b'') assert ns is not None @@ -146,6 +155,22 @@ def test_run_ns_pre_commit(): assert ns.color is True +def test_run_ns_pre_rebase(): + ns = hook_impl._run_ns('pre-rebase', True, ('main', 'topic'), b'') + assert ns is not None + assert ns.hook_stage == 'pre-rebase' + assert ns.color is True + assert ns.pre_rebase_upstream == 'main' + assert ns.pre_rebase_branch == 'topic' + + ns = hook_impl._run_ns('pre-rebase', True, ('main',), b'') + assert ns is not None + assert ns.hook_stage == 'pre-rebase' + assert ns.color is True + assert ns.pre_rebase_upstream == 'main' + assert ns.pre_rebase_branch is None + + def test_run_ns_commit_msg(): ns = hook_impl._run_ns('commit-msg', False, ('.git/COMMIT_MSG',), b'') assert ns is not None diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index a1ecda867..8b0d3ece4 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -810,6 +810,46 @@ def test_post_merge_integration(tempdir_factory, store): assert os.path.exists('post-merge.tmp') +def test_pre_rebase_integration(tempdir_factory, store): + path = git_dir(tempdir_factory) + config = { + 'repos': [ + { + 'repo': 'local', + 'hooks': [{ + 'id': 'pre-rebase', + 'name': 'Pre rebase', + 'entry': 'touch pre-rebase.tmp', + 'language': 'system', + 'always_run': True, + 'verbose': True, + 'stages': ['pre-rebase'], + }], + }, + ], + } + write_config(path, config) + with cwd(path): + install(C.CONFIG_FILE, store, hook_types=['pre-rebase']) + open('foo', 'a').close() + cmd_output('git', 'add', '.') + git_commit() + + cmd_output('git', 'checkout', '-b', 'branch') + open('bar', 'a').close() + cmd_output('git', 'add', '.') + git_commit() + + cmd_output('git', 'checkout', 'master') + open('baz', 'a').close() + cmd_output('git', 'add', '.') + git_commit() + + cmd_output('git', 'checkout', 'branch') + cmd_output('git', 'rebase', 'master', 'branch') + assert os.path.exists('pre-rebase.tmp') + + def test_post_rewrite_integration(tempdir_factory, store): path = git_dir(tempdir_factory) config = { diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 885b78d61..dd15b94c5 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -563,6 +563,16 @@ def test_merge_conflict_resolved(cap_out, store, in_merge_conflict): assert msg in printed +def test_rebase(cap_out, store, repo_with_passing_hook): + args = run_opts(pre_rebase_upstream='master', pre_rebase_branch='topic') + environ: MutableMapping[str, str] = {} + ret, printed = _do_run( + cap_out, store, repo_with_passing_hook, args, environ, + ) + assert environ['PRE_COMMIT_PRE_REBASE_UPSTREAM'] == 'master' + assert environ['PRE_COMMIT_PRE_REBASE_BRANCH'] == 'topic' + + @pytest.mark.parametrize( ('hooks', 'expected'), ( diff --git a/tests/repository_test.py b/tests/repository_test.py index 903574ce3..045656689 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -522,6 +522,7 @@ def test_manifest_hooks(tempdir_factory, store): 'pre-commit', 'pre-merge-commit', 'pre-push', + 'pre-rebase', 'prepare-commit-msg', 'manual', ], From d3c0a66d23b5cebc060f48278ddb43bcc3384dfc Mon Sep 17 00:00:00 2001 From: marsha <46257533+m-rsha@users.noreply.github.com> Date: Sun, 12 Mar 2023 08:24:38 -0500 Subject: [PATCH 07/14] move slowest python-specific tests out of repository_test --- tests/languages/python_test.py | 51 ++++++++++++++++++++++++++++++++++ tests/repository_test.py | 29 ------------------- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index a4000b416..ab26e14e7 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -233,3 +233,54 @@ def test_language_versioned_python_hook(tmp_path): return_value=False, ): assert run_language(tmp_path, python, 'myexe') == (0, b'ohai\n') + + +def _make_hello_hello(tmp_path): + setup_py = '''\ +from setuptools import setup + +setup( + name='socks', + version='0.0.0', + py_modules=['socks'], + entry_points={'console_scripts': ['socks = socks:main']}, +) +''' + + main_py = '''\ +import sys + +def main(): + print(repr(sys.argv[1:])) + print('hello hello') + return 0 +''' + tmp_path.joinpath('setup.py').write_text(setup_py) + tmp_path.joinpath('socks.py').write_text(main_py) + + +def test_simple_python_hook(tmp_path): + _make_hello_hello(tmp_path) + + ret = run_language(tmp_path, python, 'socks', [os.devnull]) + assert ret == (0, f'[{os.devnull!r}]\nhello hello\n'.encode()) + + +def test_simple_python_hook_default_version(tmp_path): + # make sure that this continues to work for platforms where default + # language detection does not work + with mock.patch.object( + python, + 'get_default_version', + return_value=C.DEFAULT, + ): + test_simple_python_hook(tmp_path) + + +def test_python_hook_weird_setup_cfg(tmp_path): + _make_hello_hello(tmp_path) + setup_cfg = '[install]\ninstall_scripts=/usr/sbin' + tmp_path.joinpath('setup.cfg').write_text(setup_cfg) + + ret = run_language(tmp_path, python, 'socks', [os.devnull]) + assert ret == (0, f'[{os.devnull!r}]\nhello hello\n'.encode()) diff --git a/tests/repository_test.py b/tests/repository_test.py index 045656689..b8dde99b4 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -82,35 +82,6 @@ def _test_hook_repo( assert out == expected -def test_python_hook(tempdir_factory, store): - _test_hook_repo( - tempdir_factory, store, 'python_hooks_repo', - 'foo', [os.devnull], - f'[{os.devnull!r}]\nHello World\n'.encode(), - ) - - -def test_python_hook_default_version(tempdir_factory, store): - # make sure that this continues to work for platforms where default - # language detection does not work - with mock.patch.object( - python, - 'get_default_version', - return_value=C.DEFAULT, - ): - test_python_hook(tempdir_factory, store) - - -def test_python_hook_weird_setup_cfg(in_git_dir, tempdir_factory, store): - in_git_dir.join('setup.cfg').write('[install]\ninstall_scripts=/usr/sbin') - - _test_hook_repo( - tempdir_factory, store, 'python_hooks_repo', - 'foo', [os.devnull], - f'[{os.devnull!r}]\nHello World\n'.encode(), - ) - - def test_python_venv_deprecation(store, caplog): config = { 'repo': 'local', From 7a7772fcdae8694107b9ab19cc93ff5fdc690755 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 14 Mar 2023 03:19:10 +0000 Subject: [PATCH 08/14] [pre-commit.ci] pre-commit autoupdate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit updates: - [github.com/pre-commit/mirrors-mypy: v1.0.1 → v1.1.1](https://github.com/pre-commit/mirrors-mypy/compare/v1.0.1...v1.1.1) --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0aa2e9ea2..cc96a7037 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -38,7 +38,7 @@ repos: hooks: - id: flake8 - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.0.1 + rev: v1.1.1 hooks: - id: mypy additional_dependencies: [types-all] From a412e5492da8cdac6642b50cc3907db06edec109 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 17 Mar 2023 12:55:34 -0400 Subject: [PATCH 09/14] don't set CARGO_HOME in rust this adds a 270 MB registry cache in the output --- pre_commit/languages/rust.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pre_commit/languages/rust.py b/pre_commit/languages/rust.py index af5f483d3..a1f4dbe16 100644 --- a/pre_commit/languages/rust.py +++ b/pre_commit/languages/rust.py @@ -50,7 +50,6 @@ def _rust_toolchain(language_version: str) -> str: def get_env_patch(target_dir: str, version: str) -> PatchesT: return ( - ('CARGO_HOME', target_dir), ('PATH', (os.path.join(target_dir, 'bin'), os.pathsep, Var('PATH'))), # Only set RUSTUP_TOOLCHAIN if we don't want use the system's default # toolchain From df2cada973da6ee689cbc8e323caccf5c00df92c Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 17 Mar 2023 14:26:34 -0400 Subject: [PATCH 10/14] v3.2.0 --- CHANGELOG.md | 15 +++++++++++++++ setup.cfg | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfcef4530..f2466e207 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,18 @@ +3.2.0 - 2023-03-17 +================== + +### Features +- Allow `pre-commit`, `pre-push`, and `pre-merge-commit` as `stages`. + - #2732 issue by @asottile. + - #2808 PR by @asottile. +- Add `pre-rebase` hook support. + - #2582 issue by @BrutalSimplicity. + - #2725 PR by @mgaligniana. + +### Fixes +- Remove bulky cargo cache from `language: rust` installs. + - #2820 PR by @asottile. + 3.1.1 - 2023-02-27 ================== diff --git a/setup.cfg b/setup.cfg index 507c0ad13..5b3d1560e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 3.1.1 +version = 3.2.0 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown From ee71a9345ce96a78e011c9635a61abc332e38961 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 25 Mar 2023 13:06:22 -0400 Subject: [PATCH 11/14] set CARGO_HOME while executing rustup --- pre_commit/languages/rust.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pre_commit/languages/rust.py b/pre_commit/languages/rust.py index a1f4dbe16..7eec0e7d6 100644 --- a/pre_commit/languages/rust.py +++ b/pre_commit/languages/rust.py @@ -80,9 +80,9 @@ def _add_dependencies( lang_base.setup_cmd(prefix, ('cargo', 'add', *crates)) -def install_rust_with_toolchain(toolchain: str) -> None: +def install_rust_with_toolchain(toolchain: str, envdir: str) -> None: with tempfile.TemporaryDirectory() as rustup_dir: - with envcontext((('RUSTUP_HOME', rustup_dir),)): + with envcontext((('CARGO_HOME', envdir), ('RUSTUP_HOME', rustup_dir))): # acquire `rustup` if not present if parse_shebang.find_executable('rustup') is None: # We did not detect rustup and need to download it first. @@ -145,7 +145,7 @@ def install_environment( ctx.enter_context(in_env(prefix, version)) if version != 'system': - install_rust_with_toolchain(_rust_toolchain(version)) + install_rust_with_toolchain(_rust_toolchain(version), envdir) tmpdir = ctx.enter_context(tempfile.TemporaryDirectory()) ctx.enter_context(envcontext((('RUSTUP_HOME', tmpdir),))) From bb49560dc99a65608c8f9161dd71467af163c0d1 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 25 Mar 2023 14:02:57 -0400 Subject: [PATCH 12/14] v3.2.1 --- CHANGELOG.md | 8 ++++++++ setup.cfg | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2466e207..dfb8f804f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +3.2.1 - 2023-03-25 +================== + +### Fixes +- Fix `language_version` for `language: rust` without global `rustup`. + - #2823 issue by @daschuer. + - #2827 PR by @asottile. + 3.2.0 - 2023-03-17 ================== diff --git a/setup.cfg b/setup.cfg index 5b3d1560e..350fe237a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 3.2.0 +version = 3.2.1 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown From 84f040f58a5710c2a9d6530f9d1e033657665f20 Mon Sep 17 00:00:00 2001 From: Eric DeLabar Date: Mon, 3 Apr 2023 15:50:55 -0400 Subject: [PATCH 13/14] fix #2235 --- pre_commit/languages/swift.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pre_commit/languages/swift.py b/pre_commit/languages/swift.py index 8250ab703..f16bb0451 100644 --- a/pre_commit/languages/swift.py +++ b/pre_commit/languages/swift.py @@ -44,7 +44,7 @@ def install_environment( os.mkdir(envdir) cmd_output_b( 'swift', 'build', - '-C', prefix.prefix_dir, + '--package-path', prefix.prefix_dir, '-c', BUILD_CONFIG, '--build-path', os.path.join(envdir, BUILD_DIR), ) From 5027592625f8df286dea831e84e7bf83021b7c1b Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 3 Apr 2023 16:31:09 -0400 Subject: [PATCH 14/14] v3.2.2 --- CHANGELOG.md | 8 ++++++++ setup.cfg | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dfb8f804f..efd96c796 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +3.2.2 - 2023-04-03 +================== + +### Fixes +- Fix support for swift >= 5.8. + - #2836 PR by @edelabar. + - #2835 issue by @kgrobelny-intive. + 3.2.1 - 2023-03-25 ================== diff --git a/setup.cfg b/setup.cfg index 350fe237a..89e8e4ada 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = pre_commit -version = 3.2.1 +version = 3.2.2 description = A framework for managing and maintaining multi-language pre-commit hooks. long_description = file: README.md long_description_content_type = text/markdown