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

[Adrian Ratiu <adrian.ratiu@collabora.com>]

On Tue, 21 Oct 2025, Patrick Steinhardt <ps@pks.im> wrote:
> On Fri, Oct 17, 2025 at 05:15:41PM +0300, Adrian Ratiu wrote: 
>> 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. 

Yes, that is a good idea. run-command is supposed to be a lower 
level, more generic layer than the hooks, so it should not be 
concerned with concepts such as "sideband". 

Will do in v3. Thanks!