diff mbox series

[3/5] sequencer: add support for multiple hooks

Message ID 20190424004948.728326-4-sandals@crustytoothpaste.net (mailing list archive)
State New, archived
Headers show
Series Multiple hook support | expand

Commit Message

brian m. carlson April 24, 2019, 12:49 a.m. UTC
Add support for multiple post-rewrite hooks, both for "git commit
--amend" and "git rebase".

Additionally add support for multiple prepare-commit-msg hooks.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/am.c                       | 28 ++++++---
 sequencer.c                        | 96 +++++++++++++++++++-----------
 t/t5407-post-rewrite-hook.sh       | 15 +++++
 t/t7505-prepare-commit-msg-hook.sh |  9 +++
 4 files changed, 105 insertions(+), 43 deletions(-)

Comments

Phillip Wood April 24, 2019, 9:51 a.m. UTC | #1
On 24/04/2019 01:49, brian m. carlson wrote:
> Add support for multiple post-rewrite hooks, both for "git commit
> --amend" and "git rebase".
> 
> Additionally add support for multiple prepare-commit-msg hooks.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/am.c                       | 28 ++++++---

Having read the patch subject I was surprised to see this touching
bulitin/am.c

>  sequencer.c                        | 96 +++++++++++++++++++-----------
>  t/t5407-post-rewrite-hook.sh       | 15 +++++
>  t/t7505-prepare-commit-msg-hook.sh |  9 +++
>  4 files changed, 105 insertions(+), 43 deletions(-)
> 
> diff --git a/builtin/am.c b/builtin/am.c
> index 4fb107a9d1..303abc98c2 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -442,22 +442,32 @@ static int run_applypatch_msg_hook(struct am_state *state)
>  static int run_post_rewrite_hook(const struct am_state *state)
>  {
>  	struct child_process cp = CHILD_PROCESS_INIT;
> -	const char *hook = find_hook("post-rewrite");
> +	struct child_process proc;
> +	struct string_list *hooks;
> +	struct string_list_item *p;
>  	int ret;
>  
> -	if (!hook)
> +	hooks = find_hooks("post-rewrite");
> +	if (!hooks)
>  		return 0;
>  
> -	argv_array_push(&cp.args, hook);
> -	argv_array_push(&cp.args, "rebase");
> +	for_each_string_list_item(p, hooks) {
> +		child_process_init(&proc);
>  
> -	cp.in = xopen(am_path(state, "rewritten"), O_RDONLY);
> -	cp.stdout_to_stderr = 1;
> -	cp.trace2_hook_name = "post-rewrite";
> +		argv_array_push(&cp.args, p->string);
> +		argv_array_push(&cp.args, "rebase");
>  
> -	ret = run_command(&cp);
> +		cp.in = xopen(am_path(state, "rewritten"), O_RDONLY);
> +		cp.stdout_to_stderr = 1;
> +		cp.trace2_hook_name = "post-rewrite";
>  
> -	close(cp.in);
> +		ret = run_command(&cp);
> +
> +		close(cp.in);
> +		if (ret)
> +			break;
> +	}
> +	free_hooks(hooks);
>  	return ret;
>  }
>  
> diff --git a/sequencer.c b/sequencer.c
> index 79a046d748..3a616ceba6 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1088,33 +1088,69 @@ int update_head_with_reflog(const struct commit *old_head,
>  static int run_rewrite_hook(const struct object_id *oldoid,
>  			    const struct object_id *newoid)
>  {
> -	struct child_process proc = CHILD_PROCESS_INIT;
> +	struct child_process proc;
> +	struct string_list *hooks;
> +	struct string_list_item *p;
>  	const char *argv[3];
>  	int code;
>  	struct strbuf sb = STRBUF_INIT;
>  
> -	argv[0] = find_hook("post-rewrite");
> -	if (!argv[0])
> +	hooks = find_hooks("post-rewrite");
> +	if (!hooks)
>  		return 0;
>  
> -	argv[1] = "amend";
> -	argv[2] = NULL;
> +	for_each_string_list_item(p, hooks) {
> +		child_process_init(&proc);
>  
> -	proc.argv = argv;
> -	proc.in = -1;
> -	proc.stdout_to_stderr = 1;
> -	proc.trace2_hook_name = "post-rewrite";
> +		argv[0] = p->string;
> +		argv[1] = "amend";
> +		argv[2] = NULL;
>  
> -	code = start_command(&proc);
> -	if (code)
> -		return code;
> -	strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
> -	sigchain_push(SIGPIPE, SIG_IGN);
> -	write_in_full(proc.in, sb.buf, sb.len);
> -	close(proc.in);
> -	strbuf_release(&sb);
> -	sigchain_pop(SIGPIPE);
> -	return finish_command(&proc);
> +		proc.argv = argv;
> +		proc.in = -1;
> +		proc.stdout_to_stderr = 1;
> +		proc.trace2_hook_name = "post-rewrite";
> +
> +		code = start_command(&proc);
> +		if (code)
> +			return code;
> +		strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
> +		sigchain_push(SIGPIPE, SIG_IGN);
> +		write_in_full(proc.in, sb.buf, sb.len);
> +		close(proc.in);
> +		strbuf_release(&sb);
> +		sigchain_pop(SIGPIPE);
> +		code = finish_command(&proc);
> +		if (code)
> +			break;
> +	}
> +	free_hooks(hooks);
> +	return code;
> +}
> +
> +static void run_interactive_rewrite_hook(void)
> +{
> +	struct string_list *hooks;
> +	struct string_list_item *p;
> +	struct child_process child;
> +
> +	hooks = find_hooks("post-rewrite");
> +	if (!hooks)
> +		return;
> +
> +	for_each_string_list_item(p, hooks) {
> +		child_process_init(&child);
> +
> +		child.in = open(rebase_path_rewritten_list(),
> +			O_RDONLY);
> +		child.stdout_to_stderr = 1;
> +		child.trace2_hook_name = "post-rewrite";
> +		argv_array_push(&child.args, p->string);
> +		argv_array_push(&child.args, "rebase");
> +		if (run_command(&child))
> +			break;
> +	}
> +	free_hooks(hooks);
>  }

If you're adding a function to do this it would be nice to use it from
am.c as well rather than duplicating essentially the same code. Is there
any way to use a helper to run all the hooks, rather than introducing a
similar loop everywhere where we call a hook?

>  void commit_post_rewrite(struct repository *r,
> @@ -1326,6 +1362,7 @@ static int try_to_commit(struct repository *r,
>  	char *amend_author = NULL;
>  	const char *hook_commit = NULL;
>  	enum commit_msg_cleanup_mode cleanup;
> +	struct string_list *hooks;
>  	int res = 0;
>  
>  	if (parse_head(r, &current_head))
> @@ -1369,7 +1406,10 @@ static int try_to_commit(struct repository *r,
>  		goto out;
>  	}
>  
> -	if (find_hook("prepare-commit-msg")) {
> +	hooks = find_hooks("prepare-commit-msg");
> +	if (hooks) {
> +		free_hooks(hooks);

I think you forgot to update run_prepare_commit_msg_hook(), it should
probably be passed this list now. It might be outside the scope of this
series but unifying this with builtin/commit.c

> +
>  		res = run_prepare_commit_msg_hook(r, msg, hook_commit);
>  		if (res)
>  			goto out;
> @@ -3771,8 +3811,6 @@ static int pick_commits(struct repository *r,
>  		if (!stat(rebase_path_rewritten_list(), &st) &&
>  				st.st_size > 0) {
>  			struct child_process child = CHILD_PROCESS_INIT;
> -			const char *post_rewrite_hook =
> -				find_hook("post-rewrite");
>  
>  			child.in = open(rebase_path_rewritten_list(), O_RDONLY);
>  			child.git_cmd = 1;
> @@ -3782,18 +3820,8 @@ static int pick_commits(struct repository *r,
>  			/* we don't care if this copying failed */
>  			run_command(&child);
>  
> -			if (post_rewrite_hook) {
> -				struct child_process hook = CHILD_PROCESS_INIT;
> -
> -				hook.in = open(rebase_path_rewritten_list(),
> -					O_RDONLY);
> -				hook.stdout_to_stderr = 1;
> -				hook.trace2_hook_name = "post-rewrite";
> -				argv_array_push(&hook.args, post_rewrite_hook);
> -				argv_array_push(&hook.args, "rebase");
> -				/* we don't care if this hook failed */
> -				run_command(&hook);
> -			}
> +			/* we don't care if this hook failed */
> +			run_interactive_rewrite_hook();
>  		}
>  		apply_autostash(opts);
>  
> diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
> index 7344253bfb..f8ce32fe3b 100755
> --- a/t/t5407-post-rewrite-hook.sh
> +++ b/t/t5407-post-rewrite-hook.sh
> @@ -5,6 +5,7 @@
>  
>  test_description='Test the post-rewrite hook.'
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-hooks.sh"
>  
>  test_expect_success 'setup' '
>  	test_commit A foo A &&
> @@ -263,4 +264,18 @@ test_expect_success 'git rebase -i (exec)' '
>  	verify_hook_input
>  '
>  
> +cmd_rebase () {
> +	git reset --hard D &&
> +	FAKE_LINES="1 fixup 2" git rebase -i B
> +}
> +
> +cmd_amend () {
> +	git reset --hard D &&
> +	echo "D new message" > newmsg &&
> +	git commit -Fnewmsg --amend
> +}
> +
> +test_multiple_hooks --ignore-exit-status post-rewrite cmd_rebase
> +test_multiple_hooks --ignore-exit-status post-rewrite cmd_amend
> +
>  test_done
> diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
> index ba8bd1b514..5b83f037b5 100755
> --- a/t/t7505-prepare-commit-msg-hook.sh
> +++ b/t/t7505-prepare-commit-msg-hook.sh
> @@ -3,6 +3,7 @@
>  test_description='prepare-commit-msg hook'
>  
>  . ./test-lib.sh
> +. "$TEST_DIRECTORY/lib-hooks.sh"
>  
>  test_expect_success 'set up commits for rebasing' '
>  	test_commit root &&
> @@ -317,4 +318,12 @@ test_expect_success C_LOCALE_OUTPUT 'with failing hook (cherry-pick)' '
>  	test $(grep -c prepare-commit-msg actual) = 1
>  '
>  
> +commit_command () {
> +	echo "$1" >>file &&
> +	git add file &&
> +	git commit -m "$1"
> +}
> +
> +test_multiple_hooks prepare-commit-msg commit_command

It's not clear to me that this is testing the sequencer

Best Wishes

Phillip

> +
>  test_done
>
brian m. carlson April 24, 2019, 10:46 p.m. UTC | #2
On Wed, Apr 24, 2019 at 10:51:56AM +0100, Phillip Wood wrote:
> On 24/04/2019 01:49, brian m. carlson wrote:
> > Add support for multiple post-rewrite hooks, both for "git commit
> > --amend" and "git rebase".
> > 
> > Additionally add support for multiple prepare-commit-msg hooks.
> > 
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  builtin/am.c                       | 28 ++++++---
> 
> Having read the patch subject I was surprised to see this touching
> bulitin/am.c

I can rewrite the subject. Unfortunately, the same hook for rebase goes
through two wildly different modules, so in order to completely convert
the post-rewrite hook, I have to update both at the same time.

> > +static void run_interactive_rewrite_hook(void)
> > +{
> > +	struct string_list *hooks;
> > +	struct string_list_item *p;
> > +	struct child_process child;
> > +
> > +	hooks = find_hooks("post-rewrite");
> > +	if (!hooks)
> > +		return;
> > +
> > +	for_each_string_list_item(p, hooks) {
> > +		child_process_init(&child);
> > +
> > +		child.in = open(rebase_path_rewritten_list(),
> > +			O_RDONLY);
> > +		child.stdout_to_stderr = 1;
> > +		child.trace2_hook_name = "post-rewrite";
> > +		argv_array_push(&child.args, p->string);
> > +		argv_array_push(&child.args, "rebase");
> > +		if (run_command(&child))
> > +			break;
> > +	}
> > +	free_hooks(hooks);
> >  }
> 
> If you're adding a function to do this it would be nice to use it from
> am.c as well rather than duplicating essentially the same code. Is there
> any way to use a helper to run all the hooks, rather than introducing a
> similar loop everywhere where we call a hook?

It's becoming clear to me that a helper is probably going to be cleaner,
so I'll add one in for v2.

> >  void commit_post_rewrite(struct repository *r,
> > @@ -1326,6 +1362,7 @@ static int try_to_commit(struct repository *r,
> >  	char *amend_author = NULL;
> >  	const char *hook_commit = NULL;
> >  	enum commit_msg_cleanup_mode cleanup;
> > +	struct string_list *hooks;
> >  	int res = 0;
> >  
> >  	if (parse_head(r, &current_head))
> > @@ -1369,7 +1406,10 @@ static int try_to_commit(struct repository *r,
> >  		goto out;
> >  	}
> >  
> > -	if (find_hook("prepare-commit-msg")) {
> > +	hooks = find_hooks("prepare-commit-msg");
> > +	if (hooks) {
> > +		free_hooks(hooks);
> 
> I think you forgot to update run_prepare_commit_msg_hook(), it should
> probably be passed this list now. It might be outside the scope of this
> series but unifying this with builtin/commit.c

run_prepare_commit_msg_hook calls run_hook_le, which looks up that value
itself. It's unfortunate that we have to do it twice, but we need to
know whether we need to re-read the commit msg or not. I can explain
this further in the commit message.

> > diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
> > index ba8bd1b514..5b83f037b5 100755
> > --- a/t/t7505-prepare-commit-msg-hook.sh
> > +++ b/t/t7505-prepare-commit-msg-hook.sh
> > @@ -3,6 +3,7 @@
> >  test_description='prepare-commit-msg hook'
> >  
> >  . ./test-lib.sh
> > +. "$TEST_DIRECTORY/lib-hooks.sh"
> >  
> >  test_expect_success 'set up commits for rebasing' '
> >  	test_commit root &&
> > @@ -317,4 +318,12 @@ test_expect_success C_LOCALE_OUTPUT 'with failing hook (cherry-pick)' '
> >  	test $(grep -c prepare-commit-msg actual) = 1
> >  '
> >  
> > +commit_command () {
> > +	echo "$1" >>file &&
> > +	git add file &&
> > +	git commit -m "$1"
> > +}
> > +
> > +test_multiple_hooks prepare-commit-msg commit_command
> 
> It's not clear to me that this is testing the sequencer

You're right. I need to adopt a different approach here.
Phillip Wood April 25, 2019, 2:59 p.m. UTC | #3
On 24/04/2019 23:46, brian m. carlson wrote:
> On Wed, Apr 24, 2019 at 10:51:56AM +0100, Phillip Wood wrote:
>> On 24/04/2019 01:49, brian m. carlson wrote:
>>> Add support for multiple post-rewrite hooks, both for "git commit
>>> --amend" and "git rebase".
>>>
>>> Additionally add support for multiple prepare-commit-msg hooks.
>>>
>>> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
>>> ---
>>>   builtin/am.c                       | 28 ++++++---
>>
>> Having read the patch subject I was surprised to see this touching
>> bulitin/am.c
> 
> I can rewrite the subject. Unfortunately, the same hook for rebase goes
> through two wildly different modules, so in order to completely convert
> the post-rewrite hook, I have to update both at the same time.

It makes sense to convert both in the same commit, it's just that when I 
see "sequencer" I think of the subset of rebase (-k/-i/-m/-r/-x) that 
uses sequencer.c

>>> +static void run_interactive_rewrite_hook(void)
>>> +{
>>> +	struct string_list *hooks;
>>> +	struct string_list_item *p;
>>> +	struct child_process child;
>>> +
>>> +	hooks = find_hooks("post-rewrite");
>>> +	if (!hooks)
>>> +		return;
>>> +
>>> +	for_each_string_list_item(p, hooks) {
>>> +		child_process_init(&child);
>>> +
>>> +		child.in = open(rebase_path_rewritten_list(),
>>> +			O_RDONLY);
>>> +		child.stdout_to_stderr = 1;
>>> +		child.trace2_hook_name = "post-rewrite";
>>> +		argv_array_push(&child.args, p->string);
>>> +		argv_array_push(&child.args, "rebase");
>>> +		if (run_command(&child))
>>> +			break;
>>> +	}
>>> +	free_hooks(hooks);
>>>   }
>>
>> If you're adding a function to do this it would be nice to use it from
>> am.c as well rather than duplicating essentially the same code. Is there
>> any way to use a helper to run all the hooks, rather than introducing a
>> similar loop everywhere where we call a hook?
> 
> It's becoming clear to me that a helper is probably going to be cleaner,
> so I'll add one in for v2.
> 
>>>   void commit_post_rewrite(struct repository *r,
>>> @@ -1326,6 +1362,7 @@ static int try_to_commit(struct repository *r,
>>>   	char *amend_author = NULL;
>>>   	const char *hook_commit = NULL;
>>>   	enum commit_msg_cleanup_mode cleanup;
>>> +	struct string_list *hooks;
>>>   	int res = 0;
>>>   
>>>   	if (parse_head(r, &current_head))
>>> @@ -1369,7 +1406,10 @@ static int try_to_commit(struct repository *r,
>>>   		goto out;
>>>   	}
>>>   
>>> -	if (find_hook("prepare-commit-msg")) {
>>> +	hooks = find_hooks("prepare-commit-msg");
>>> +	if (hooks) {
>>> +		free_hooks(hooks);
>>
>> I think you forgot to update run_prepare_commit_msg_hook(), it should
>> probably be passed this list now. It might be outside the scope of this
>> series but unifying this with builtin/commit.c
> 
> run_prepare_commit_msg_hook calls run_hook_le, which looks up that value
> itself. It's unfortunate that we have to do it twice, but we need to
> know whether we need to re-read the commit msg or not. I can explain
> this further in the commit message.

Thanks for explaining that, it would be nice to have that in the commit 
message (I probably should have read the previous two patches to see 
what run_hook_le() was doing).

Best Wishes

Phillip


>>> diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
>>> index ba8bd1b514..5b83f037b5 100755
>>> --- a/t/t7505-prepare-commit-msg-hook.sh
>>> +++ b/t/t7505-prepare-commit-msg-hook.sh
>>> @@ -3,6 +3,7 @@
>>>   test_description='prepare-commit-msg hook'
>>>   
>>>   . ./test-lib.sh
>>> +. "$TEST_DIRECTORY/lib-hooks.sh"
>>>   
>>>   test_expect_success 'set up commits for rebasing' '
>>>   	test_commit root &&
>>> @@ -317,4 +318,12 @@ test_expect_success C_LOCALE_OUTPUT 'with failing hook (cherry-pick)' '
>>>   	test $(grep -c prepare-commit-msg actual) = 1
>>>   '
>>>   
>>> +commit_command () {
>>> +	echo "$1" >>file &&
>>> +	git add file &&
>>> +	git commit -m "$1"
>>> +}
>>> +
>>> +test_multiple_hooks prepare-commit-msg commit_command
>>
>> It's not clear to me that this is testing the sequencer
> 
> You're right. I need to adopt a different approach here.
>
diff mbox series

Patch

diff --git a/builtin/am.c b/builtin/am.c
index 4fb107a9d1..303abc98c2 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -442,22 +442,32 @@  static int run_applypatch_msg_hook(struct am_state *state)
 static int run_post_rewrite_hook(const struct am_state *state)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
-	const char *hook = find_hook("post-rewrite");
+	struct child_process proc;
+	struct string_list *hooks;
+	struct string_list_item *p;
 	int ret;
 
-	if (!hook)
+	hooks = find_hooks("post-rewrite");
+	if (!hooks)
 		return 0;
 
-	argv_array_push(&cp.args, hook);
-	argv_array_push(&cp.args, "rebase");
+	for_each_string_list_item(p, hooks) {
+		child_process_init(&proc);
 
-	cp.in = xopen(am_path(state, "rewritten"), O_RDONLY);
-	cp.stdout_to_stderr = 1;
-	cp.trace2_hook_name = "post-rewrite";
+		argv_array_push(&cp.args, p->string);
+		argv_array_push(&cp.args, "rebase");
 
-	ret = run_command(&cp);
+		cp.in = xopen(am_path(state, "rewritten"), O_RDONLY);
+		cp.stdout_to_stderr = 1;
+		cp.trace2_hook_name = "post-rewrite";
 
-	close(cp.in);
+		ret = run_command(&cp);
+
+		close(cp.in);
+		if (ret)
+			break;
+	}
+	free_hooks(hooks);
 	return ret;
 }
 
diff --git a/sequencer.c b/sequencer.c
index 79a046d748..3a616ceba6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1088,33 +1088,69 @@  int update_head_with_reflog(const struct commit *old_head,
 static int run_rewrite_hook(const struct object_id *oldoid,
 			    const struct object_id *newoid)
 {
-	struct child_process proc = CHILD_PROCESS_INIT;
+	struct child_process proc;
+	struct string_list *hooks;
+	struct string_list_item *p;
 	const char *argv[3];
 	int code;
 	struct strbuf sb = STRBUF_INIT;
 
-	argv[0] = find_hook("post-rewrite");
-	if (!argv[0])
+	hooks = find_hooks("post-rewrite");
+	if (!hooks)
 		return 0;
 
-	argv[1] = "amend";
-	argv[2] = NULL;
+	for_each_string_list_item(p, hooks) {
+		child_process_init(&proc);
 
-	proc.argv = argv;
-	proc.in = -1;
-	proc.stdout_to_stderr = 1;
-	proc.trace2_hook_name = "post-rewrite";
+		argv[0] = p->string;
+		argv[1] = "amend";
+		argv[2] = NULL;
 
-	code = start_command(&proc);
-	if (code)
-		return code;
-	strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
-	sigchain_push(SIGPIPE, SIG_IGN);
-	write_in_full(proc.in, sb.buf, sb.len);
-	close(proc.in);
-	strbuf_release(&sb);
-	sigchain_pop(SIGPIPE);
-	return finish_command(&proc);
+		proc.argv = argv;
+		proc.in = -1;
+		proc.stdout_to_stderr = 1;
+		proc.trace2_hook_name = "post-rewrite";
+
+		code = start_command(&proc);
+		if (code)
+			return code;
+		strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
+		sigchain_push(SIGPIPE, SIG_IGN);
+		write_in_full(proc.in, sb.buf, sb.len);
+		close(proc.in);
+		strbuf_release(&sb);
+		sigchain_pop(SIGPIPE);
+		code = finish_command(&proc);
+		if (code)
+			break;
+	}
+	free_hooks(hooks);
+	return code;
+}
+
+static void run_interactive_rewrite_hook(void)
+{
+	struct string_list *hooks;
+	struct string_list_item *p;
+	struct child_process child;
+
+	hooks = find_hooks("post-rewrite");
+	if (!hooks)
+		return;
+
+	for_each_string_list_item(p, hooks) {
+		child_process_init(&child);
+
+		child.in = open(rebase_path_rewritten_list(),
+			O_RDONLY);
+		child.stdout_to_stderr = 1;
+		child.trace2_hook_name = "post-rewrite";
+		argv_array_push(&child.args, p->string);
+		argv_array_push(&child.args, "rebase");
+		if (run_command(&child))
+			break;
+	}
+	free_hooks(hooks);
 }
 
 void commit_post_rewrite(struct repository *r,
@@ -1326,6 +1362,7 @@  static int try_to_commit(struct repository *r,
 	char *amend_author = NULL;
 	const char *hook_commit = NULL;
 	enum commit_msg_cleanup_mode cleanup;
+	struct string_list *hooks;
 	int res = 0;
 
 	if (parse_head(r, &current_head))
@@ -1369,7 +1406,10 @@  static int try_to_commit(struct repository *r,
 		goto out;
 	}
 
-	if (find_hook("prepare-commit-msg")) {
+	hooks = find_hooks("prepare-commit-msg");
+	if (hooks) {
+		free_hooks(hooks);
+
 		res = run_prepare_commit_msg_hook(r, msg, hook_commit);
 		if (res)
 			goto out;
@@ -3771,8 +3811,6 @@  static int pick_commits(struct repository *r,
 		if (!stat(rebase_path_rewritten_list(), &st) &&
 				st.st_size > 0) {
 			struct child_process child = CHILD_PROCESS_INIT;
-			const char *post_rewrite_hook =
-				find_hook("post-rewrite");
 
 			child.in = open(rebase_path_rewritten_list(), O_RDONLY);
 			child.git_cmd = 1;
@@ -3782,18 +3820,8 @@  static int pick_commits(struct repository *r,
 			/* we don't care if this copying failed */
 			run_command(&child);
 
-			if (post_rewrite_hook) {
-				struct child_process hook = CHILD_PROCESS_INIT;
-
-				hook.in = open(rebase_path_rewritten_list(),
-					O_RDONLY);
-				hook.stdout_to_stderr = 1;
-				hook.trace2_hook_name = "post-rewrite";
-				argv_array_push(&hook.args, post_rewrite_hook);
-				argv_array_push(&hook.args, "rebase");
-				/* we don't care if this hook failed */
-				run_command(&hook);
-			}
+			/* we don't care if this hook failed */
+			run_interactive_rewrite_hook();
 		}
 		apply_autostash(opts);
 
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index 7344253bfb..f8ce32fe3b 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -5,6 +5,7 @@ 
 
 test_description='Test the post-rewrite hook.'
 . ./test-lib.sh
+. "$TEST_DIRECTORY/lib-hooks.sh"
 
 test_expect_success 'setup' '
 	test_commit A foo A &&
@@ -263,4 +264,18 @@  test_expect_success 'git rebase -i (exec)' '
 	verify_hook_input
 '
 
+cmd_rebase () {
+	git reset --hard D &&
+	FAKE_LINES="1 fixup 2" git rebase -i B
+}
+
+cmd_amend () {
+	git reset --hard D &&
+	echo "D new message" > newmsg &&
+	git commit -Fnewmsg --amend
+}
+
+test_multiple_hooks --ignore-exit-status post-rewrite cmd_rebase
+test_multiple_hooks --ignore-exit-status post-rewrite cmd_amend
+
 test_done
diff --git a/t/t7505-prepare-commit-msg-hook.sh b/t/t7505-prepare-commit-msg-hook.sh
index ba8bd1b514..5b83f037b5 100755
--- a/t/t7505-prepare-commit-msg-hook.sh
+++ b/t/t7505-prepare-commit-msg-hook.sh
@@ -3,6 +3,7 @@ 
 test_description='prepare-commit-msg hook'
 
 . ./test-lib.sh
+. "$TEST_DIRECTORY/lib-hooks.sh"
 
 test_expect_success 'set up commits for rebasing' '
 	test_commit root &&
@@ -317,4 +318,12 @@  test_expect_success C_LOCALE_OUTPUT 'with failing hook (cherry-pick)' '
 	test $(grep -c prepare-commit-msg actual) = 1
 '
 
+commit_command () {
+	echo "$1" >>file &&
+	git add file &&
+	git commit -m "$1"
+}
+
+test_multiple_hooks prepare-commit-msg commit_command
+
 test_done