From ce3ae8073d190174028364fcd9af405b57795fb2 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 26 Dec 2025 14:23:24 +0200 Subject: [PATCH 01/14] run-command: add first helper for pp child states There is a recurring pattern of testing parallel process child states and file descriptors to determine if a child is running, receiving any input or if it's ready for cleanup. Name the pp_child structure and introduce a first helper to make these checks more readable. Next commits will add more helpers and checks. Suggested-by: Junio C Hamano Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- run-command.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/run-command.c b/run-command.c index e3e02475cc..3989673569 100644 --- a/run-command.c +++ b/run-command.c @@ -1478,15 +1478,22 @@ enum child_state { GIT_CP_WAIT_CLEANUP, }; +struct parallel_child { + enum child_state state; + struct child_process process; + struct strbuf err; + void *data; +}; + +static int child_is_working(const struct parallel_child *pp_child) +{ + return pp_child->state == GIT_CP_WORKING; +} + struct parallel_processes { size_t nr_processes; - struct { - enum child_state state; - struct child_process process; - struct strbuf err; - void *data; - } *children; + struct parallel_child *children; /* * The struct pollfd is logically part of *children, * but the system call expects it as its own array. @@ -1509,7 +1516,7 @@ static void kill_children(const struct parallel_processes *pp, int signo) { for (size_t i = 0; i < opts->processes; i++) - if (pp->children[i].state == GIT_CP_WORKING) + if (child_is_working(&pp->children[i])) kill(pp->children[i].process.pid, signo); } @@ -1665,7 +1672,7 @@ static void pp_buffer_stderr(struct parallel_processes *pp, /* Buffer output from all pipes. */ for (size_t i = 0; i < opts->processes; i++) { - if (pp->children[i].state == GIT_CP_WORKING && + if (child_is_working(&pp->children[i]) && pp->pfd[i].revents & (POLLIN | POLLHUP)) { int n = strbuf_read_once(&pp->children[i].err, pp->children[i].process.err, 0); @@ -1683,7 +1690,7 @@ static void pp_output(const struct parallel_processes *pp) { size_t i = pp->output_owner; - if (pp->children[i].state == GIT_CP_WORKING && + if (child_is_working(&pp->children[i]) && pp->children[i].err.len) { strbuf_write(&pp->children[i].err, stderr); strbuf_reset(&pp->children[i].err); @@ -1748,7 +1755,7 @@ static int pp_collect_finished(struct parallel_processes *pp, * running process time. */ for (i = 0; i < n; i++) - if (pp->children[(pp->output_owner + i) % n].state == GIT_CP_WORKING) + if (child_is_working(&pp->children[(pp->output_owner + i) % n])) break; pp->output_owner = (pp->output_owner + i) % n; } From 6e2e7f6c1e4722e451168e1ca4f04d139ff758b0 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 26 Dec 2025 14:23:25 +0200 Subject: [PATCH 02/14] run-command: add stdin callback for parallelization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a user of the run_processes_parallel() API wants to pipe a large amount of information to the stdin of each parallel command, that data could exceed the pipe buffer of the process's stdin and can be too big to store in-memory via strbuf & friends or to slurp to a file. Generally this is solved by repeatedly writing to child_process.in between calls to start_command() and finish_command(). For a specific pre-existing example of this, see transport.c:run_pre_push_hook(). This adds a generic callback API to run_processes_parallel() to do exactly that in a unified manner, similar to the existing callback APIs, which can then be used by hooks.h to convert the remaining hooks to the new, simpler parallel interface. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- run-command.c | 87 ++++++++++++++++++++++++++++++++++--- run-command.h | 21 +++++++++ t/helper/test-run-command.c | 52 +++++++++++++++++++++- t/t0061-run-command.sh | 31 +++++++++++++ 4 files changed, 182 insertions(+), 9 deletions(-) diff --git a/run-command.c b/run-command.c index 3989673569..aaf0e4ecee 100644 --- a/run-command.c +++ b/run-command.c @@ -1490,6 +1490,16 @@ static int child_is_working(const struct parallel_child *pp_child) return pp_child->state == GIT_CP_WORKING; } +static int child_is_ready_for_cleanup(const struct parallel_child *pp_child) +{ + return child_is_working(pp_child) && !pp_child->process.in; +} + +static int child_is_receiving_input(const struct parallel_child *pp_child) +{ + return child_is_working(pp_child) && pp_child->process.in > 0; +} + struct parallel_processes { size_t nr_processes; @@ -1659,6 +1669,44 @@ static int pp_start_one(struct parallel_processes *pp, return 0; } +static void pp_buffer_stdin(struct parallel_processes *pp, + const struct run_process_parallel_opts *opts) +{ + /* Buffer stdin for each pipe. */ + for (size_t i = 0; i < opts->processes; i++) { + struct child_process *proc = &pp->children[i].process; + int ret; + + if (!child_is_receiving_input(&pp->children[i])) + continue; + + /* + * child input is provided via path_to_stdin when the feed_pipe cb is + * missing, so we just signal an EOF. + */ + if (!opts->feed_pipe) { + close(proc->in); + proc->in = 0; + continue; + } + + /** + * Feed the pipe: + * ret < 0 means error + * ret == 0 means there is more data to be fed + * ret > 0 means feeding finished + */ + ret = opts->feed_pipe(proc->in, opts->data, pp->children[i].data); + if (ret < 0) + die_errno("feed_pipe"); + + if (ret) { + close(proc->in); + proc->in = 0; + } + } +} + static void pp_buffer_stderr(struct parallel_processes *pp, const struct run_process_parallel_opts *opts, int output_timeout) @@ -1729,6 +1777,7 @@ static int pp_collect_finished(struct parallel_processes *pp, pp->children[i].state = GIT_CP_FREE; if (pp->pfd) pp->pfd[i].fd = -1; + pp->children[i].process.in = 0; child_process_init(&pp->children[i].process); if (opts->ungroup) { @@ -1763,6 +1812,27 @@ static int pp_collect_finished(struct parallel_processes *pp, return result; } +static void pp_handle_child_IO(struct parallel_processes *pp, + const struct run_process_parallel_opts *opts, + int output_timeout) +{ + /* + * First push input, if any (it might no-op), to child tasks to avoid them blocking + * after input. This also prevents deadlocks when ungrouping below, if a child blocks + * while the parent also waits for them to finish. + */ + pp_buffer_stdin(pp, opts); + + if (opts->ungroup) { + for (size_t i = 0; i < opts->processes; i++) + if (child_is_ready_for_cleanup(&pp->children[i])) + pp->children[i].state = GIT_CP_WAIT_CLEANUP; + } else { + pp_buffer_stderr(pp, opts, output_timeout); + pp_output(pp); + } +} + void run_processes_parallel(const struct run_process_parallel_opts *opts) { int i, code; @@ -1782,6 +1852,13 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) "max:%"PRIuMAX, (uintmax_t)opts->processes); + /* + * Child tasks might receive input via stdin, terminating early (or not), so + * ignore the default SIGPIPE which gets handled by each feed_pipe_fn which + * actually writes the data to children stdin fds. + */ + sigchain_push(SIGPIPE, SIG_IGN); + pp_init(&pp, opts, &pp_sig); while (1) { for (i = 0; @@ -1799,13 +1876,7 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) } if (!pp.nr_processes) break; - if (opts->ungroup) { - for (size_t i = 0; i < opts->processes; i++) - pp.children[i].state = GIT_CP_WAIT_CLEANUP; - } else { - pp_buffer_stderr(&pp, opts, output_timeout); - pp_output(&pp); - } + pp_handle_child_IO(&pp, opts, output_timeout); code = pp_collect_finished(&pp, opts); if (code) { pp.shutdown = 1; @@ -1816,6 +1887,8 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) pp_cleanup(&pp, opts); + sigchain_pop(SIGPIPE); + if (do_trace2) trace2_region_leave(tr2_category, tr2_label, NULL); } diff --git a/run-command.h b/run-command.h index 0df25e445f..e1ca965b5b 100644 --- a/run-command.h +++ b/run-command.h @@ -420,6 +420,21 @@ typedef int (*start_failure_fn)(struct strbuf *out, void *pp_cb, void *pp_task_cb); +/** + * This callback is repeatedly called on every child process who requests + * start_command() to create a pipe by setting child_process.in < 0. + * + * pp_cb is the callback cookie as passed into run_processes_parallel, and + * pp_task_cb is the callback cookie as passed into get_next_task_fn. + * + * Returns < 0 for error + * Returns == 0 when there is more data to be fed (will be called again) + * Returns > 0 when finished (child closed fd or no more data to be fed) + */ +typedef int (*feed_pipe_fn)(int child_in, + void *pp_cb, + void *pp_task_cb); + /** * This callback is called on every child process that finished processing. * @@ -473,6 +488,12 @@ struct run_process_parallel_opts */ start_failure_fn start_failure; + /* + * feed_pipe: see feed_pipe_fn() above. This can be NULL to omit any + * special handling. + */ + feed_pipe_fn feed_pipe; + /** * task_finished: See task_finished_fn() above. This can be * NULL to omit any special handling. diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 3719f23cc2..4a56456894 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -23,19 +23,26 @@ static int number_callbacks; static int parallel_next(struct child_process *cp, struct strbuf *err, void *cb, - void **task_cb UNUSED) + void **task_cb) { struct child_process *d = cb; if (number_callbacks >= 4) return 0; strvec_pushv(&cp->args, d->args.v); + cp->in = d->in; + cp->no_stdin = d->no_stdin; if (err) strbuf_addstr(err, "preloaded output of a child\n"); else fprintf(stderr, "preloaded output of a child\n"); number_callbacks++; + + /* test_stdin callback will use this to count remaining lines */ + *task_cb = xmalloc(sizeof(int)); + *(int*)(*task_cb) = 2; + return 1; } @@ -54,15 +61,48 @@ static int no_job(struct child_process *cp UNUSED, static int task_finished(int result UNUSED, struct strbuf *err, void *pp_cb UNUSED, - void *pp_task_cb UNUSED) + void *pp_task_cb) { if (err) strbuf_addstr(err, "asking for a quick stop\n"); else fprintf(stderr, "asking for a quick stop\n"); + + FREE_AND_NULL(pp_task_cb); + return 1; } +static int task_finished_quiet(int result UNUSED, + struct strbuf *err UNUSED, + void *pp_cb UNUSED, + void *pp_task_cb) +{ + FREE_AND_NULL(pp_task_cb); + return 0; +} + +static int test_stdin_pipe_feed(int hook_stdin_fd, void *cb UNUSED, void *task_cb) +{ + int *lines_remaining = task_cb; + + if (*lines_remaining) { + struct strbuf buf = STRBUF_INIT; + strbuf_addf(&buf, "sample stdin %d\n", --(*lines_remaining)); + if (write_in_full(hook_stdin_fd, buf.buf, buf.len) < 0) { + if (errno == EPIPE) { + /* child closed stdin, nothing more to do */ + strbuf_release(&buf); + return 1; + } + die_errno("write"); + } + strbuf_release(&buf); + } + + return !(*lines_remaining); +} + struct testsuite { struct string_list tests, failed; int next; @@ -157,6 +197,7 @@ static int testsuite(int argc, const char **argv) struct run_process_parallel_opts opts = { .get_next_task = next_test, .start_failure = test_failed, + .feed_pipe = test_stdin_pipe_feed, .task_finished = test_finished, .data = &suite, }; @@ -460,12 +501,19 @@ int cmd__run_command(int argc, const char **argv) if (!strcmp(argv[1], "run-command-parallel")) { opts.get_next_task = parallel_next; + opts.task_finished = task_finished_quiet; } else if (!strcmp(argv[1], "run-command-abort")) { opts.get_next_task = parallel_next; opts.task_finished = task_finished; } else if (!strcmp(argv[1], "run-command-no-jobs")) { opts.get_next_task = no_job; opts.task_finished = task_finished; + } else if (!strcmp(argv[1], "run-command-stdin")) { + proc.in = -1; + proc.no_stdin = 0; + opts.get_next_task = parallel_next; + opts.task_finished = task_finished_quiet; + opts.feed_pipe = test_stdin_pipe_feed; } else { ret = 1; fprintf(stderr, "check usage\n"); diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 76d4936a87..2f77fde0d9 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -164,6 +164,37 @@ test_expect_success 'run_command runs ungrouped in parallel with more tasks than test_line_count = 4 err ' +test_expect_success 'run_command listens to stdin' ' + cat >expect <<-\EOF && + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + preloaded output of a child + listening for stdin: + sample stdin 1 + sample stdin 0 + EOF + + write_script stdin-script <<-\EOF && + echo "listening for stdin:" + while read line + do + echo "$line" + done + EOF + test-tool run-command run-command-stdin 2 ./stdin-script 2>actual && + test_cmp expect actual +' + cat >expect <<-EOF preloaded output of a child asking for a quick stop From cc1f505ff961925282cf3e902d5552afcea04dfb Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 26 Dec 2025 14:23:26 +0200 Subject: [PATCH 03/14] hook: provide stdin via callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a callback mechanism for feeding stdin to hooks alongside the existing path_to_stdin (which slurps a file's content to stdin). The advantage of this new callback is that it can feed stdin without going through the FS layer. This helps when feeding large amount of data and uses the run-command parallel stdin callback introduced in the preceding commit. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- hook.c | 23 ++++++++++++++++++++++- hook.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/hook.c b/hook.c index b3de1048bf..5ddd7678d1 100644 --- a/hook.c +++ b/hook.c @@ -55,7 +55,7 @@ int hook_exists(struct repository *r, const char *name) static int pick_next_hook(struct child_process *cp, struct strbuf *out UNUSED, void *pp_cb, - void **pp_task_cb UNUSED) + void **pp_task_cb) { struct hook_cb_data *hook_cb = pp_cb; const char *hook_path = hook_cb->hook_path; @@ -65,11 +65,22 @@ static int pick_next_hook(struct child_process *cp, cp->no_stdin = 1; strvec_pushv(&cp->env, hook_cb->options->env.v); + + if (hook_cb->options->path_to_stdin && hook_cb->options->feed_pipe) + BUG("options path_to_stdin and feed_pipe are mutually exclusive"); + /* reopen the file for stdin; run_command closes it. */ if (hook_cb->options->path_to_stdin) { cp->no_stdin = 0; cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY); } + + if (hook_cb->options->feed_pipe) { + cp->no_stdin = 0; + /* start_command() will allocate a pipe / stdin fd for us */ + cp->in = -1; + } + cp->stdout_to_stderr = 1; cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; @@ -77,6 +88,12 @@ static int pick_next_hook(struct child_process *cp, strvec_push(&cp->args, hook_path); strvec_pushv(&cp->args, hook_cb->options->args.v); + /* + * Provide per-hook internal state via task_cb for easy access, so + * hook callbacks don't have to go through hook_cb->options. + */ + *pp_task_cb = hook_cb->options->feed_pipe_cb_data; + /* * This pick_next_hook() will be called again, we're only * running one hook, so indicate that no more work will be @@ -140,6 +157,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, + .feed_pipe = options->feed_pipe, .task_finished = notify_hook_finished, .data = &cb_data, @@ -148,6 +166,9 @@ int run_hooks_opt(struct repository *r, const char *hook_name, if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); + if (options->path_to_stdin && options->feed_pipe) + BUG("options path_to_stdin and feed_pipe are mutually exclusive"); + if (options->invoked_hook) *options->invoked_hook = 0; diff --git a/hook.h b/hook.h index 11863fa734..2169d4a6bd 100644 --- a/hook.h +++ b/hook.h @@ -1,6 +1,7 @@ #ifndef HOOK_H #define HOOK_H #include "strvec.h" +#include "run-command.h" struct repository; @@ -37,6 +38,43 @@ struct run_hooks_opt * Path to file which should be piped to stdin for each hook. */ const char *path_to_stdin; + + /** + * Callback used to incrementally feed a child hook stdin pipe. + * + * Useful especially if a hook consumes large quantities of data + * (e.g. a list of all refs in a client push), so feeding it via + * in-memory strings or slurping to/from files is inefficient. + * While the callback allows piecemeal writing, it can also be + * used for smaller inputs, where it gets called only once. + * + * Add hook callback initalization context to `feed_pipe_ctx`. + * Add hook callback internal state to `feed_pipe_cb_data`. + * + */ + feed_pipe_fn feed_pipe; + + /** + * Opaque data pointer used to pass context to `feed_pipe_fn`. + * + * It can be accessed via the second callback arg 'pp_cb': + * ((struct hook_cb_data *) pp_cb)->hook_cb->options->feed_pipe_ctx; + * + * The caller is responsible for managing the memory for this data. + * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. + */ + void *feed_pipe_ctx; + + /** + * Opaque data pointer used to keep internal state across callback calls. + * + * It can be accessed directly via the third callback arg 'pp_task_cb': + * struct ... *state = pp_task_cb; + * + * The caller is responsible for managing the memory for this data. + * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. + */ + void *feed_pipe_cb_data; }; #define RUN_HOOKS_OPT_INIT { \ From 143963fda8d52340cecdbddbe97822ea69212255 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 26 Dec 2025 14:23:27 +0200 Subject: [PATCH 04/14] hook: convert 'post-rewrite' hook in sequencer.c to hook API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the custom run-command calls used by post-rewrite with the newer and simpler hook_run_opt(), which does not need to create a custom 'struct child_process' or call find_hook(). Another benefit of using the hook API is that hook_run_opt() handles the SIGPIPE toggle logic. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- sequencer.c | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/sequencer.c b/sequencer.c index 5476d39ba9..71ed31c774 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1292,32 +1292,40 @@ int update_head_with_reflog(const struct commit *old_head, return ret; } +static int pipe_from_strbuf(int hook_stdin_fd, void *pp_cb, void *pp_task_cb UNUSED) +{ + struct hook_cb_data *hook_cb = pp_cb; + struct strbuf *to_pipe = hook_cb->options->feed_pipe_ctx; + int ret; + + if (!to_pipe) + BUG("pipe_from_strbuf called without feed_pipe_ctx"); + + ret = write_in_full(hook_stdin_fd, to_pipe->buf, to_pipe->len); + if (ret < 0 && errno != EPIPE) + return ret; + + return 1; /* done writing */ +} + static int run_rewrite_hook(const struct object_id *oldoid, const struct object_id *newoid) { - struct child_process proc = CHILD_PROCESS_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; int code; struct strbuf sb = STRBUF_INIT; - const char *hook_path = find_hook(the_repository, "post-rewrite"); - if (!hook_path) - return 0; - - strvec_pushl(&proc.args, hook_path, "amend", NULL); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = "post-rewrite"; - - code = start_command(&proc); - if (code) - return code; strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid)); - sigchain_push(SIGPIPE, SIG_IGN); - write_in_full(proc.in, sb.buf, sb.len); - close(proc.in); + + opt.feed_pipe_ctx = &sb; + opt.feed_pipe = pipe_from_strbuf; + + strvec_push(&opt.args, "amend"); + + code = run_hooks_opt(the_repository, "post-rewrite", &opt); + strbuf_release(&sb); - sigchain_pop(SIGPIPE); - return finish_command(&proc); + return code; } void commit_post_rewrite(struct repository *r, From 438ee013649bb26cf34dfdfa2557e4767fd48f03 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 26 Dec 2025 14:23:28 +0200 Subject: [PATCH 05/14] transport: convert pre-push to hook API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the pre-push hook from custom run-command invocations to the new hook API which doesn't require a custom child_process structure and signal toggling. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- transport.c | 103 ++++++++++++++++++++++++++-------------------------- 1 file changed, 52 insertions(+), 51 deletions(-) diff --git a/transport.c b/transport.c index c7f06a7382..6d0f02be5d 100644 --- a/transport.c +++ b/transport.c @@ -1316,65 +1316,66 @@ static void die_with_unpushed_submodules(struct string_list *needs_pushing) die(_("Aborting.")); } +struct feed_pre_push_hook_data { + struct strbuf buf; + const struct ref *refs; +}; + +static int pre_push_hook_feed_stdin(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_task_cb) +{ + struct feed_pre_push_hook_data *data = pp_task_cb; + const struct ref *r = data->refs; + int ret = 0; + + if (!r) + return 1; /* no more refs */ + + data->refs = r->next; + + switch (r->status) { + case REF_STATUS_REJECT_NONFASTFORWARD: + case REF_STATUS_REJECT_REMOTE_UPDATED: + case REF_STATUS_REJECT_STALE: + case REF_STATUS_UPTODATE: + return 0; /* skip refs which won't be pushed */ + default: + break; + } + + if (!r->peer_ref) + return 0; + + strbuf_reset(&data->buf); + strbuf_addf(&data->buf, "%s %s %s %s\n", + r->peer_ref->name, oid_to_hex(&r->new_oid), + r->name, oid_to_hex(&r->old_oid)); + + ret = write_in_full(hook_stdin_fd, data->buf.buf, data->buf.len); + if (ret < 0 && errno != EPIPE) + return ret; /* We do not mind if a hook does not read all refs. */ + + return 0; +} + static int run_pre_push_hook(struct transport *transport, struct ref *remote_refs) { - int ret = 0, x; - struct ref *r; - struct child_process proc = CHILD_PROCESS_INIT; - struct strbuf buf; - const char *hook_path = find_hook(the_repository, "pre-push"); + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct feed_pre_push_hook_data data; + int ret = 0; - if (!hook_path) - return 0; + strvec_push(&opt.args, transport->remote->name); + strvec_push(&opt.args, transport->url); - strvec_push(&proc.args, hook_path); - strvec_push(&proc.args, transport->remote->name); - strvec_push(&proc.args, transport->url); + strbuf_init(&data.buf, 0); + data.refs = remote_refs; - proc.in = -1; - proc.trace2_hook_name = "pre-push"; + opt.feed_pipe = pre_push_hook_feed_stdin; + opt.feed_pipe_cb_data = &data; - if (start_command(&proc)) { - finish_command(&proc); - return -1; - } + ret = run_hooks_opt(the_repository, "pre-push", &opt); - sigchain_push(SIGPIPE, SIG_IGN); - - strbuf_init(&buf, 256); - - for (r = remote_refs; r; r = r->next) { - if (!r->peer_ref) continue; - if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue; - if (r->status == REF_STATUS_REJECT_STALE) continue; - if (r->status == REF_STATUS_REJECT_REMOTE_UPDATED) continue; - if (r->status == REF_STATUS_UPTODATE) continue; - - strbuf_reset(&buf); - strbuf_addf( &buf, "%s %s %s %s\n", - r->peer_ref->name, oid_to_hex(&r->new_oid), - r->name, oid_to_hex(&r->old_oid)); - - if (write_in_full(proc.in, buf.buf, buf.len) < 0) { - /* We do not mind if a hook does not read all refs. */ - if (errno != EPIPE) - ret = -1; - break; - } - } - - strbuf_release(&buf); - - x = close(proc.in); - if (!ret) - ret = x; - - sigchain_pop(SIGPIPE); - - x = finish_command(&proc); - if (!ret) - ret = x; + strbuf_release(&data.buf); return ret; } From bb98a78a5f1e35be109bbdb51c67ad6f7805c178 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 26 Dec 2025 14:23:29 +0200 Subject: [PATCH 06/14] reference-transaction: use hook API instead of run-command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the reference-transaction hook to the new hook API, so it doesn't need to set up a struct child_process, call find_hook or toggle the pipe signals. The stdin feed callback is processing one ref update per call. I haven't noticed any performance degradation due to this, however we can batch as many we want in each call, to ensure a good pipe throughtput (i.e. the child does not wait after stdin). Helped-by: Emily Shaffer Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- refs.c | 110 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 57 insertions(+), 53 deletions(-) diff --git a/refs.c b/refs.c index 046b695bb2..e06e0cb072 100644 --- a/refs.c +++ b/refs.c @@ -2422,68 +2422,72 @@ static int ref_update_reject_duplicates(struct string_list *refnames, return 0; } +struct transaction_feed_cb_data { + size_t index; + struct strbuf buf; +}; + +static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_task_cb) +{ + struct hook_cb_data *hook_cb = pp_cb; + struct ref_transaction *transaction = hook_cb->options->feed_pipe_ctx; + struct transaction_feed_cb_data *feed_cb_data = pp_task_cb; + struct strbuf *buf = &feed_cb_data->buf; + struct ref_update *update; + size_t i = feed_cb_data->index++; + int ret; + + if (i >= transaction->nr) + return 1; /* No more refs to process */ + + update = transaction->updates[i]; + + if (update->flags & REF_LOG_ONLY) + return 0; + + strbuf_reset(buf); + + if (!(update->flags & REF_HAVE_OLD)) + strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); + else if (update->old_target) + strbuf_addf(buf, "ref:%s ", update->old_target); + else + strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid)); + + if (!(update->flags & REF_HAVE_NEW)) + strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); + else if (update->new_target) + strbuf_addf(buf, "ref:%s ", update->new_target); + else + strbuf_addf(buf, "%s ", oid_to_hex(&update->new_oid)); + + strbuf_addf(buf, "%s\n", update->refname); + + ret = write_in_full(hook_stdin_fd, buf->buf, buf->len); + if (ret < 0 && errno != EPIPE) + return ret; + + return 0; /* no more input to feed */ +} + static int run_transaction_hook(struct ref_transaction *transaction, const char *state) { - struct child_process proc = CHILD_PROCESS_INIT; - struct strbuf buf = STRBUF_INIT; - const char *hook; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct transaction_feed_cb_data feed_ctx = { 0 }; int ret = 0; - hook = find_hook(transaction->ref_store->repo, "reference-transaction"); - if (!hook) - return ret; + strvec_push(&opt.args, state); - strvec_pushl(&proc.args, hook, state, NULL); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = "reference-transaction"; + opt.feed_pipe = transaction_hook_feed_stdin; + opt.feed_pipe_ctx = transaction; + opt.feed_pipe_cb_data = &feed_ctx; - ret = start_command(&proc); - if (ret) - return ret; + strbuf_init(&feed_ctx.buf, 0); - sigchain_push(SIGPIPE, SIG_IGN); + ret = run_hooks_opt(transaction->ref_store->repo, "reference-transaction", &opt); - for (size_t i = 0; i < transaction->nr; i++) { - struct ref_update *update = transaction->updates[i]; - - if (update->flags & REF_LOG_ONLY) - continue; - - strbuf_reset(&buf); - - if (!(update->flags & REF_HAVE_OLD)) - strbuf_addf(&buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); - else if (update->old_target) - strbuf_addf(&buf, "ref:%s ", update->old_target); - else - strbuf_addf(&buf, "%s ", oid_to_hex(&update->old_oid)); - - if (!(update->flags & REF_HAVE_NEW)) - strbuf_addf(&buf, "%s ", oid_to_hex(null_oid(the_hash_algo))); - else if (update->new_target) - strbuf_addf(&buf, "ref:%s ", update->new_target); - else - strbuf_addf(&buf, "%s ", oid_to_hex(&update->new_oid)); - - strbuf_addf(&buf, "%s\n", update->refname); - - if (write_in_full(proc.in, buf.buf, buf.len) < 0) { - if (errno != EPIPE) { - /* Don't leak errno outside this API */ - errno = 0; - ret = -1; - } - break; - } - } - - close(proc.in); - sigchain_pop(SIGPIPE); - strbuf_release(&buf); - - ret |= finish_command(&proc); + strbuf_release(&feed_ctx.buf); return ret; } From e2ffb1ff043e0f8bd888f07a386e48b8fe9a9779 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 26 Dec 2025 14:23:30 +0200 Subject: [PATCH 07/14] hook: allow overriding the ungroup option When calling run_process_parallel() in run_hooks_opt(), the ungroup option is currently hardcoded to .ungroup = 1. This causes problems when ungrouping should be disabled, for example when sideband-reading collated output from child hooks, because sideband-reading and ungrouping are mutually exclusive. Thus a new hook.h option is added to allow overriding. The existing ungroup=1 behavior is preserved in the run_hooks() API and the "hook run" command. We could modify these to take an option if necessary, so I added two code comments there. Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- builtin/hook.c | 6 ++++++ commit.c | 3 +++ hook.c | 5 ++++- hook.h | 5 +++++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/builtin/hook.c b/builtin/hook.c index 7afec380d2..73e7b8c2e8 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -43,6 +43,12 @@ static int run(int argc, const char **argv, const char *prefix, if (!argc) goto usage; + /* + * All current "hook run" use-cases require ungrouped child output. + * If this changes, a hook run argument can be added to toggle it. + */ + opt.ungroup = 1; + /* * Having a -- for "run" when providing is * mandatory. diff --git a/commit.c b/commit.c index 28bb5ce029..efd0c02683 100644 --- a/commit.c +++ b/commit.c @@ -1978,6 +1978,9 @@ int run_commit_hook(int editor_is_used, const char *index_file, strvec_push(&opt.args, arg); va_end(args); + /* All commit hook use-cases require ungrouping child output. */ + opt.ungroup = 1; + opt.invoked_hook = invoked_hook; return run_hooks_opt(the_repository, name, &opt); } diff --git a/hook.c b/hook.c index 5ddd7678d1..00a1e2ad22 100644 --- a/hook.c +++ b/hook.c @@ -153,7 +153,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name, .tr2_label = hook_name, .processes = 1, - .ungroup = 1, + .ungroup = options->ungroup, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, @@ -198,6 +198,9 @@ int run_hooks(struct repository *r, const char *hook_name) { struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + /* All use-cases of this API require ungrouping. */ + opt.ungroup = 1; + return run_hooks_opt(r, hook_name, &opt); } diff --git a/hook.h b/hook.h index 2169d4a6bd..78a1a44690 100644 --- a/hook.h +++ b/hook.h @@ -34,6 +34,11 @@ struct run_hooks_opt */ int *invoked_hook; + /** + * Allow hooks to set run_processes_parallel() 'ungroup' behavior. + */ + unsigned int ungroup:1; + /** * Path to file which should be piped to stdin for each hook. */ From 5f14c14aecfeda119a1a4f5c4874f52619cb4652 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 26 Dec 2025 14:23:31 +0200 Subject: [PATCH 08/14] run-command: allow capturing of collated output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some callers, for example server-side hooks which wish to relay hook output to clients across a transport, want to capture what would normally print to stderr and do something else with it. Allow that via a callback. By calling the callback regardless of whether there's output available, we allow clients to send e.g. a keepalive if necessary. Because we expose a strbuf, not a fd or FILE*, there's no need to create a temporary pipe or similar - we can just skip the print to stderr and instead hand it to the caller. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- run-command.c | 30 ++++++++++++++++++++++-------- run-command.h | 17 +++++++++++++++++ t/helper/test-run-command.c | 15 +++++++++++++++ t/t0061-run-command.sh | 7 +++++++ 4 files changed, 61 insertions(+), 8 deletions(-) diff --git a/run-command.c b/run-command.c index aaf0e4ecee..2d3c2ac55c 100644 --- a/run-command.c +++ b/run-command.c @@ -1595,7 +1595,10 @@ static void pp_cleanup(struct parallel_processes *pp, * When get_next_task added messages to the buffer in its last * iteration, the buffered output is non empty. */ - strbuf_write(&pp->buffered_output, stderr); + if (opts->consume_output) + opts->consume_output(&pp->buffered_output, opts->data); + else + strbuf_write(&pp->buffered_output, stderr); strbuf_release(&pp->buffered_output); sigchain_pop_common(); @@ -1734,13 +1737,17 @@ static void pp_buffer_stderr(struct parallel_processes *pp, } } -static void pp_output(const struct parallel_processes *pp) +static void pp_output(const struct parallel_processes *pp, + const struct run_process_parallel_opts *opts) { size_t i = pp->output_owner; if (child_is_working(&pp->children[i]) && pp->children[i].err.len) { - strbuf_write(&pp->children[i].err, stderr); + if (opts->consume_output) + opts->consume_output(&pp->children[i].err, opts->data); + else + strbuf_write(&pp->children[i].err, stderr); strbuf_reset(&pp->children[i].err); } } @@ -1788,11 +1795,15 @@ static int pp_collect_finished(struct parallel_processes *pp, } else { const size_t n = opts->processes; - strbuf_write(&pp->children[i].err, stderr); + /* Output errors, then all other finished child processes */ + if (opts->consume_output) { + opts->consume_output(&pp->children[i].err, opts->data); + opts->consume_output(&pp->buffered_output, opts->data); + } else { + strbuf_write(&pp->children[i].err, stderr); + strbuf_write(&pp->buffered_output, stderr); + } strbuf_reset(&pp->children[i].err); - - /* Output all other finished child processes */ - strbuf_write(&pp->buffered_output, stderr); strbuf_reset(&pp->buffered_output); /* @@ -1829,7 +1840,7 @@ static void pp_handle_child_IO(struct parallel_processes *pp, pp->children[i].state = GIT_CP_WAIT_CLEANUP; } else { pp_buffer_stderr(pp, opts, output_timeout); - pp_output(pp); + pp_output(pp, opts); } } @@ -1852,6 +1863,9 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts) "max:%"PRIuMAX, (uintmax_t)opts->processes); + if (opts->ungroup && opts->consume_output) + BUG("ungroup and reading output are mutualy exclusive"); + /* * Child tasks might receive input via stdin, terminating early (or not), so * ignore the default SIGPIPE which gets handled by each feed_pipe_fn which diff --git a/run-command.h b/run-command.h index e1ca965b5b..7093252863 100644 --- a/run-command.h +++ b/run-command.h @@ -435,6 +435,17 @@ typedef int (*feed_pipe_fn)(int child_in, void *pp_cb, void *pp_task_cb); +/** + * If this callback is provided, output is collated into a new pipe instead + * of the process stderr. Then `consume_output_fn` will be called repeatedly + * with output contained in the `output` arg. It will also be called with an + * empty `output` to allow for keepalives or similar operations if necessary. + * + * pp_cb is the callback cookie as passed into run_processes_parallel. + * No task cookie is provided because the callback receives collated output. + */ +typedef void (*consume_output_fn)(struct strbuf *output, void *pp_cb); + /** * This callback is called on every child process that finished processing. * @@ -494,6 +505,12 @@ struct run_process_parallel_opts */ feed_pipe_fn feed_pipe; + /* + * consume_output: see consume_output_fn() above. This can be NULL + * to omit any special handling. + */ + consume_output_fn consume_output; + /** * task_finished: See task_finished_fn() above. This can be * NULL to omit any special handling. diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 4a56456894..49eace8dce 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -58,6 +58,16 @@ static int no_job(struct child_process *cp UNUSED, return 0; } +static void test_divert_output(struct strbuf *output, void *cb UNUSED) +{ + FILE *output_file; + + output_file = fopen("./output_file", "a"); + + strbuf_write(output, output_file); + fclose(output_file); +} + static int task_finished(int result UNUSED, struct strbuf *err, void *pp_cb UNUSED, @@ -198,6 +208,7 @@ static int testsuite(int argc, const char **argv) .get_next_task = next_test, .start_failure = test_failed, .feed_pipe = test_stdin_pipe_feed, + .consume_output = test_divert_output, .task_finished = test_finished, .data = &suite, }; @@ -514,6 +525,10 @@ int cmd__run_command(int argc, const char **argv) opts.get_next_task = parallel_next; opts.task_finished = task_finished_quiet; opts.feed_pipe = test_stdin_pipe_feed; + } else if (!strcmp(argv[1], "run-command-divert-output")) { + opts.get_next_task = parallel_next; + opts.consume_output = test_divert_output; + opts.task_finished = task_finished_quiet; } else { ret = 1; fprintf(stderr, "check usage\n"); diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 2f77fde0d9..74529e219e 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -164,6 +164,13 @@ test_expect_success 'run_command runs ungrouped in parallel with more tasks than test_line_count = 4 err ' +test_expect_success 'run_command can divert output' ' + test_when_finished rm output_file && + test-tool run-command run-command-divert-output 3 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual && + test_must_be_empty actual && + test_cmp expect output_file +' + test_expect_success 'run_command listens to stdin' ' cat >expect <<-\EOF && preloaded output of a child From 31bc927a9b8067c3cb2f2dd0455e1b2e8f2402f4 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 26 Dec 2025 14:23:32 +0200 Subject: [PATCH 09/14] hooks: allow callers to capture output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some server-side hooks will require capturing output to send over sideband instead of printing directly to stderr. Expose that capability. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- hook.c | 1 + hook.h | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/hook.c b/hook.c index 00a1e2ad22..35211e5ed7 100644 --- a/hook.c +++ b/hook.c @@ -158,6 +158,7 @@ int run_hooks_opt(struct repository *r, const char *hook_name, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, .feed_pipe = options->feed_pipe, + .consume_output = options->consume_output, .task_finished = notify_hook_finished, .data = &cb_data, diff --git a/hook.h b/hook.h index 78a1a44690..ae502178b9 100644 --- a/hook.h +++ b/hook.h @@ -80,6 +80,14 @@ struct run_hooks_opt * Only useful when using `run_hooks_opt.feed_pipe`, otherwise ignore it. */ void *feed_pipe_cb_data; + + /* + * 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_output_fn consume_output; }; #define RUN_HOOKS_OPT_INIT { \ From 4750485897b6681e0b6ca2caaadadaf660b4bb9c Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 26 Dec 2025 14:23:33 +0200 Subject: [PATCH 10/14] receive-pack: convert update hooks to new API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the new hook sideband API introduced in the previous commit. 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 current hook.[ch] via the run_process_parallel_opts.processes=1 arg. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 64 +++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 39 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 9c49174616..d1c40a768d 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -918,6 +918,16 @@ static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep) return 0; } +static void hook_output_to_sideband(struct strbuf *output, void *cb_data UNUSED) +{ + if (!output) + BUG("output must be non-NULL"); + + /* buffer might be empty for keepalives */ + if (output->len) + send_sideband(1, 2, output->buf, output->len, use_sideband); +} + static int run_receive_hook(struct command *commands, const char *hook_name, int skip_broken, @@ -941,29 +951,18 @@ static int run_receive_hook(struct command *commands, static int run_update_hook(struct command *cmd) { - struct child_process proc = CHILD_PROCESS_INIT; - int code; - const char *hook_path = find_hook(the_repository, "update"); + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; - 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)); - - proc.no_stdin = 1; - proc.stdout_to_stderr = 1; - proc.err = use_sideband ? -1 : 0; - proc.trace2_hook_name = "update"; - - code = start_command(&proc); - if (code) - return code; if (use_sideband) - copy_to_sideband(proc.err, -1, NULL); - return finish_command(&proc); + opt.consume_output = hook_output_to_sideband; + + return run_hooks_opt(the_repository, "update", &opt); } static struct command *find_command_by_refname(struct command *list, @@ -1640,33 +1639,20 @@ out: static void run_update_post_hook(struct command *commands) { struct command *cmd; - struct child_process proc = CHILD_PROCESS_INIT; - const char *hook; - - hook = find_hook(the_repository, "post-update"); - if (!hook) - return; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; 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) + opt.consume_output = hook_output_to_sideband; - 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); } static void check_aliased_update_internal(struct command *cmd, From 60a803639d37ecfae4906522b4692b68c821dc14 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Fri, 26 Dec 2025 14:23:34 +0200 Subject: [PATCH 11/14] receive-pack: convert receive hooks to hook API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This converts the last remaining hooks to the new hook API, for the same benefits as the previous conversions (no need to toggle signals, manage custom struct child_process, call find_hook(), prepares for specifyinig hooks via configs, etc.). I noticed a performance degradation when processing large amounts of hook input with just 1 line per callback, due to run-command's poll loop, therefore I batched 500 lines per callback, to ensure similar pipe throughput as before and to avoid hook child waiting on stdin. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- builtin/receive-pack.c | 220 ++++++++++++++++++----------------------- 1 file changed, 97 insertions(+), 123 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index d1c40a768d..ef1f77be8c 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -749,7 +749,7 @@ static int check_cert_push_options(const struct string_list *push_options) return retval; } -static void prepare_push_cert_sha1(struct child_process *proc) +static void prepare_push_cert_sha1(struct run_hooks_opt *opt) { static int already_done; @@ -775,23 +775,23 @@ static void prepare_push_cert_sha1(struct child_process *proc) nonce_status = check_nonce(sigcheck.payload); } if (!is_null_oid(&push_cert_oid)) { - strvec_pushf(&proc->env, "GIT_PUSH_CERT=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT=%s", oid_to_hex(&push_cert_oid)); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_SIGNER=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_SIGNER=%s", sigcheck.signer ? sigcheck.signer : ""); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_KEY=%s", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_KEY=%s", sigcheck.key ? sigcheck.key : ""); - strvec_pushf(&proc->env, "GIT_PUSH_CERT_STATUS=%c", + strvec_pushf(&opt->env, "GIT_PUSH_CERT_STATUS=%c", sigcheck.result); if (push_cert_nonce) { - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE=%s", push_cert_nonce); - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE_STATUS=%s", nonce_status); if (nonce_status == NONCE_SLOP) - strvec_pushf(&proc->env, + strvec_pushf(&opt->env, "GIT_PUSH_CERT_NONCE_SLOP=%ld", nonce_stamp_slop); } @@ -803,119 +803,64 @@ struct receive_hook_feed_state { struct ref_push_report *report; int skip_broken; struct strbuf buf; - const struct string_list *push_options; }; -typedef int (*feed_fn)(void *, const char **, size_t *); -static int run_and_feed_hook(const char *hook_name, feed_fn feed, - struct receive_hook_feed_state *feed_state) +static int feed_receive_hook_cb(int hook_stdin_fd, void *pp_cb UNUSED, void *pp_task_cb) { - struct child_process proc = CHILD_PROCESS_INIT; - struct async muxer; - int code; - const char *hook_path = find_hook(the_repository, hook_name); - - if (!hook_path) - return 0; - - strvec_push(&proc.args, hook_path); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = hook_name; - - if (feed_state->push_options) { - size_t i; - for (i = 0; i < feed_state->push_options->nr; i++) - strvec_pushf(&proc.env, - "GIT_PUSH_OPTION_%"PRIuMAX"=%s", - (uintmax_t)i, - feed_state->push_options->items[i].string); - strvec_pushf(&proc.env, "GIT_PUSH_OPTION_COUNT=%"PRIuMAX"", - (uintmax_t)feed_state->push_options->nr); - } else - strvec_pushf(&proc.env, "GIT_PUSH_OPTION_COUNT"); - - if (tmp_objdir) - strvec_pushv(&proc.env, tmp_objdir_env(tmp_objdir)); - - if (use_sideband) { - memset(&muxer, 0, sizeof(muxer)); - muxer.proc = copy_to_sideband; - muxer.in = -1; - code = start_async(&muxer); - if (code) - return code; - proc.err = muxer.in; - } - - prepare_push_cert_sha1(&proc); - - code = start_command(&proc); - if (code) { - if (use_sideband) - finish_async(&muxer); - return code; - } - - sigchain_push(SIGPIPE, SIG_IGN); - - while (1) { - const char *buf; - size_t n; - if (feed(feed_state, &buf, &n)) - break; - if (write_in_full(proc.in, buf, n) < 0) - break; - } - close(proc.in); - if (use_sideband) - finish_async(&muxer); - - sigchain_pop(SIGPIPE); - - return finish_command(&proc); -} - -static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep) -{ - struct receive_hook_feed_state *state = state_; + struct receive_hook_feed_state *state = pp_task_cb; struct command *cmd = state->cmd; + unsigned int lines_batch_size = 500; - while (cmd && - state->skip_broken && (cmd->error_string || cmd->did_not_exist)) - cmd = cmd->next; - if (!cmd) - return -1; /* EOF */ - if (!bufp) - return 0; /* OK, can feed something. */ strbuf_reset(&state->buf); - if (!state->report) - state->report = cmd->report; - if (state->report) { - struct object_id *old_oid; - struct object_id *new_oid; - const char *ref_name; - old_oid = state->report->old_oid ? state->report->old_oid : &cmd->old_oid; - new_oid = state->report->new_oid ? state->report->new_oid : &cmd->new_oid; - ref_name = state->report->ref_name ? state->report->ref_name : cmd->ref_name; - strbuf_addf(&state->buf, "%s %s %s\n", - oid_to_hex(old_oid), oid_to_hex(new_oid), - ref_name); - state->report = state->report->next; + /* batch lines to avoid going through run-command's poll loop for each line */ + for (unsigned int i = 0; i < lines_batch_size; i++) { + while (cmd && + state->skip_broken && (cmd->error_string || cmd->did_not_exist)) + cmd = cmd->next; + + if (!cmd) + break; /* no more commands left */ + if (!state->report) - state->cmd = cmd->next; - } else { - strbuf_addf(&state->buf, "%s %s %s\n", - oid_to_hex(&cmd->old_oid), oid_to_hex(&cmd->new_oid), - cmd->ref_name); - state->cmd = cmd->next; + state->report = cmd->report; + + if (state->report) { + struct object_id *old_oid; + struct object_id *new_oid; + const char *ref_name; + + old_oid = state->report->old_oid ? state->report->old_oid : &cmd->old_oid; + new_oid = state->report->new_oid ? state->report->new_oid : &cmd->new_oid; + ref_name = state->report->ref_name ? state->report->ref_name : cmd->ref_name; + + strbuf_addf(&state->buf, "%s %s %s\n", + oid_to_hex(old_oid), oid_to_hex(new_oid), + ref_name); + + state->report = state->report->next; + if (!state->report) + cmd = cmd->next; + } else { + strbuf_addf(&state->buf, "%s %s %s\n", + oid_to_hex(&cmd->old_oid), oid_to_hex(&cmd->new_oid), + cmd->ref_name); + cmd = cmd->next; + } } - if (bufp) { - *bufp = state->buf.buf; - *sizep = state->buf.len; + + state->cmd = cmd; + + if (state->buf.len > 0) { + int ret = write_in_full(hook_stdin_fd, state->buf.buf, state->buf.len); + if (ret < 0) { + if (errno == EPIPE) + return 1; /* child closed pipe */ + return ret; + } } - return 0; + + return state->cmd ? 0 : 1; /* 0 = more to come, 1 = EOF */ } static void hook_output_to_sideband(struct strbuf *output, void *cb_data UNUSED) @@ -933,20 +878,49 @@ static int run_receive_hook(struct command *commands, int skip_broken, const struct string_list *push_options) { - struct receive_hook_feed_state state; - int status; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct command *iter = commands; + struct receive_hook_feed_state feed_state; + int ret; - strbuf_init(&state.buf, 0); - state.cmd = commands; - state.skip_broken = skip_broken; - state.report = NULL; - if (feed_receive_hook(&state, NULL, NULL)) + /* if there are no valid commands, don't invoke the hook at all. */ + while (iter && skip_broken && (iter->error_string || iter->did_not_exist)) + iter = iter->next; + if (!iter) return 0; - state.cmd = commands; - state.push_options = push_options; - status = run_and_feed_hook(hook_name, feed_receive_hook, &state); - strbuf_release(&state.buf); - return status; + + if (push_options) { + for (int i = 0; i < push_options->nr; i++) + strvec_pushf(&opt.env, "GIT_PUSH_OPTION_%d=%s", i, + push_options->items[i].string); + strvec_pushf(&opt.env, "GIT_PUSH_OPTION_COUNT=%"PRIuMAX"", + (uintmax_t)push_options->nr); + } else { + strvec_push(&opt.env, "GIT_PUSH_OPTION_COUNT"); + } + + if (tmp_objdir) + strvec_pushv(&opt.env, tmp_objdir_env(tmp_objdir)); + + prepare_push_cert_sha1(&opt); + + /* set up sideband printer */ + if (use_sideband) + opt.consume_output = hook_output_to_sideband; + + /* set up stdin callback */ + feed_state.cmd = commands; + feed_state.skip_broken = skip_broken; + feed_state.report = NULL; + strbuf_init(&feed_state.buf, 0); + opt.feed_pipe_cb_data = &feed_state; + opt.feed_pipe = feed_receive_hook_cb; + + ret = run_hooks_opt(the_repository, hook_name, &opt); + + strbuf_release(&feed_state.buf); + + return ret; } static int run_update_hook(struct command *cmd) From 465c604cef62a228d3699d89b8568ab8ab80da74 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Fri, 9 Jan 2026 20:19:12 +0200 Subject: [PATCH 12/14] hook: check for NULL pointer before deref Fix a compiler warning (-Werror=analyzer-deref-before-check) due to dereferencing the options pointer before NULL checking it. In practice run_hooks_opt() is never called with a NULL opt struct, so this just fixes the code to not trigger the warning anymore. The NULL check is kept as-is because some future patches might end up calling run_hooks_opt with a NULL opt struct, which is clearly a bug. While at it, also fix the BUG message function name. Reported-by: correctmost Suggested-by: Patrick Steinhardt Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- hook.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hook.c b/hook.c index 35211e5ed7..6b8ddfe7b6 100644 --- a/hook.c +++ b/hook.c @@ -148,28 +148,29 @@ int run_hooks_opt(struct repository *r, const char *hook_name, }; const char *const hook_path = find_hook(r, hook_name); int ret = 0; - const struct run_process_parallel_opts opts = { + struct run_process_parallel_opts opts = { .tr2_category = "hook", .tr2_label = hook_name, .processes = 1, - .ungroup = options->ungroup, .get_next_task = pick_next_hook, .start_failure = notify_start_failure, - .feed_pipe = options->feed_pipe, - .consume_output = options->consume_output, .task_finished = notify_hook_finished, .data = &cb_data, }; if (!options) - BUG("a struct run_hooks_opt must be provided to run_hooks"); + BUG("a struct run_hooks_opt must be provided to run_hooks_opt"); if (options->path_to_stdin && options->feed_pipe) BUG("options path_to_stdin and feed_pipe are mutually exclusive"); + opts.ungroup = options->ungroup; + opts.feed_pipe = options->feed_pipe; + opts.consume_output = options->consume_output; + if (options->invoked_hook) *options->invoked_hook = 0; From 1bb18a00dacd80c562bd9c1f74a92b122faaccb3 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Wed, 14 Jan 2026 20:57:30 +0200 Subject: [PATCH 13/14] hook: allow hooks to disable stdout_to_stderr The last batch of hooks converted to the hook.[ch] API introduced a regression because pick_next_hook() always sets stdout_to_stderr for its child processes. Pre-push is the only hook API user which requires stdout_to_stderr to be 0, so it can be argued that pre-push needs fixing, however this will likely break many pre-push hooks, so it's better to allow it to be 0, i.e. to match the previous behavior. To prevent such regressions in the future, extend the hook tests to verify hooks write to the expected stdout vs stderr streams and maintain backward compatibility with the hooks output assumptions. The tests are independent of the actual hook implementations: I've tested they work the same before and after the hook.[ch] conversion and will continue to work after we eventually introduce parallel hook execution and config-based hooks. Fixes: 3e2836a742d8 ("transport: convert pre-push to hook API") Reported-by: Chris Darroch Suggested-by: brian m. carlson Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- hook.c | 2 +- hook.h | 6 +++ t/t1800-hook.sh | 127 ++++++++++++++++++++++++++++++++++++++++++++++++ transport.c | 6 +++ 4 files changed, 140 insertions(+), 1 deletion(-) diff --git a/hook.c b/hook.c index 6b8ddfe7b6..47feff9cd7 100644 --- a/hook.c +++ b/hook.c @@ -81,7 +81,7 @@ static int pick_next_hook(struct child_process *cp, cp->in = -1; } - cp->stdout_to_stderr = 1; + cp->stdout_to_stderr = hook_cb->options->stdout_to_stderr; cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; diff --git a/hook.h b/hook.h index ae502178b9..2488db7133 100644 --- a/hook.h +++ b/hook.h @@ -39,6 +39,11 @@ struct run_hooks_opt */ unsigned int ungroup:1; + /** + * Send the hook's stdout to stderr. + */ + unsigned int stdout_to_stderr:1; + /** * Path to file which should be piped to stdin for each hook. */ @@ -93,6 +98,7 @@ struct run_hooks_opt #define RUN_HOOKS_OPT_INIT { \ .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ + .stdout_to_stderr = 1, \ } struct hook_cb_data { diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 4feaf0d7be..0e4f93fb31 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -184,4 +184,131 @@ test_expect_success 'stdin to hooks' ' test_cmp expect actual ' +check_stdout_separate_from_stderr () { + for hook in "$@" + do + test_grep ! "Hook $hook stdout" stderr.actual && + test_grep ! "Hook $hook stderr" stdout.actual && + test_grep "Hook $hook stderr" stderr.actual && + test_grep "Hook $hook stdout" stdout.actual || return 1 + done +} + +check_stdout_merged_to_stderr () { + test_grep ! "Hook .* stdout" stdout.actual && + test_grep ! "Hook .* stderr" stdout.actual && + for hook in "$@" + do + test_grep "Hook $hook stdout" stderr.actual && + test_grep "Hook $hook stderr" stderr.actual || return 1 + done +} + +test_expect_success 'client pre-push hook expects separate stdout and stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init --bare remote && + git remote add origin remote && + test_commit A && + + hook=pre-push && + test_hook $hook <<-EOF && + echo >&1 Hook $hook stdout + echo >&2 Hook $hook stderr + EOF + + git push origin HEAD:main >stdout.actual 2>stderr.actual && + check_stdout_separate_from_stderr pre-push +' + +test_expect_success 'client hooks expect stdout redirected to stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + for hook in pre-commit post-commit post-checkout pre-merge-commit \ + prepare-commit-msg commit-msg post-merge post-rewrite reference-transaction \ + applypatch-msg pre-applypatch post-applypatch pre-rebase post-index-change + do + test_hook $hook <<-EOF || return 1 + echo >&1 Hook $hook stdout + echo >&2 Hook $hook stderr + EOF + done && + + git checkout -B main && + git checkout -b branch-a && + test_commit commit-on-branch-a && + + # Trigger pre-commit, prepare-commit-msg, commit-msg, post-commit, reference-transaction + git commit --allow-empty -m "Test" >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-commit prepare-commit-msg commit-msg post-commit reference-transaction && + + # Trigger post-checkout, reference-transaction + git checkout -b new-branch main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-checkout reference-transaction && + + # Trigger pre-merge-commit, post-merge, reference-transaction + test_commit new-branch-commit && + git merge --no-ff branch-a >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-merge-commit post-merge reference-transaction && + + # Trigger post-rewrite, reference-transaction + git commit --amend --allow-empty --no-edit >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-rewrite reference-transaction && + + # Trigger applypatch-msg, pre-applypatch, post-applypatch + git checkout -b branch-b main && + test_commit branch-b && + git format-patch -1 --stdout >patch && + git checkout -b branch-c main && + git am patch >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr applypatch-msg pre-applypatch post-applypatch && + + # Trigger pre-rebase + git checkout -b branch-d main && + test_commit branch-d && + git checkout main && + test_commit diverge-main && + git checkout branch-d && + git rebase main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-rebase && + + # Trigger post-index-change + oid=$(git hash-object -w --stdin stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr post-index-change +' + +test_expect_success 'server hooks expect stdout redirected to stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init --bare remote-server && + git remote add origin-server remote-server && + + for hook in pre-receive update post-receive post-update + do + write_script remote-server/hooks/$hook <<-EOF || return 1 + echo >&1 Hook $hook stdout + echo >&2 Hook $hook stderr + EOF + done && + + # Trigger pre-receive update post-receive post-update + git push origin-server HEAD:new-branch >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr pre-receive update post-receive post-update +' + +test_expect_success 'server push-to-checkout hook expects stdout redirected to stderr' ' + test_when_finished "rm -f stdout.actual stderr.actual" && + git init server && + git -C server checkout -b main && + test_config -C server receive.denyCurrentBranch updateInstead && + git remote add origin-server-2 server && + + write_script server/.git/hooks/push-to-checkout <<-EOF && + echo >&1 Hook push-to-checkout stdout + echo >&2 Hook push-to-checkout stderr + EOF + + # Trigger push-to-checkout + git push origin-server-2 HEAD:main >stdout.actual 2>stderr.actual && + check_stdout_merged_to_stderr push-to-checkout +' + test_done diff --git a/transport.c b/transport.c index 6d0f02be5d..e876cc9189 100644 --- a/transport.c +++ b/transport.c @@ -1373,6 +1373,12 @@ static int run_pre_push_hook(struct transport *transport, opt.feed_pipe = pre_push_hook_feed_stdin; opt.feed_pipe_cb_data = &data; + /* + * pre-push hooks expect stdout & stderr to be separate, so don't merge + * them to keep backwards compatibility with existing hooks. + */ + opt.stdout_to_stderr = 0; + ret = run_hooks_opt(the_repository, "pre-push", &opt); strbuf_release(&data.buf); From d075ed42205843917913ab6d1c9b02657eb01191 Mon Sep 17 00:00:00 2001 From: Adrian Ratiu Date: Wed, 14 Jan 2026 20:57:31 +0200 Subject: [PATCH 14/14] hook: make ungroup opt-out instead of opt-in In 857f047e40 (hook: allow overriding the ungroup option, 2025-12-26), I accidentally made the ungroup option opt-in instead of opt-out and despite my best efforts to set it for all API users, I missed a case which requires it to be set: the pre-push hook which regressed. The only thing I needed in that commit was a way to change the default, to convert the remaining receive-pack hooks which require ungroup == 0 for sideband output, so it doesn't matter if it's on or off by default. Bring back the original behavior by setting it for all hooks in the struct run_hooks_opt initializer, which nicely allows changing the default value only where needed, in receive-pack.c. While at it add a few hook tests which exercise receive-pack sideband output since they are the only ungroup=0 exceptions and there are no other tests exercising this functionality. Fixes: 857f047e40f7 ("hook: allow overriding the ungroup option") Reported-by: Kristoffer Haugsbakk Suggested-by: Jeff King Signed-off-by: Adrian Ratiu Signed-off-by: Junio C Hamano --- builtin/hook.c | 6 ------ builtin/receive-pack.c | 12 ++++++++--- commit.c | 3 --- hook.c | 3 --- hook.h | 1 + t/t1800-hook.sh | 49 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 59 insertions(+), 15 deletions(-) diff --git a/builtin/hook.c b/builtin/hook.c index 73e7b8c2e8..7afec380d2 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -43,12 +43,6 @@ static int run(int argc, const char **argv, const char *prefix, if (!argc) goto usage; - /* - * All current "hook run" use-cases require ungrouped child output. - * If this changes, a hook run argument can be added to toggle it. - */ - opt.ungroup = 1; - /* * Having a -- for "run" when providing is * mandatory. diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index ef1f77be8c..2d1a94f3a9 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -905,8 +905,10 @@ static int run_receive_hook(struct command *commands, prepare_push_cert_sha1(&opt); /* set up sideband printer */ - if (use_sideband) + if (use_sideband) { opt.consume_output = hook_output_to_sideband; + opt.ungroup = 0; /* mandatory for sideband output */ + } /* set up stdin callback */ feed_state.cmd = commands; @@ -933,8 +935,10 @@ static int run_update_hook(struct command *cmd) oid_to_hex(&cmd->new_oid), NULL); - if (use_sideband) + if (use_sideband) { opt.consume_output = hook_output_to_sideband; + opt.ungroup = 0; /* mandatory for sideband output */ + } return run_hooks_opt(the_repository, "update", &opt); } @@ -1623,8 +1627,10 @@ static void run_update_post_hook(struct command *commands) if (!opt.args.nr) return; - if (use_sideband) + if (use_sideband) { opt.consume_output = hook_output_to_sideband; + opt.ungroup = 0; /* mandatory for sideband output */ + } run_hooks_opt(the_repository, "post-update", &opt); } diff --git a/commit.c b/commit.c index efd0c02683..28bb5ce029 100644 --- a/commit.c +++ b/commit.c @@ -1978,9 +1978,6 @@ int run_commit_hook(int editor_is_used, const char *index_file, strvec_push(&opt.args, arg); va_end(args); - /* All commit hook use-cases require ungrouping child output. */ - opt.ungroup = 1; - opt.invoked_hook = invoked_hook; return run_hooks_opt(the_repository, name, &opt); } diff --git a/hook.c b/hook.c index 47feff9cd7..bbf7f0df21 100644 --- a/hook.c +++ b/hook.c @@ -200,9 +200,6 @@ int run_hooks(struct repository *r, const char *hook_name) { struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; - /* All use-cases of this API require ungrouping. */ - opt.ungroup = 1; - return run_hooks_opt(r, hook_name, &opt); } diff --git a/hook.h b/hook.h index 2488db7133..b2b4db2b3d 100644 --- a/hook.h +++ b/hook.h @@ -99,6 +99,7 @@ struct run_hooks_opt .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ .stdout_to_stderr = 1, \ + .ungroup = 1, \ } struct hook_cb_data { diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 0e4f93fb31..0555a1fd42 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -311,4 +311,53 @@ test_expect_success 'server push-to-checkout hook expects stdout redirected to s check_stdout_merged_to_stderr push-to-checkout ' +test_expect_success 'receive-pack hooks sideband output works' ' + git init --bare target-sideband.git && + test_commit sideband-a && + git remote add origin-sideband ./target-sideband.git && + + # pre-receive hook + test_hook -C target-sideband.git pre-receive <<-\EOF && + echo "stdout pre-receive" + echo "stderr pre-receive" >&2 + EOF + + git push origin-sideband HEAD:refs/heads/pre-receive 2>actual && + test_grep "remote: stdout pre-receive" actual && + test_grep "remote: stderr pre-receive" actual && + + # update hook + test_hook -C target-sideband.git update <<-\EOF && + echo "stdout update" + echo "stderr update" >&2 + EOF + + test_commit sideband-b && + git push origin-sideband HEAD:refs/heads/update 2>actual && + test_grep "remote: stdout update" actual && + test_grep "remote: stderr update" actual && + + # post-receive hook + test_hook -C target-sideband.git post-receive <<-\EOF && + echo >&1 "stdout post-receive" + echo >&2 "stderr post-receive" + EOF + + test_commit sideband-c && + git push origin-sideband HEAD:refs/heads/post-receive 2>actual && + test_grep "remote: stdout post-receive" actual && + test_grep "remote: stderr post-receive" actual && + + # post-update hook + test_hook -C target-sideband.git post-update <<-\EOF && + echo >&1 "stdout post-update" + echo >&2 "stderr post-update" + EOF + + test_commit sideband-d && + git push origin-sideband HEAD:refs/heads/post-update 2>actual && + test_grep "remote: stdout post-update" actual && + test_grep "remote: stderr post-update" actual +' + test_done