From: Patrick Steinhardt <ps@pks.im>
Date: Mon, 31 Aug 2026 06:45:04 GMT
Subject: Re: [PATCH v4 3/3] hook: introduce the receive-report hook
Message-ID: <apUi8I-b69XxDAYY@pks.im>
In-Reply-To: <20260826-758-introduce-hook-v4-3-6b14975ad957@gmail.com>

On Wed, Aug 26, 2026 at 12:19:39PM +0200, Karthik Nayak wrote:
> diff --git a/Documentation/git-receive-pack.adoc b/Documentation/git-receive-pack.adoc
> index 4349487e6a..f2d52b7df2 100644
> --- a/Documentation/git-receive-pack.adoc
> +++ b/Documentation/git-receive-pack.adoc
> @@ -243,6 +243,15 @@ 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.
>  
> +RECEIVE-REPORT HOOK
> +-------------------
> +This hook is invoked by 'git-receive-pack' after all the ref updates
> +have been applied but before the report is sent to the client. The hook
> +receives the complete report in pkt-line format on stdin and its stdout
> +replaces the report sent to the client. Allowing the hook to rewrite

s/\. Allowing/, which allows/

> diff --git a/Documentation/githooks.adoc b/Documentation/githooks.adoc
> index ed045940d1..e83ebde667 100644
> --- a/Documentation/githooks.adoc
> +++ b/Documentation/githooks.adoc
> @@ -527,6 +527,49 @@ 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.
>  
> +receive-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 all
> +accepted ref changes are applied to the 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, see linkgit:gitprotocol-pack[5] for details on the
> +structure. 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:

Aren't there three? The hook can also update the "unpack" status to
indicate failure.

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

s/ <reason>/[ <reason>]/

> +  The client will then mark those specific refs as rejected while
> +  treating any `ok` refs as successful. The push as a whole is
> +  considered failed if any ref is `ng`, and `git push` will exit with
> +  a non-zero status on the client side.
> +
> +* To abort the entire push unconditionally, exit with a non-zero
> +  status. In this case the hook's stdout is discarded, `receive-pack`
> +  modifies all references to be rejected with a 'receive-report hook

Yup, I think this is a lot more sensible.

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 70a686c142..1358285589 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -2534,9 +2569,12 @@ static void update_shallow_info(struct command *commands,
>   * Generate the response to be sent to the client invoking 'git-receive-pack(1)'.
>   * For v2 protocol, set `add_reports` to true, which will also add additional
>   * report per reference update.
> + * If `ref_error` is set, then all references will be rejected with the given
> + * error message.
>   */
>  static void generate_response(struct strbuf *buf, struct command *commands,
> -			      const char *unpack_status, bool add_reports)
> +			      const char *unpack_status, bool add_reports,
> +			      const char *ref_error)
>  {
>  	struct command *cmd;
>  
> @@ -2550,10 +2588,13 @@ static void generate_response(struct strbuf *buf, struct command *commands,
>  		if (cmd->error_string)
>  			packet_buf_write(buf, "ng %s %s\n",
>  					 cmd->ref_name, cmd->error_string);
> +		else if (ref_error)
> +			packet_buf_write(buf, "ng %s %s\n",
> +					 cmd->ref_name, ref_error);

Precedence is a bit weird here, as I would have expected the explicit
error to override the implicit per-command ones. It also raises the
question whether it's correct to retain any populated error strings in
favor of updating everything to "receive-report hook failed".

This makes me wonder whetther it would be preferable to update the
`cmd->error_string`s instead of adding this new parameter?

Thanks!

Patrick