Re: [PATCH] hook: introduce the report hook for git-receive-pack(1)
Phillip Wood <[email protected]>
| Newsgroups | org.kernel.vger.git |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
> ---
> 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
>
>