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

Patrick Steinhardt <[email protected]> writes:

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

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

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

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

-----BEGIN PGP SIGNATURE-----

iQHKBAEBCgA0FiEEV85Mf2N1cQ/LZcYGPtWfJI5GjH8FAmqId+sWHGthcnRoaWsu
MTg4QGdtYWlsLmNvbQAKCRA+1Z8kjkaMfxFsC/sGURw0KoVd68WNyo4e5WI0t63X
R55uXNSR/duHsGZuA8pekLQMXQ2kJmiNH7JhM5/G0tGC4zyo6FyXIpQ5wJAdcX2e
cFQdr9r/7rCgCq5YFrYjVfWPaybQxl0BUGednnuxBycXA+zOe961w2nMmJJ9o2pk
DuOmrqfCOQZg44nmfvZgZay3q58IJFOIWhIcUVD0rqJdntD4ZD+rS+NMQG17tteK
3jR65LYxALFtMoMSwoAsCuTq7AqEWKlP+f8erw9qETPhqu6r4p6NCzrA26a6jJus
fK3etI2sWXNmslzG1WK9M24/TxuCS+hVlNRHl6wSmk9tS0BwRBOsuklNDFhT3G3c
3rW1I8/geSmQlIiWbJ44gFVvQY6rJQPOmWUzGD5nsuUlPtm/EOAU0+8JXi/zLctn
CKjjH9i56Ml6RFvoKaddV+ZRZWRmPhbuluLoOvDAo0o4xiC2hYGQucNH4rigU4aN
X0dYsRPYiT1dEMSetZQLBuA7/b7LD05KhEzVeuk=
=Zxid
-----END PGP SIGNATURE-----