From: Phillip Wood Date: Thu, 20 Aug 2026 09:50:06 GMT Subject: Re: [PATCH] hook: introduce the report hook for git-receive-pack(1) Message-ID: <29f39d8b-6cf0-4811-afb3-0a1656877f31@gmail.com> In-Reply-To: <20260818-758-introduce-hook-v1-1-8a8d89e65838@gmail.com> Hi Karthik On 18/08/2026 08:55, Karthik Nayak wrote: > > Similar to the 'proc-receive' hook, this does not use the config-based > hook infrastructure. That infrastructure is designed for parallelizable > notification hooks. As this hook is a bidirectional filter, it would > require significant modifications to that infrastructure and this hook > cannot be parallelized anyway. Config based hooks are about running more than one script to run per hook event, they're not about parallel execution per-se. Indeed the documentation for git hook notes Some hooks always run sequentially regardless of this flag or the hook.jobs config, because Git knows they cannot safely run in parallel: applypatch-msg, pre-commit, prepare-commit-msg, commit- msg, post-commit, post-checkout, and push-to-checkout. I think the question the commit message should be answering is, whether a design like proc-receive that predates config based hooks and only allows a single hook script, makes sense now that we have config based hooks, or, if we were adding that functionality now, would we design it differently? I think the answer for server side hooks is that a design around a single script is probably reasonable but it would be worth discussing that in the commit message. Thanks Phillip > Signed-off-by: Karthik Nayak > --- > To give some context, we at GitLab are building a custom MVCC around > Git. Each git-push would initialize a new version which is then > committed as the default post some operations. These operations take > place after the reference transaction and based on the output status of > those operations, we want to propagate the status to the user. There > currently exists no good mechanism to do so. > > Having a report hook which allows us to modify the report being > propagated to the user, allows us to modify the report based on the > status of our MVCC commit phase. > --- > Documentation/githooks.adoc | 23 ++++++ > builtin/receive-pack.c | 41 +++++++++++ > t/meson.build | 1 + > t/t5412-report-hook.sh | 176 ++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 241 insertions(+) > > diff --git a/Documentation/githooks.adoc b/Documentation/githooks.adoc > index ed045940d1..7e6643ad89 100644 > --- a/Documentation/githooks.adoc > +++ b/Documentation/githooks.adoc > @@ -527,6 +527,29 @@ The exit status of the hook is ignored for any state except for the > status will cause the transaction to be aborted. The hook will not be > called with "aborted" state in that case. > > +report > +~~~~~~ > + > +This hook is invoked by linkgit:git-receive-pack[1] when it reacts to > +`git push` and updates reference(s) in its repository. It executes on > +the remote repository once after all refs have been updated, but before > +the status report is sent back to the client. > + > +The hook receives the pkt-line encoded status report on standard input > +and its standard output replaces the report sent to the client. Any > +output written to standard error is forwarded to the client over the > +sideband channel and will appear as `remote:` lines on the client's > +terminal. To reject individual ref updates, rewrite the corresponding > +`ok` lines to `ng` lines in the output report (with an explanatory > +error string) and exit zero; standard error can accompany this to > +provide a human-readable explanation. A non-zero exit status causes > +`receive-pack` to die. > + > +Note that by the time this hook runs, all ref updates have already been > +applied to the repository. A non-zero exit causes the client to see the > +push as failed, but does *not* roll back any ref changes that were > +already committed server-side. > + > push-to-checkout > ~~~~~~~~~~~~~~~~ > > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c > index 86933d8d7e..bc22b3ec31 100644 > --- a/builtin/receive-pack.c > +++ b/builtin/receive-pack.c > @@ -1004,6 +1004,41 @@ static int run_update_hook(struct command *cmd) > return code; > } > > +static int run_report_hook(struct strbuf *report) > +{ > + struct child_process proc = CHILD_PROCESS_INIT; > + struct async sideband_async; > + int sideband_async_started = 0; > + int saved_stderr = -1; > + struct strbuf out = STRBUF_INIT; > + const char *hook_path; > + int code; > + > + hook_path = find_hook(the_repository, "report"); > + if (!hook_path) > + return 0; > + > + strvec_push(&proc.args, hook_path); > + proc.trace2_hook_name = "report"; > + > + prepare_sideband_async(&sideband_async, &saved_stderr, > + &sideband_async_started); > + > + sigchain_push(SIGPIPE, SIG_IGN); > + code = pipe_command(&proc, report->buf, report->len, &out, > + report->len, NULL, 0); > + sigchain_pop(SIGPIPE); > + > + finish_sideband_async(&sideband_async, saved_stderr, > + sideband_async_started); > + > + if (!code) > + strbuf_swap(&out, report); > + > + strbuf_release(&out); > + return code; > +} > + > static struct command *find_command_by_refname(struct command *list, > const char *refname) > { > @@ -2547,6 +2582,9 @@ static void report(struct command *commands, const char *unpack_status) > } > packet_buf_flush(&buf); > > + if (run_report_hook(&buf)) > + die("report hook failed"); > + > if (use_sideband) > send_sideband(1, 1, buf.buf, buf.len, use_sideband); > else > @@ -2592,6 +2630,9 @@ static void report_v2(struct command *commands, const char *unpack_status) > } > packet_buf_flush(&buf); > > + if (run_report_hook(&buf)) > + die("report hook failed"); > + > if (use_sideband) > send_sideband(1, 1, buf.buf, buf.len, use_sideband); > else > diff --git a/t/meson.build b/t/meson.build > index a25f37d2f5..7056e31326 100644 > --- a/t/meson.build > +++ b/t/meson.build > @@ -651,6 +651,7 @@ integration_tests = [ > 't5409-colorize-remote-messages.sh', > 't5410-receive-pack.sh', > 't5411-proc-receive-hook.sh', > + 't5412-report-hook.sh', > 't5500-fetch-pack.sh', > 't5501-fetch-push-alternates.sh', > 't5502-quickfetch.sh', > diff --git a/t/t5412-report-hook.sh b/t/t5412-report-hook.sh > new file mode 100755 > index 0000000000..47f20e8d67 > --- /dev/null > +++ b/t/t5412-report-hook.sh > @@ -0,0 +1,176 @@ > +#!/bin/sh > + > +test_description='test report hook' > + > +GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main > +export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > + > +. ./test-lib.sh > + > +. "$TEST_DIRECTORY"/t5411/common-functions.sh > + > +URL_PREFIX="\.\." > + > +test_expect_success "setup workbench" ' > + git init workbench && > + create_commits_in workbench A B > +' > + > +test_expect_success "no report hook, push succeeds" ' > + test_when_finished "rm -rf upstream" && > + test_when_finished "git -C workbench remote remove origin" && > + git init --bare upstream && > + > + git -C workbench remote add origin ../upstream && > + git -C workbench push origin $A:refs/heads/main && > + git -C workbench push origin $B:refs/heads/main >out 2>&1 && > + > + make_user_friendly_and_stable_output actual && > + cat >expect <<-\EOF && > + To ../upstream > + .. -> main > + EOF > + test_cmp expect actual > +' > + > +test_expect_success "passthrough does not alter report" ' > + test_when_finished "rm -rf upstream" && > + test_when_finished "git -C workbench remote remove origin" && > + git init --bare upstream && > + > + test_hook -C upstream --setup report <<-\EOF && > + cat > + EOF > + > + git -C workbench remote add origin ../upstream && > + git -C workbench push origin $A:refs/heads/main && > + git -C workbench push origin $B:refs/heads/main >out 2>&1 && > + > + make_user_friendly_and_stable_output actual && > + cat >expect <<-\EOF && > + To ../upstream > + .. -> main > + EOF > + test_cmp expect actual > +' > + > +test_expect_success "non-zero exit causes receive-pack to die" ' > + test_when_finished "rm -rf upstream" && > + test_when_finished "git -C workbench remote remove origin" && > + > + git init --bare upstream && > + git -C workbench remote add origin ../upstream && > + git -C workbench push origin $A:refs/heads/main && > + > + test_hook -C upstream --setup report <<-\EOF && > + exit 1 > + EOF > + > + test_must_fail git -C workbench push origin $B:refs/heads/main >out 2>&1 && > + make_user_friendly_and_stable_output actual && > + cat >expect <<-\EOF && > + fatal: report hook failed > + send-pack: unexpected disconnect while reading sideband packet > + fatal: the remote end hung up unexpectedly > + EOF > + test_cmp expect actual > +' > + > +test_expect_success "hook is invoked and receives report on stdin" ' > + test_when_finished "rm -rf upstream" && > + test_when_finished "git -C workbench remote remove origin" && > + > + git init --bare upstream && > + test_hook -C upstream --setup report <<-EOF && > + tee raw > + EOF > + > + git -C workbench remote add origin ../upstream && > + git -C workbench push origin $A:refs/heads/main && > + git -C workbench push origin $B:refs/heads/main >out 2>&1 && > + > + make_user_friendly_and_stable_output actual && > + cat >expect <<-EOF && > + To ../upstream > + .. -> main > + EOF > + test_cmp expect actual && > + > + test-tool pkt-line unpack actual-report && > + cat >expect-report <<-EOF && > + unpack ok > + ok refs/heads/main > + 0000 > + EOF > + test_cmp expect-report actual-report > +' > + > +test_expect_success "hook can modify the report sent to client" ' > + test_when_finished "rm -rf upstream" && > + test_when_finished "git -C workbench remote remove origin" && > + > + git init --bare upstream && > + git -C workbench remote add origin ../upstream && > + git -C workbench push origin $A:refs/heads/main && > + > + test_hook -C upstream --setup report <<-\EOF && > + test-tool pkt-line unpack | > + sed "s/^ok /ng /" | > + test-tool pkt-line pack > + EOF > + > + test_must_fail git -C workbench push origin $B:refs/heads/main >out 2>&1 && > + make_user_friendly_and_stable_output actual && > + cat >expect <<-\EOF && > + To ../upstream > + ! [remote rejected] -> main (failed) > + EOF > + test_cmp expect actual > +' > + > +test_expect_success "hook can report a custom failure message" ' > + test_when_finished "rm -rf upstream" && > + test_when_finished "git -C workbench remote remove origin" && > + > + git init --bare upstream && > + git -C workbench remote add origin ../upstream && > + git -C workbench push origin $A:refs/heads/main && > + > + test_hook -C upstream --setup report <<-\EOF && > + echo "push rejected: service X is down" >&2 > + test-tool pkt-line unpack | > + sed "s/^ok \(.*\)/ng \1 service-x-is-down/" | > + test-tool pkt-line pack | > + tee raw > + EOF > + > + test_must_fail git -C workbench push origin $B:refs/heads/main >out 2>&1 && > + test_grep "push rejected: service X is down" out && > + > + test-tool pkt-line unpack actual-report && > + cat >expect-report <<-\EOF && > + unpack ok > + ng refs/heads/main service-x-is-down > + 0000 > + EOF > + test_cmp expect-report actual-report > +' > + > +test_expect_success "hook stderr is relayed to client via sideband" ' > + test_when_finished "rm -rf upstream" && > + test_when_finished "git -C workbench remote remove origin" && > + > + git init --bare upstream && > + git -C workbench remote add origin ../upstream && > + git -C workbench push origin $A:refs/heads/main && > + > + test_hook -C upstream --setup report <<-\EOF && > + echo "hook-stderr-message" >&2 > + exit 1 > + EOF > + > + test_must_fail git -C workbench push origin $B:refs/heads/main >out 2>&1 && > + test_grep "hook-stderr-message" out > +' > + > +test_done > > --- > base-commit: 11c6700f10234578d10523faf35656ca491425c9 > change-id: 20260812-758-introduce-hook-5b3af9f1a7e8 > > > Thanks > - Karthik > >