Repository: pre-commit/pre-commit
Pull request #2725 (2023-01-27 18:36:41)

Add pre-rebase hook support
Hello!

This PR adds the `pre-rebase` hook

Thank you!

Fixes: https://github.com/pre-commit/pre-commit/issues/2582

mgaligniana: @asottile sorry for the delayed updates. Could I ask you for help one more time? I can't find why the `test_pre_rebase_integration` test is not creating the `pre-rebase.tmp` file
mgaligniana: @asottile Sorry for this taking me too long, I was busy the past month but I'm still trying to solve this issue!

I think I'm not understanding how the pre-commit's flow works because I couldn't fix the `test_pre_rebase_integration` at `install_uninstall_test.py` and I don't understand why 😞 .

Could I ask you for help one more time? In case I'm delaying the fix I can publish it in the discord channel or I would love review it during streaming 👾 !

Thank you!


Patch:
diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py
index cb7778b..d0651ca 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 25d99c2..dab2135 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 c9bc55b..c867799 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 62d171e..9615c5e 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`',


Test patch:
diff --git a/testing/util.py b/testing/util.py
index 8e3934c..08d52cb 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 169e141..d757e85 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 a1ecda8..8b0d3ec 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 885b78d..dd15b94 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 903574c..0456566 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',
         ],