Re: [PATCH] hook: introduce the report hook for git-receive-pack(1)

"Kristoffer Haugsbakk" [off-list ref] writes:

On Tue, Aug 18, 2026, at 09:55, Karthik Nayak wrote:

quoted

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 <redacted>
---
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.

quoted

---
 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.

quoted

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 :)

quoted

+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

quoted

+
 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.

quoted

+
 	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.

quoted

+	git init --bare upstream &&
[snip]