Re: [PATCH v7 11/12] receive-pack: convert update hooks to new API

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

On Mon, 26 Jan 2026, Emily Shaffer <nasamuffin@google.com> wrote:
> On Wed, Jan 21, 2026 at 1:55 PM Adrian Ratiu <adrian.ratiu@collabora.com> wrote:
<snip>
>>  static void run_update_post_hook(struct command *commands)
>>  {
>> +       struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
>> +       struct async muxer;
>>         struct command *cmd;
>> -       struct child_process proc = CHILD_PROCESS_INIT;
>> -       const char *hook;
>> -
>> -       hook = find_hook(the_repository, "post-update");
>> -       if (!hook)
>> -               return;
>> +       int saved_stderr = -1;
>> +       int muxer_started = 0;
>>
>>         for (cmd = commands; cmd; cmd = cmd->next) {
>>                 if (cmd->error_string || cmd->did_not_exist)
>>                         continue;
>> -               if (!proc.args.nr)
>> -                       strvec_push(&proc.args, hook);
>> -               strvec_push(&proc.args, cmd->ref_name);
>> +               strvec_push(&opt.args, cmd->ref_name);
>>         }
>> -       if (!proc.args.nr)
>> +       if (!opt.args.nr)
>>                 return;
>>
>> -       proc.no_stdin = 1;
>> -       proc.stdout_to_stderr = 1;
>> -       proc.err = use_sideband ? -1 : 0;
>> -       proc.trace2_hook_name = "post-update";
>> +       if (use_sideband) {
>> +               memset(&muxer, 0, sizeof(muxer));
>> +               muxer.proc = copy_to_sideband;
>> +               muxer.in = -1;
>> +               if (!start_async(&muxer)) {
>> +                       muxer_started = 1;
>> +                       saved_stderr = dup(STDERR_FILENO);
>> +                       if (saved_stderr >= 0)
>> +                               dup2(muxer.in, STDERR_FILENO);
>> +                       close(muxer.in);
>> +               }
>> +       }
>>
>> -       if (!start_command(&proc)) {
>> -               if (use_sideband)
>> -                       copy_to_sideband(proc.err, -1, NULL);
>> -               finish_command(&proc);
>> +       run_hooks_opt(the_repository, "post-update", &opt);
>> +
>> +       if (saved_stderr >= 0) {
>> +               dup2(saved_stderr, STDERR_FILENO);
>> +               close(saved_stderr);
>>         }
>> +       if (muxer_started)
>> +               finish_async(&muxer);
>
> I guess I'm confused about how the muxer is working here. I guess I
> would expect an async to get created for each child, then to dump the
> entire output of the child when that child terminates, but it seems
> like you're setting up the async to capture the output of all the
> hooks (that is, it seems like start_async() and finish_async()
> encapsulate run_hooks_opt(), which runs multiple children...?)

Thanks for raising this Emily, your confusion is perfectly justified.

Before sending this patch I actually thought that the word "muxer" can
cause confusion and likely it's the wrong word to describe this thread,
because it doesn't actually mux anything. :)

I kept the "mux" terminology only because it was already used by the
other hooks (eg see the next commit) at this abstraction level.

The role of this thread is just to asynchronously capture the output of
run_hooks_opt's children via stderr without blocking the main process
and to send the output ASAP over the sideband channel to achieve as
close to real-time output performance as possible (this was one of
Peff's complaints about our original API).

The per-process output buffering/de-interlacing or "muxing" happens at a
lower level, in run-command, which gets called via run_hooks_opt, and is
not concerned with higher-level "hook" concepts such as whether
post-update needs to output or not over a sideband, it just prints the
final clean output from all children to stderr.

Hope this all makes sense and clarifies why we only need 1 "muxer"
thread per hook type + sideband combination. I did try to capture the
new design in an ASCII art drawing two commits before this one. :)

For more clarity, in v8 I will also rename "muxer" to something like
"sideband_output_redirect_thread". Suggestions welcome.

Please do let me know if you have any questions or uncertainties, happy
to help clarify things (or identify / fix bugs).

P.S.
I've also experimented with factoring out this sideband_output_redirect
("muxer") thread into a common helper which can be reused by all the
hooks requriing sideband output, to avoid the code duplication, however
I gave up on that because this series was already 12 patches long,
though I do think it's a good follow-up.

P.P.S.
When running sequentially (jobs=1 and ungroup=1), run-command does not
buffer/capture the hook output and the single hook child inherits
stdout/stderr to which it outputs directly. If use_sideband is also 0,
then we achieve the best possible real-time output, like in the current
local client hook sequential execution (another Peff requirement).