* [PATCH] hook: introduce the report hook for git-receive-pack(1)
@ 2026-08-18 7:55 Karthik Nayak
2026-08-18 20:54 ` Junio C Hamano
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Karthik Nayak @ 2026-08-18 7:55 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak
When running 'git-receive-pack(1)', there is currently no way for the
server to intercept and modify the status report before it is sent back
to the client. This is useful for servers with custom logic that need
to transform or gate the report based on the outcome of external logic
post reference updates.
Introduce a new 'report' hook which receives the pkt-line encoded
status report on stdin and whose stdout replaces the report sent to the
client. A non-zero exit status causes `receive-pack` to die and the
client to treat the push as failed.
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.
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] hook: introduce the report hook for git-receive-pack(1)
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 7:39 ` Patrick Steinhardt
2 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2026-08-18 20:54 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git
Karthik Nayak <karthik.188@gmail.com> writes:
> When running 'git-receive-pack(1)', there is currently no way for the
> server to intercept and modify the status report before it is sent back
> to the client. This is useful for servers with custom logic that need
> to transform or gate the report based on the outcome of external logic
> post reference updates.
One sentence is missing. The fact that there is no way for the
server to customize the report is not useful, but that is how the
above reads. Drop "currently", as the introductory observation is
always about the status, explaining what is missing to make
readers realize why they may want the new feature introduced by the
change.
> Introduce a new 'report' hook which receives the pkt-line encoded
> status report on stdin and whose stdout replaces the report sent to the
> client. A non-zero exit status causes `receive-pack` to die and the
> client to treat the push as failed.
After getting asked to accept a push to three refs and receiving
the object transfer, the hook can say "I'll let these two refs be
updated, but refuse to update the other one" and return success by
exiting 0. What does the other side of the connection see? Two
successes with one rejection, I guess. If the hook instead rewrites
the report to say "all three ref updates were rejected" and returns
success, then what does the other side see? Failures on all three
refs, right?
What should happen when the hook says "all three ref updates were
accepted and they updated to point at objects X, Y, Z", but the hook
itself exits with a non-zero status? How does the other side tell
if their push succeeded (as described in the returned report) or
failed (as receive-pack(1) noticed the hook's exit status was not
0)?
How is the failure due to the hook's exit status propagated back to
the other side of the connection? Does receive-pack(1) hold on to
the report until the hook dies, and if it dies with status 0 give
that report back to 'git push'? And if it dies with a non-zero
status, then what? Ignore the report and send a failure report
generated on its own?
The observation made in the preceding paragraphs shows that allowing
the exit status of the hook to further affect the outcome is a bit
iffy as a design to define what a "failure" is, unless it is more
tightly described. The hook can signal failure in its report
output without exiting with a non-zero status at all, and if
receive-pack(1) wants to allow the exit code of the hook to affect
the outcome, it cannot stream the report back to 'git push' as it
receives it from the hook.
> @@ -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
OK. The other side does not even hear the report if the hook
aborts. And lack of success report is what the other side
interprets as a failure.
Ugly, but may work OK. Needs to be documented a bit more clearly,
though.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] hook: introduce the report hook for git-receive-pack(1)
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 7:39 ` Patrick Steinhardt
2 siblings, 1 reply; 5+ messages in thread
From: Kristoffer Haugsbakk @ 2026-08-19 7:03 UTC (permalink / raw)
To: Karthik Nayak, git
On Tue, Aug 18, 2026, at 09:55, Karthik Nayak wrote:
> When running 'git-receive-pack(1)', there is currently no way for the
> server to intercept and modify the status report before it is sent back
> to the client. This is useful for servers with custom logic that need
> to transform or gate the report based on the outcome of external logic
> post reference updates.
>
> Introduce a new 'report' hook which receives the pkt-line encoded
> status report on stdin and whose stdout replaces the report sent to the
> client. A non-zero exit status causes `receive-pack` to die and the
> client to treat the push as failed.
>
> 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.
>
> 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.
Personally I think understanding concrete things is easier than
understanding general things. And discussing the concrete case in the
commit message would help with that as well as provide the context for
git-log(1) rather than just the people who have read these emails.
> ---
> Documentation/githooks.adoc | 23 ++++++
> builtin/receive-pack.c | 41 +++++++++++
> t/meson.build | 1 +
> t/t5412-report-hook.sh | 176 ++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 241 insertions(+)
Should the git-receive-pack(1) doc be updated to mention that this hook
exists? I don’t understand the setup here. The existing
git-receive-pack(1) doc has sections for these hooks:
• `update`
• `pre-receive`
• `post-receive`
• `post-update`
But not these:
• `push-to-checkout`
• `proc-receive`
(referenced against githooks(5))
>
> 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
Another naive question (I have never used any of this). Should this link
to some gitprotocol-X(5) after `pkt-line` in order to have a link that
explains what it is? I don’t see any mention of `pkt-line` on
git-receive-pack(1) or a mention of a gitprotocol-X(5).
> +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.
To my naive eyes this description looks good and without any obvious
errors (typos ;) ).
> +
> push-to-checkout
> ~~~~~~~~~~~~~~~~
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>[snip]
> @@ -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");
Okay, it seems typical for this command to use regular strings (not
translated) for errors. Which makes sense given the application. There
does seem to be translated error strings but one example is “refusing to
update current branch”, which seems to be more of a non-bare, end-user
error than a server error.
> +
> if (use_sideband)
> send_sideband(1, 1, buf.buf, buf.len, use_sideband);
> else
>[snip]
> diff --git a/t/t5412-report-hook.sh b/t/t5412-report-hook.sh
>[snip]
> +test_expect_success "no report hook, push succeeds" '
> + test_when_finished "rm -rf upstream" &&
> + test_when_finished "git -C workbench remote remove origin" &&
This teardown routine is common to all the tests. Is it better style
here to write it out compared to using a helper function (test code is
different from “normal” code)?
> + git init --bare upstream &&
>[snip]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] hook: introduce the report hook for git-receive-pack(1)
2026-08-19 7:03 ` Kristoffer Haugsbakk
@ 2026-08-19 12:11 ` Karthik Nayak
0 siblings, 0 replies; 5+ messages in thread
From: Karthik Nayak @ 2026-08-19 12:11 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git
[-- Attachment #1: Type: text/plain, Size: 7119 bytes --]
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
> On Tue, Aug 18, 2026, at 09:55, Karthik Nayak wrote:
>> When running 'git-receive-pack(1)', there is currently no way for the
>> server to intercept and modify the status report before it is sent back
>> to the client. This is useful for servers with custom logic that need
>> to transform or gate the report based on the outcome of external logic
>> post reference updates.
>>
>> Introduce a new 'report' hook which receives the pkt-line encoded
>> status report on stdin and whose stdout replaces the report sent to the
>> client. A non-zero exit status causes `receive-pack` to die and the
>> client to treat the push as failed.
>>
>> 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.
>>
>> 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.
>
> Personally I think understanding concrete things is easier than
> understanding general things. And discussing the concrete case in the
> commit message would help with that as well as provide the context for
> git-log(1) rather than just the people who have read these emails.
>
I was conflicted about it, since while it does provide some context, it
doesn't apply to most usecases. I will add a little more context in the
commit message.
>> ---
>> Documentation/githooks.adoc | 23 ++++++
>> builtin/receive-pack.c | 41 +++++++++++
>> t/meson.build | 1 +
>> t/t5412-report-hook.sh | 176 ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 241 insertions(+)
>
> Should the git-receive-pack(1) doc be updated to mention that this hook
> exists? I don’t understand the setup here. The existing
> git-receive-pack(1) doc has sections for these hooks:
>
> • `update`
> • `pre-receive`
> • `post-receive`
> • `post-update`
>
> But not these:
>
> • `push-to-checkout`
> • `proc-receive`
>
> (referenced against githooks(5))
>
I didn't know about this. I wonder why we have two sources of truth for
the same. As you see, it's already starting to diverge.
I will add both of them with links to githooks(5), but perhaps a cleanup
there is in order. I would say making githooks(5) the canonical location
with git-receive-pack(1) referencing it makes sense.
>>
>> 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
>
> Another naive question (I have never used any of this). Should this link
> to some gitprotocol-X(5) after `pkt-line` in order to have a link that
> explains what it is? I don’t see any mention of `pkt-line` on
> git-receive-pack(1) or a mention of a gitprotocol-X(5).
>
We could link to 'Documentation/gitprotocol-common.adoc', but I'm not
sure if it is erring on the side of being too verbose. I'll leave it out
since its already existing and assumed to be common knowledge for users
of such hooks. But happy to add it in if others disagree :)
>> +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.
>
> To my naive eyes this description looks good and without any obvious
> errors (typos ;) ).
>
Thanks for reading through
>> +
>> push-to-checkout
>> ~~~~~~~~~~~~~~~~
>>
>> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
>>[snip]
>> @@ -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");
>
> Okay, it seems typical for this command to use regular strings (not
> translated) for errors. Which makes sense given the application. There
> does seem to be translated error strings but one example is “refusing to
> update current branch”, which seems to be more of a non-bare, end-user
> error than a server error.
>
Yeah, since these are generally less user-facing (I say less because
this can be propagated to the user, if the hook exists with a non-zero
error code) I choose not to translate it. As you mentioned, this seems
to be the way for such error messages.
>> +
>> if (use_sideband)
>> send_sideband(1, 1, buf.buf, buf.len, use_sideband);
>> else
>>[snip]
>> diff --git a/t/t5412-report-hook.sh b/t/t5412-report-hook.sh
>>[snip]
>> +test_expect_success "no report hook, push succeeds" '
>> + test_when_finished "rm -rf upstream" &&
>> + test_when_finished "git -C workbench remote remove origin" &&
>
> This teardown routine is common to all the tests. Is it better style
> here to write it out compared to using a helper function (test code is
> different from “normal” code)?
>
Since tests are self-contained, I usually keep the teardowns within
them if they're simple enough.
>> + git init --bare upstream &&
>>[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hook: introduce the report hook for git-receive-pack(1)
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 7:39 ` Patrick Steinhardt
2 siblings, 0 replies; 5+ messages in thread
From: Patrick Steinhardt @ 2026-08-19 7:39 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git
On Tue, Aug 18, 2026 at 09:55:55AM +0200, Karthik Nayak wrote:
> When running 'git-receive-pack(1)', there is currently no way for the
> server to intercept and modify the status report before it is sent back
> to the client. This is useful for servers with custom logic that need
> to transform or gate the report based on the outcome of external logic
> post reference updates.
>
> Introduce a new 'report' hook which receives the pkt-line encoded
> status report on stdin and whose stdout replaces the report sent to the
> client. A non-zero exit status causes `receive-pack` to die and the
> client to treat the push as failed.
I think it would have been useful to add context why none of the
preexisting hooks work for us:
- The pre-receive hook runs too early, as we haven't updated
references at that point yet and we need to have the full view of
all resulting updates (both objects and references).
- The update hook is too inefficient as it runs once per reference,
and we cannot trivially determine the last update.
- The reference-transaction hook cannot be used by us because we care
about the phase where it was committed already. And while the hook
fires in that phase, it does not allow the caller to modify the
result in any capacity.
- The post-receive and post-update hooks cannot be used as they run
too late, at the point where we have already reported success to the
client.
> 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
I'd drop "remote" here -- from the point of view of git-receive-pack(1)
it really is the local repository.
> +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.
This assumes a bit too much about the implementation of the client, as
it may not even be git-push(1) in the first place. We could still
mention this, but we should say that this depends on the client.
> 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.
We should probably document that we expect the hook to never return
non-zero, even if it rejects reference updates, and that doing so
indicates a bug. This is mostly because git-receive-pack(1) shouldn't
ever just die on the client without giving it a proper status.
> +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.
Good thing to call out.
> 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;
Nit: I think it's more commont to call this `ret` rather than `code`.
> 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="\.\."
I was about to say that this looks unused, but it's used by
"common-functions.sh".
[snip]
> +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
Should we maybe not exit abnormally here to see that the push succeeds?
> + EOF
> +
> + test_must_fail git -C workbench push origin $B:refs/heads/main >out 2>&1 &&
> + test_grep "hook-stderr-message" out
> +'
This should have the "remote: " prefix, right? If so, should we verify
that?
Patrick
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-19 12:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 7:39 ` Patrick Steinhardt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox