Re: [PATCH v2 07/10] run-command: allow capturing of collated output

From: Patrick Steinhardt <ps@pks.im>
Date: Oct 21, 2025, 07:41 UTC
Message-ID: <aPc5JTxw5uVHEyjH@pks.im>
In-Reply-To: <20251017141544.1538542-8-adrian.ratiu@collabora.com>

On Fri, Oct 17, 2025 at 05:15:41PM +0300, Adrian Ratiu wrote:

Show 21 quoted lines

> diff --git a/run-command.h b/run-command.h
> index e536ed7544..2c2484478b 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -436,6 +436,20 @@ typedef int (*feed_pipe_fn)(int child_in,
>  				void *pp_cb,
>  				void *pp_task_cb);
>  
> +/**
> + * If this callback is provided, instead of collating process output to stderr,
> + * they will be collated into a new pipe. consume_sideband_fn will be called
> + * repeatedly. When output is available on that pipe, it will be contained in
> + * 'output'. But it will be called with an empty 'output' too, to allow for
> + * keepalives or similar operations if necessary.
> + *
> + * pp_cb is the callback cookie as passed into run_processes_parallel.
> + *
> + * Since this callback is provided with the collated output, no task cookie is
> + * provided.
> + */
> +typedef void (*consume_sideband_fn)(struct strbuf *output, void *pp_cb);

I think the interface overall makes sense, but isn't the wording we use here very specific for hooks?

Taking a step back, what this thing seems to do is to take the hook's output and customize how exactly we handle this. That isn't really specific to any kind of "sideband", even though our use case in the hook code does use this for sidebands.

So maybe we should call this `consume_output_fn` and adapt the rest of the code accordingly? Because that's what we ultimately do here, IIUC.

Patrick