From: Phillip Wood <phillip.wood123@gmail.com>
To: Karthik Nayak <karthik.188@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH] hook: introduce the report hook for git-receive-pack(1)
Date: Thu, 20 Aug 2026 10:50:06 +0100 [thread overview]
Message-ID: <29f39d8b-6cf0-4811-afb3-0a1656877f31@gmail.com> (raw)
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 <karthik.188@gmail.com>
> ---
> 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 <out >actual &&
> + cat >expect <<-\EOF &&
> + To ../upstream
> + <COMMIT-A>..<COMMIT-B> <COMMIT-B> -> 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 <out >actual &&
> + cat >expect <<-\EOF &&
> + To ../upstream
> + <COMMIT-A>..<COMMIT-B> <COMMIT-B> -> 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 <out >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 <out >actual &&
> + cat >expect <<-EOF &&
> + To ../upstream
> + <COMMIT-A>..<COMMIT-B> <COMMIT-B> -> main
> + EOF
> + test_cmp expect actual &&
> +
> + test-tool pkt-line unpack <upstream/raw >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 <out >actual &&
> + cat >expect <<-\EOF &&
> + To ../upstream
> + ! [remote rejected] <COMMIT-B> -> 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 <upstream/raw >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
>
>
next prev parent reply other threads:[~2026-08-20 9:50 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-18 7:55 [PATCH] hook: introduce the report hook for git-receive-pack(1) Karthik Nayak
2026-08-18 20:54 ` Junio C Hamano
2026-08-19 7:03 ` Kristoffer Haugsbakk
2026-08-19 12:11 ` Karthik Nayak
2026-08-19 14:47 ` Kristoffer Haugsbakk
2026-08-19 7:39 ` Patrick Steinhardt
2026-08-19 13:13 ` Karthik Nayak
2026-08-19 13:20 ` Patrick Steinhardt
2026-08-19 13:24 ` Karthik Nayak
2026-08-20 9:50 ` Phillip Wood [this message]
2026-08-20 15:43 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=29f39d8b-6cf0-4811-afb3-0a1656877f31@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=git@vger.kernel.org \
--cc=karthik.188@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox