From fc148b146ad41be71a7852c4867f0773cbfe1ff9 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Wed, 28 Jan 2026 23:39:26 +0200 Subject: [PATCH] receive-pack: convert update hooks to new API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook API avoids creating a custom struct child_process and other internal hook plumbing (e.g. calling find_hook()) and prepares for the specification of hooks via configs or running parallel hooks. Execution is still sequential through the run_hooks_opt .jobs == 1, which is the unchanged default for all hooks. When use_sideband==1, the async thread redirects the hook outputs to sideband 2, otherwise it is not used and the hooks write directly to the fds inherited from the main parent process. When .jobs == 1, run-command's poll loop is avoided entirely via the ungroup=1 option like before (this was Jeff's suggestion), achieving the same real-time output performance. When running in parallel, run-command with ungroup=0 will capture and de-interleave the output of each hook, then write to the parent stderr which is redirected via dup2 to the sideband thread, so that each parallel hook output is presented clearly to the client. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Helped-by: Jeff King Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 103 ++++++++++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 37 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9c49174616..f554dac1ef 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -561,6 +561,48 @@ static int copy_to_sideband(int in, int out UNUSED, void *arg UNUSED) return 0; } +/* + * Start an async thread which redirects hook stderr over the sideband. + * The original stderr fd is saved to `saved_stderr` and STDERR_FILENO is + * redirected to the async's input pipe. + */ +static void prepare_sideband_async(struct async *sideband_async, int *saved_stderr, int *started) +{ + *started = 0; + + if (!use_sideband) + return; + + memset(sideband_async, 0, sizeof(*sideband_async)); + sideband_async->proc = copy_to_sideband; + sideband_async->in = -1; + + if (!start_async(sideband_async)) { + *started = 1; + *saved_stderr = dup(STDERR_FILENO); + if (*saved_stderr >= 0) + dup2(sideband_async->in, STDERR_FILENO); + close(sideband_async->in); + } +} + +/* + * Restore the original stderr and wait for the async sideband thread to finish. + */ +static void finish_sideband_async(struct async *sideband_async, int saved_stderr, int started) +{ + if (!use_sideband) + return; + + if (saved_stderr >= 0) { + dup2(saved_stderr, STDERR_FILENO); + close(saved_stderr); + } + + if (started) + finish_async(sideband_async); +} + static void hmac_hash(unsigned char *out, const char *key_in, size_t key_len, const char *text, size_t text_len) @@ -941,29 +983,25 @@ static int run_receive_hook(struct command *commands, static int run_update_hook(struct command *cmd) { - struct child_process proc = CHILD_PROCESS_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct async sideband_async; + int sideband_async_started = 0; + int saved_stderr = -1; int code; - const char *hook_path = find_hook(the_repository, "update"); - if (!hook_path) - return 0; + strvec_pushl(&opt.args, + cmd->ref_name, + oid_to_hex(&cmd->old_oid), + oid_to_hex(&cmd->new_oid), + NULL); - strvec_push(&proc.args, hook_path); - strvec_push(&proc.args, cmd->ref_name); - strvec_push(&proc.args, oid_to_hex(&cmd->old_oid)); - strvec_push(&proc.args, oid_to_hex(&cmd->new_oid)); + prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); - proc.no_stdin = 1; - proc.stdout_to_stderr = 1; - proc.err = use_sideband ? -1 : 0; - proc.trace2_hook_name = "update"; + code = run_hooks_opt(the_repository, "update", &opt); - code = start_command(&proc); - if (code) - return code; - if (use_sideband) - copy_to_sideband(proc.err, -1, NULL); - return finish_command(&proc); + finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); + + return code; } static struct command *find_command_by_refname(struct command *list, @@ -1639,34 +1677,25 @@ out: static void run_update_post_hook(struct command *commands) { + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct async sideband_async; struct command *cmd; - struct child_process proc = CHILD_PROCESS_INIT; - const char *hook; - - hook = find_hook(the_repository, "post-update"); - if (!hook) - return; + int sideband_async_started = 0; + int saved_stderr = -1; 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"; + prepare_sideband_async(&sideband_async, &saved_stderr, &sideband_async_started); - 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); + + finish_sideband_async(&sideband_async, saved_stderr, sideband_async_started); } static void check_aliased_update_internal(struct command *cmd,