All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: git@vger.kernel.org, gitster@pobox.com,
	kristofferhaugsbakk@fastmail.com,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH v2] hook: introduce the report hook for git-receive-pack(1)
Date: Fri, 21 Aug 2026 15:49:30 +0200	[thread overview]
Message-ID: <aohXatWhxCAUQTcq@pks.im> (raw)
In-Reply-To: <20260821-758-introduce-hook-v2-1-e90e2f7ac2cf@gmail.com>

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.

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

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

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

Thanks!

Patrick

  reply	other threads:[~2026-08-21 13:49 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 14:47     ` Kristoffer Haugsbakk
2026-08-19  7:39 ` Patrick Steinhardt
2026-08-19 13:13   ` Karthik Nayak
2026-08-19 13:20     ` Patrick Steinhardt
2026-08-19 13:24       ` Karthik Nayak
2026-08-20  9:50 ` Phillip Wood
2026-08-20 15:43   ` Junio C Hamano
2026-08-21 12:51   ` Karthik Nayak
2026-08-21 13:34 ` [PATCH v2] " Karthik Nayak
2026-08-21 13:49   ` Patrick Steinhardt [this message]
2026-08-21 16:08     ` Karthik Nayak
2026-08-21 16:55     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aohXatWhxCAUQTcq@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=kristofferhaugsbakk@fastmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.