From f39154f69f864457595b21f00e81f0e989d05ddf Mon Sep 17 00:00:00 2001 From: Marcelo Galigniana Date: Fri, 27 Jan 2023 16:18:06 -0300 Subject: [PATCH] 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 cb7778bb..d0651cae 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -30,6 +30,7 @@ HOOK_TYPES = ( '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 25d99c29..dab2135d 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 c9bc55b4..c867799e 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 62d171e6..9615c5e1 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 8e3934cf..08d52cbc 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 169e1414..d757e85c 100644 --- a/tests/commands/hook_impl_test.py +++ b/tests/commands/hook_impl_test.py @@ -100,6 +100,8 @@ def test_run_legacy_recursive(tmpdir): ('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 a1ecda86..8b0d3ece 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 885b78d6..dd15b94c 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 903574ce..04565668 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', ],