Re: [PATCH v2] hook: introduce the report hook for git-receive-pack(1)
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Aug 21, 2026, 16:08 UTC
- Message-ID
- <CAOLa=ZTkW14coLA4st-m6B6P-9pUr+Yzh7Ph6nb0ohXJSbTk4A@mail.gmail.com>
- In-Reply-To
- <aohXatWhxCAUQTcq@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 19 quoted lines
> On Fri, Aug 21, 2026 at 03:34:58PM +0200, Karthik Nayak wrote: > [snip] >> - Exit 0: the hook's stdout is used as the report. The hook can >> rewrite 'ok' lines to 'ng' lines to signal per-ref rejection to the >> client while receive-pack itself exits cleanly. The client marks >> rejected refs as '[remote rejected]' and exits with a non-zero >> status if any ref is 'ng'. >> >> - Non-zero exit: the hook's stdout is discarded, receive-pack calls >> die(), and no report is sent to the client at all. The client >> observes a sideband disconnect and reports 'the remote end hung up >> unexpectedly', treating the entire push as failed. > > I was thinking about this case a bit more. Should we maybe handle it > similarly to the pre-receive hook instead of dieing? If that hook fails > we basically update all references to "pre-receive hook declined", > whereas we could update all of them to "report hook failed". That might > make for a better user experience. >
I didn't know that. Just had a quick look, It would be a bit awkward, since we send the pkt buf to the report, and if we go this way, we'd have to restructure the output again.
Show 17 quoted lines
>> diff --git a/Documentation/git-receive-pack.adoc b/Documentation/git-receive-pack.adoc >> index 0956086d61..e6cc0acaaf 100644 >> --- a/Documentation/git-receive-pack.adoc >> +++ b/Documentation/git-receive-pack.adoc >> @@ -236,6 +236,21 @@ if the repository is packed and is served via a dumb transport. >> exec git update-server-info >> ---- >> >> +PROC-RECEIVE HOOK >> +----------------- >> +This hook is invoked by 'git-receive-pack' when it processes push >> +requests. It handles refs whose names match the patterns defined by >> +`receive.procReceiveRefs` and executes the actual ref updates. See >> +linkgit:githooks[5] for the full protocol description. > > This feels like it should've been a separate commit. >
Will split it out.
Show 20 quoted lines
>> diff --git a/Documentation/githooks.adoc b/Documentation/githooks.adoc >> index ed045940d1..06c9e4b017 100644 >> --- a/Documentation/githooks.adoc >> +++ b/Documentation/githooks.adoc >> @@ -527,6 +527,57 @@ 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 references in its repository. It executes on >> +the repository once after all refs have been updated and after >> +`execute_commands()` has applied all accepted ref changes to the > > Nit: I think we shouldn't talk about functions in our documentation, but > rather about behaviour. Functions are likely to change, and I don't > think we should expect our users to read our code. >
That's a good point, will change
Show 26 quoted lines
>> +repository, but before the pkt-line encoded status report is sent back >> +to the client. >> + >> +The hook receives the complete pkt-line encoded status report on >> +standard input. The report begins with an `unpack` line indicating >> +whether the object transfer succeeded (`unpack ok` or >> +`unpack <error>`), followed by one `ok <refname>` or >> +`ng <refname> <reason>` line per ref that was pushed, and is >> +terminated by a flush packet. >> + >> +The hook's standard output entirely replaces the report that is sent >> +to the client. The hook must write a valid pkt-line encoded report in >> +the same format it received. The hook's stdout is fully buffered by >> +`receive-pack` before any data is sent to the client, so the hook's >> +exit status is known before the client receives anything. >> + >> +There are two distinct ways the hook can affect the push outcome: >> + >> +* To reject individual ref updates while keeping `receive-pack` alive, >> + rewrite the corresponding `ok <refname>` lines to >> + `ng <refname> <reason>` lines in the output and exit with status 0. > > It's `ng <refname>[ <reason>]`, right? I think the reason itself is > optional. We might also want to clarify whether there should be a > trailing newline or not. >
You're right, since 'send-pack' will default to 'failed' if there is no reason.
We do say 'terminated by a flush packed'.
> Thanks! > > Patrick
Thanks for the review :)