Subject: Re: [PATCH 10/17] hook: convert 'post-rewrite' hook to config

From: Emily Shaffer <[email-masked]>
Date: Tue, 15 Dec 2020 15:42:18 -0800

On Tue, Dec 08, 2020 at 03:02:13PM -0800, Josh Steadmon wrote:
> 
> On 2020.12.04 17:49, Emily Shaffer wrote:
> > diff --git a/sequencer.c b/sequencer.c
> > index 5a98fd2fbc..4befd862ff 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -35,6 +35,7 @@
> >  #include "rebase-interactive.h"
> >  #include "reset.h"
> >  #include "hook.h"
> > +#include "string-list.h"
> >  
> >  #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
> >  
> > @@ -1143,33 +1144,23 @@ 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;
> > -	const char *argv[3];
> > +	struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_ASYNC;
> > +	struct strbuf tmp = STRBUF_INIT;
> >  	int code;
> > -	struct strbuf sb = STRBUF_INIT;
> >  
> > -	argv[0] = find_hook("post-rewrite");
> > -	if (!argv[0])
> > -		return 0;
> > +	strvec_push(&opt.args, "amend");
> >  
> > -	argv[1] = "amend";
> > -	argv[2] = NULL;
> > -
> > -	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);
> 
> Here and in a few other later patches, we're removing some signal
> handling that doesn't seem to be replicated in the run_hooks()
> implementation. Can you add a note to the commit message about why this
> is OK?

Yeah, I have added it to the commit message for next rollup. I think
offline you said you found it - but this signal handling was added when
I taught run_processes_parallel() to use a callback to provide stdin. It
lives in run-command.h now.

 - Emily