Re: [PATCH v5 3/3] hook: introduce the receive-report hook

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> We cannot use any of the existing hooks as:
>
>   - The pre-receive hook runs too early, as we haven't updated
>     references at that point yet and we need to have the full view of
>     all resulting updates (both objects and references).
>
>   - The update hook is too inefficient as it runs once per reference,
>     and we cannot trivially determine the last update.
>
>   - The reference-transaction hook cannot be used by us because we care
>     about the phase where it was committed already. And while the hook
>     fires in that phase, it does not allow the caller to modify the
>     result in any capacity.

Here you explain that the reason this is not suited for your use
case is because it does not allow the caller to modify the result.
The transaction hook is notified in what phase of the reference
updates we are in, and what updates are planned or have happened.
But the hook cannot interfere to change the outcome (except it can
make the transaction abort as a whole in preparation phases).

>   - The post-receive and post-update hooks cannot be used as they run
>     too late, at the point where we have already reported success to the
>     client.
>
> Introduce a new 'receive-report' hook. The hook receives the complete
> pkt-line encoded status report on standard input, after all ref updates
> have been applied to the repository by execute_commands() but before the
> report is sent to the client. See linkgit:gitprotocol-pack[5] details on
> the protocol structure.
>
> The hook's stdout fully replaces the report sent to the client.
> receive-pack fully buffers the hook's stdout before acting on the exit
> status, so the exit code is known before the client receives anything.
> This gives two distinct behaviors depending on exit status:
>
> - 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 modifies
>   all references to be rejected with a 'receive-report hook failed'
>   error.

And the new hook lets you pretend to the other side of the
connection that ref updates that happened on our side is totally
different from what actually happened, but ...

> In both cases, any output the hook writes to standard error is
> forwarded to the client over the sideband channel and appears as
> 'remote:' lines on the client terminal. Writing to stderr alone does
> not affect the push outcome.
>
> Note that in either failure mode, ref updates already applied by
> execute_commands() are not rolled back. The hook can cause the client
> to perceive the push as failed, but cannot undo server-side changes.

... it still cannot interfere to change the outcome.  What has been
committed as reference updates have happened and there is no way to
change it.  So the reason to reject reference-transaction hook seems
a bit weak.  The explanation I heard so far makes it sound as if it
is an equally viable, if not even more viable, alternative to teach
the reference-transaction hook at the commit phase to optionally
allow rejecting the transaction, instead of adding an entirely
different hook (note: I am not suggesting it as an alternative; I am
just saying that the explanation is weak to support this design).

In any case, if the actual ref updates and the reported ref updates
result can be made different, somebody then needs to step in and
reconcile the inconsistencies, no?

The way pusher perceives the state of their remote repository they
just pushed to, which they learn from the output of receive-report
hook, would have no link to reality when this hook is used on the
remote side.  This may matter because the "git push" updates its own
remote-tracking branches to match what the remote says (i.e.,
pretends as if "git push" was immediately followed by "git fetch" to
the same remote).