Subject: Re: [PATCH v8 17/37] hooks: allow callers to capture output

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 12 Mar 2021 10:08:04 +0100

On Thu, Mar 11 2021, Emily Shaffer wrote:

> Some server-side hooks will require capturing output to send over
> sideband instead of printing directly to stderr. Expose that capability.

So added here in 17/37 and not used until 30/37. As a point on
readability (this isn't the first such patch) I think it would be better
to just squash those together with some "since we now need access to
consume_sideband in hooks, do that ...".

If there's a much larger API it makes sense to do it as another step...

>  hook.c | 3 ++-
>  hook.h | 8 ++++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hook.c b/hook.c
> index e16b082cbd..2322720ffe 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -256,6 +256,7 @@ void run_hooks_opt_init_sync(struct run_hooks_opt *o)
>  	o->dir = NULL;
>  	o->feed_pipe = NULL;
>  	o->feed_pipe_ctx = NULL;
> +	o->consume_sideband = NULL;
>  }
>  
>  void run_hooks_opt_init_async(struct run_hooks_opt *o)
> @@ -434,7 +435,7 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options)
>  				   pick_next_hook,
>  				   notify_start_failure,
>  				   options->feed_pipe,
> -				   NULL,
> +				   options->consume_sideband,
>  				   notify_hook_finished,
>  				   &cb_data,
>  				   "hook",
> diff --git a/hook.h b/hook.h
> index ecf0228a46..4ff9999b04 100644
> --- a/hook.h
> +++ b/hook.h
> @@ -78,6 +78,14 @@ struct run_hooks_opt
>  	feed_pipe_fn feed_pipe;
>  	void *feed_pipe_ctx;
>  
> +	/*
> +	 * Populate this to capture output and prevent it from being printed to
> +	 * stderr. This will be passed directly through to
> +	 * run_command:run_parallel_processes(). See t/helper/test-run-command.c
> +	 * for an example.
> +	 */
> +	consume_sideband_fn consume_sideband;
> +
>  	/* Number of threads to parallelize across */
>  	int jobs;

...but this scaffolding is rather trivial.