[v10,2/8] receive-pack: add new proc-receive hook

Commit Message

Jiang Xin April 12, 2020, 1:30 p.m. UTC

From: Jiang Xin <[e-mail]>

Git calls an internal `execute_commands` function to handle commands
sent from client to `git-receive-pack`.  Regardless of what references
the user pushes, git creates or updates the corresponding references if
the user has write-permission.  A contributor who has no
write-permission, cannot push to the repository directly.  So, the
contributor has to write commits to an alternate location, and sends
pull request by emails or by other ways.  We call this workflow as a
distributed workflow.

It would be more convenient to work in a centralized workflow like what
Gerrit provided for some cases.  For example, a read-only user who
cannot push to a branch directly can run the following `git push`
command to push commits to a pseudo reference (has a prefix "refs/for/",
not "refs/heads/") to create a code review.

    git push origin \
        HEAD:refs/for/<branch-name>/<session>

The `<branch-name>` in the above example can be as simple as "master",
or a more complicated branch name like "foo/bar".  The `<session>` in
the above example command can be the local branch name of the client
side, such as "my/topic".

We cannot implement a centralized workflow elegantly by using
"pre-receive" + "post-receive", because Git will call the internal
function "execute_commands" to create references (even the special
pseudo reference) between these two hooks.  Even though we can delete
the temporarily created pseudo reference via the "post-receive" hook,
having a temporary reference is not safe for concurrent pushes.

So, add a filter and a new handler to support this kind of workflow.
The filter will check the prefix of the reference name, and if the
command has a special reference name, the filter will turn a specific
field (`run_proc_receive`) on for the command.  Commands with this filed
turned on will be executed by a new handler (an hook named
"proc-receive") instead of the internal `execute_commands` function.
We can use this "proc-receive" command to create pull requests or send
emails for code review.

Suggested by Junio, this "proc-receive" hook reads the commands,
push-options (optional), and send result using a protocol in pkt-line
format.  In the following example, The letter "S" stands for
"receive-pack" and letter "H" stands for the hook.

    # Version and features negotiation.
    S: PKT-LINE(version=1\0push-options atomic...)
    S: flush-pkt
    H: PKT-LINE(version=1\0push-options...)
    H: flush-pkt

    # Send commands from server to the hook.
    S: PKT-LINE(<old-oid> <new-oid> <ref>)
    S: ... ...
    S: flush-pkt
    # Send push-options only if the 'push-options' feature is enabled.
    S: PKT-LINE(push-option)
    S: ... ...
    S: flush-pkt

    # Receive result from the hook.
    # OK, run this command successfully.
    H: PKT-LINE(<old-oid> <new-oid> <ref> ok)
    # NO, I reject it.
    H: PKT-LINE(<old-oid> <new-oid> <ref> ng <reason>)
    # Fall through, let 'receive-pack' to execute it.
    H: PKT-LINE(<old-oid> <new-oid> <ref> ft)
    # OK, but has an alternate reference.  The alternate reference name
    # and other status are given in key=value pairs after the null
    # character.
    H: PKT-LINE(<old-oid> <new-oid> <ref> ok\0ref=refs/pull/123/head
                forced-update)
    H: ... ...
    H: flush-pkt

After receiving a command, the hook will execute the command, and may
create/update different reference.  For example, a command for a pseudo
reference "refs/for/master/topic" may create/update different reference
such as "refs/pull/123/head".  The alternate reference name and other
status are given in key-value pairs as extended status of the report
line.

The list of commands returned from "proc-receive" will replace the
relevant commands that are sent from user to "receive-pack", and
"receive-pack" will continue to run the "execute_commands" function and
other routines.  Finally, the result of the execution of these commands
will be reported to end user.

Suggested-by: Junio C Hamano <[e-mail]>
Signed-off-by: Jiang Xin <[e-mail]>
---
 Makefile                     |   1 +
 builtin/receive-pack.c       | 260 +++++++++++++++-
 t/helper/test-proc-receive.c | 168 ++++++++++
 t/helper/test-tool.c         |   1 +
 t/helper/test-tool.h         |   1 +
 t/t5411/common-functions.sh  |  10 +-
 t/t5411/common-test-cases.sh | 576 +++++++++++++++++++++++++++++++++++
 7 files changed, 1011 insertions(+), 6 deletions(-)
 create mode 100644 t/helper/test-proc-receive.c

Comments

Junio C Hamano April 12, 2020, 9:30 p.m. UTC | #1

Jiang Xin <[e-mail]> writes:

> The filter will check the prefix of the reference name, and if the
> command has a special reference name, the filter will turn a specific
> field (`run_proc_receive`) on for the command.  Commands with this filed
> turned on will be executed by a new handler (an hook named
> "proc-receive") instead of the internal `execute_commands` function.
> We can use this "proc-receive" command to create pull requests or send
> emails for code review.
>
> Suggested by Junio, this "proc-receive" hook reads the commands,
> push-options (optional), and send result using a protocol in pkt-line
> format.  In the following example, The letter "S" stands for
> "receive-pack" and letter "H" stands for the hook.

Heh, you're giving me too much credit here.  The exchange
illustrated below makes sense to me, ...

>     # Version and features negotiation.
>     S: PKT-LINE(version=1\0push-options atomic...)
>     S: flush-pkt
>     H: PKT-LINE(version=1\0push-options...)
>     H: flush-pkt
>
>     # Send commands from server to the hook.
>     S: PKT-LINE(<old-oid> <new-oid> <ref>)
>     S: ... ...
>     S: flush-pkt
>     # Send push-options only if the 'push-options' feature is enabled.
>     S: PKT-LINE(push-option)
>     S: ... ...
>     S: flush-pkt
>
>     # Receive result from the hook.
>     # OK, run this command successfully.
>     H: PKT-LINE(<old-oid> <new-oid> <ref> ok)
>     # NO, I reject it.
>     H: PKT-LINE(<old-oid> <new-oid> <ref> ng <reason>)
>     # Fall through, let 'receive-pack' to execute it.
>     H: PKT-LINE(<old-oid> <new-oid> <ref> ft)
>     # OK, but has an alternate reference.  The alternate reference name
>     # and other status are given in key=value pairs after the null
>     # character.
>     H: PKT-LINE(<old-oid> <new-oid> <ref> ok\0ref=refs/pull/123/head
>                 forced-update)

The semantics of this one is fuzzy.  We made an update to a ref that
is different from the one that was requested (presumably that is what
is reported after "\0ref="), OK, but did we update the ref in the
request, too, or did we leave the ref in the request intact?  Or, do
we say "no\0ref=..." if we don't update the requested ref and
instead update a different one?  Let's hold the answer at this point
and keep thinking...

>     H: ... ...
>     H: flush-pkt
>
> After receiving a command, the hook will execute the command, and may
> create/update different reference.  For example, a command for a pseudo
> reference "refs/for/master/topic" may create/update different reference
> such as "refs/pull/123/head".  The alternate reference name and other
> status are given in key-value pairs as extended status of the report
> line.
>
> The list of commands returned from "proc-receive" will replace the
> relevant commands that are sent from user to "receive-pack", and
> "receive-pack" will continue to run the "execute_commands" function and
> other routines.  Finally, the result of the execution of these commands
> will be reported to end user.

It is not clear if there needs to be any correspondence between the
sequence of input commands and the sequence of output commands.  I
am guessing that there is not any and the hook is allowed to do
anything as it wants to.  For example:

 - it is OK to execute them all and report them, but in a totally
   random order.

 - it is OK to ignore all input and report no update (not even
   "ng").

 - it is OK to be asked to update one ref, but update and report
   updates to two refs that are not related to the requested ref.

Is my understanding correct?

Or does the design tie the set of input and output ref updates
closely together, i.e. one input corresponds to one output and they
are in order, so all the hook is allowed to do is

 - to execute and update as it is told, and report "ok",

 - to reject and report "ng", or

 - to update *another* ref without updating the requested one, with
   "ok\0ref=..." mechanism.

I am not sure which one is more sensible, though.

If we choose to use the "anything goes" approach, I do not think
there is no need for the "ok\nref=..." mechanism---we can just give
two output records, one "ok" for the original request, and a new
"ok" that reports the "we updated this one instead".

> +#define RUN_PROC_RECEIVE_SCHEDULED	1
> +#define RUN_PROC_RECEIVE_RETURNED	2
>  struct command {
>  	struct command *next;
>  	const char *error_string;
>  	unsigned int skip_update:1,
> -		     did_not_exist:1;
> +		     did_not_exist:1,
> +		     run_proc_receive:2;
>  	int index;
>  	struct object_id old_oid;
>  	struct object_id new_oid;
> @@ -817,6 +820,236 @@ static int run_update_hook(struct command *cmd)
>  	return finish_command(&proc);
>  }
>  
> +static struct command *find_command_by_refname(const struct command *list,
> +					       const char *refname)
> +{
> +	for (; list; list = list->next)
> +		if (!strcmp(list->ref_name, refname))
> +			return (struct command *)list;
> +	return NULL;
> +}
> +
> +static int read_proc_receive_report(struct packet_reader *reader,
> +				    struct command *commands,
> +				    struct strbuf *errmsg)
> +{
> +	struct command *cmd;
> +	struct command *hint = NULL;
> +	int code = 0;
> +
> +	for (;;) {
> +		struct object_id old_oid, new_oid;
> +		const char *refname;
> +		const char *p;
> +		char *status;
> +		char *msg = NULL;
> +
> +		if (packet_reader_read(reader) != PACKET_READ_NORMAL)
> +			break;
> +		if (parse_oid_hex(reader->line, &old_oid, &p) ||
> +		    *p++ != ' ' ||
> +		    parse_oid_hex(p, &new_oid, &p) ||
> +		    *p++ != ' ') {
> +			strbuf_addf(errmsg, "proc-receive expected 'old new ref status [msg]', got '%s'\n",
> +				    reader->line);
> +			code = -1;
> +			continue;
> +		}
> +
> +		refname = p;
> +		status = strchr(p, ' ');
> +		if (!status) {
> +			strbuf_addf(errmsg, "proc-receive expected 'old new ref status [msg]', got '%s'\n",
> +				    reader->line);
> +			code = -1;
> +			continue;
> +		}
> +		*status++ = '\0';
> +		if (strlen(status) > 2 && *(status + 2) == ' ') {
> +			msg = status + 2;
> +			*msg++ = '\0';
> +		}

So, here we parsed "<old> <new> <ref> (ok|ng)", with status pointing
at ok/ng, or "<old> <new> <ref> (ok|ng) <why>", with status pointing
at ok/ng and msg pointing at <why>.  refname points at <ref>.

Most notably, "ok\0ref=..." is not handled any specially from a
normal "ok" here.

> +		if (strlen(status) != 2) {
> +			strbuf_addf(errmsg, "proc-receive has bad status '%s' for '%s'\n",
> +				    status, reader->line);
> +			code = -1;
> +			continue;
> +		}
> +
> +		/* first try searching at our hint, falling back to all refs */
> +		if (hint)
> +			hint = find_command_by_refname(hint, refname);
> +		if (!hint)
> +			hint = find_command_by_refname(commands, refname);
> +		if (!hint) {
> +			strbuf_addf(errmsg, "proc-receive reported status on unknown ref: %s\n",
> +				    refname);
> +			continue;
> +		}
> +		if (!hint->run_proc_receive) {
> +			strbuf_addf(errmsg, "proc-receive reported status on ref of builtin command: %s\n",
> +				    refname);
> +			continue;

Updates against a request to update certain ref hierarchies by the
hook is rejected here.

> +		}
> +		if (!strcmp(status, "ng")) {
> +			if (msg)
> +				hint->error_string = xstrdup(msg);
> +			else
> +				hint->error_string = "failed";
> +			code = -1;

The <why> in "(ok|ng) <why>" message is only used for "ng", which is
probably good, as the reason why something succeeded is not usually
very interesting.

> +		} else if (strcmp("ok", status)) {
> +			strbuf_addf(errmsg, "proc-receive has bad status '%s' for '%s'\n",
> +				    status, reader->line);
> +			code = -1;
> +			/* Skip marking it as RUN_PROC_RECEIVE_RETURNED */

But then shouldn't we be complaining if msg is not NULL here,
instead of silently ignoring?  Also we didn't see what happened to
the promised "ok\0ref=..." stuff here, or the passthru "ft".
Puzzled...

> +			continue;
> +		}
> +		oidcpy(&hint->old_oid, &old_oid);
> +		oidcpy(&hint->new_oid, &new_oid);
> +		hint->run_proc_receive |= RUN_PROC_RECEIVE_RETURNED;

Or, is this last one the catch all for "ft" (in other words, the
hook does not have to say "ft", but as long as it says two non-SP
letters that are not "ok" nor "ng", it is taken as "ft")?

> +	}
> +
> +	for (cmd = commands; cmd; cmd = cmd->next)
> +		if (cmd->run_proc_receive && !cmd->error_string &&
> +		    !(cmd->run_proc_receive & RUN_PROC_RECEIVE_RETURNED)) {
> +		    cmd->error_string = "no report from proc-receive";
> +		    code = -1;
> +		}
> +
> +	return code;
> +}

OK, so this sort-of answers my earlier question.  But not quite...

The output records and the input requests are tied by "<ref>" each
input record wanted to udpate.  The order does not matter, but not
having a corresponding report in the hook's output is like the hook
reporting a "ng" failure.  It also means that the hook can update
two refs in response to one request, but it is awkward.

> +static int run_proc_receive_hook(struct command *commands,
> +				 const struct string_list *push_options)
> +{
> +	struct child_process proc = CHILD_PROCESS_INIT;
> +	struct async muxer;
> +	struct command *cmd;
> +	const char *argv[2];
> +	struct packet_reader reader;
> +	struct strbuf cap = STRBUF_INIT;
> +	struct strbuf errmsg = STRBUF_INIT;
> +	int pr_use_push_options = 0;
> +	int version = 0;
> +	int code;
> +
> +	argv[0] = find_hook("proc-receive");
> +	if (!argv[0]) {
> +		rp_error("cannot find hook 'proc-receive'");
> +		return -1;
> +	}
> +	argv[1] = NULL;
> +
> +	proc.argv = argv;
> +	proc.in = -1;
> +	proc.out = -1;
> +	proc.trace2_hook_name = "proc-receive";
> +

Isn't this a brand new protocol between receive-pack and a hook?  I
am puzzled why we want a choice between using and not using
sideband.  It's not like you are maintaining compatibility with
ancient version of Git that talked with proc-receive hook but
without sideband, because it did not know how to multiplex sideband
communication.  Shouldn't we just always assume that the sideband is
available?

Or are we just letting the hook directly answer the push client and
that is why this thing needs to know if we are using sideband
between us and the client?  I kind of expected that you'd keep the
two communication channels on both sides isolated so that the side
that talks with the hook does not need to know how we are talking
with the client.

> +	if (use_sideband) {
> +		memset(&muxer, 0, sizeof(muxer));
> +		muxer.proc = copy_to_sideband;
> +		muxer.in = -1;
> +		code = start_async(&muxer);
> +		if (code)
> +			return code;
> +		proc.err = muxer.in;
> +	} else {
> +		proc.err = 0;
> +	}
> +
> +	code = start_command(&proc);
> +	if (code) {
> +		if (use_sideband)
> +			finish_async(&muxer);
> +		return code;
> +	}
> +
> +	sigchain_push(SIGPIPE, SIG_IGN);
> +
> +	/* Version negotiaton */
> +	packet_reader_init(&reader, proc.out, NULL, 0,
> +			   PACKET_READ_CHOMP_NEWLINE |
> +			   PACKET_READ_DIE_ON_ERR_PACKET);
> ...
> @@ -1497,6 +1731,20 @@ static void execute_commands(struct command *commands,
>  
>  	reject_updates_to_hidden(commands);
>  
> +	/* Try to find commands that have special prefix in their reference names,

Style?

> +	 * and mark them to run an external "proc-receive" hook later.
> +	 */

Jiang Xin April 13, 2020, 10:58 a.m. UTC | #2

Junio C Hamano <[e-mail]> 于2020年4月13日周一 上午5:30写道：
>
> Jiang Xin <[e-mail]> writes:
> >     # Receive result from the hook.
> >     # OK, run this command successfully.
> >     H: PKT-LINE(<old-oid> <new-oid> <ref> ok)
> >     # NO, I reject it.
> >     H: PKT-LINE(<old-oid> <new-oid> <ref> ng <reason>)
> >     # Fall through, let 'receive-pack' to execute it.
> >     H: PKT-LINE(<old-oid> <new-oid> <ref> ft)
> >     # OK, but has an alternate reference.  The alternate reference name
> >     # and other status are given in key=value pairs after the null
> >     # character.
> >     H: PKT-LINE(<old-oid> <new-oid> <ref> ok\0ref=refs/pull/123/head
> >                 forced-update)
>
> The semantics of this one is fuzzy.  We made an update to a ref that
> is different from the one that was requested (presumably that is what
> is reported after "\0ref="), OK, but did we update the ref in the
> request, too, or did we leave the ref in the request intact?  Or, do
> we say "no\0ref=..." if we don't update the requested ref and
> instead update a different one?  Let's hold the answer at this point
> and keep thinking...

Before making a decision, we must consider some limitations or backward
compatibility issues.  See the limitations from following code snippet
of "send-pack.c" and "transport.c":

* "receive-pack" should not send status other than "ok" and "ng" to "send-pack".
  We must keep this for backward compatibility.

    # File: send-pack.c
    #
    152 static int receive_status(struct packet_reader *reader, struct ref *refs)
    153 {
    159     while (1) {
    160         const char *refname;
    161         char *msg;
    162         if (packet_reader_read(reader) != PACKET_READ_NORMAL)
    163             break;
    164         if (!starts_with(reader->line, "ok ") && !starts_with(reader->line, "ng ")) {
    165             error("invalid ref status from remote: %s", reader->line);
    166             ret = -1;
    167             break;
    168         }

* Do not report unknown reference to "send-pack". Each returned reference
  must be included in `remote_refs`.

    175         /* first try searching at our hint, falling back to all refs */
    176         if (hint)
    177             hint = find_ref_by_name(hint, refname);
    178         if (!hint)
    179             hint = find_ref_by_name(refs, refname);
    180         if (!hint) {
    181             warning("remote reported status on unknown ref: %s",
    182                     refname);

* Do not report unexpected reference to "send-pack".

    185         if (hint->status != REF_STATUS_EXPECTING_REPORT) {
    186             warning("remote reported status on unexpected ref: %s",
    187                     refname);
    188             continue;
    189         }

* Client will complain if a expecting command not is not received from
  the report.

    # File: transport.c
    #
    531 static int print_one_push_status(struct ref *ref, const char *dest, int count,
    532                                  int porcelain, int summary_width)
    540     switch(ref->status) {
    584     case REF_STATUS_EXPECTING_REPORT:
    585         print_ref_status('!', "[remote failure]", ref,
    586                          ref->deletion ? NULL : ref->peer_ref,
    587                          "remote failed to report status",
    588                          porcelain, summary_width);


So we should not invent new status between "receive-pack" and "send-pack".
It is reasonable to to extend the report function from 'receive-pack'
to 'send-pack' (not the report from 'proc-receive' to 'receive-pack')
by adding a null character and multiple key-value pairs as extended
status after the "ok" and "ng" message.

For example, the following git-push command:

    git push origin HEAD:refs/for/master/topic

The refspec points to a pseudo reference "refs/for/master/topic", which
does not exist on the server, so the old-oid of the command to
"receive-pack" must be a zero-oid. The command for 'proc-receive' is:

    <zero-oid> <new-oid> refs/for/master/topic

If "proc-receive" find this `git-push` command is associated with an already
exist pull request (such as "refs/pull/123/head"), it will update this
reference instead of creating a new one.

For this case, we can extend the report line (from "receive-pack" to
"send-pack”) like this:

    ok refs/for/master/topic\0ref=refs/pull/123/head old-oid=<hash> [forced-push]

This extended report line is backward compatible. Old version of git
client will show message like this:

    To upstream.git
     * [new branch]          HEAD -> refs/for/master/topic

New version of git has the knowledge of how to handle the extended-status
after the null character, will print the following message:

    To upstream.git
     + 1234567...7654321     HEAD -> refs/pull/124/head (forced update)


Actually, both 'ok' and 'ng' status have the knowledge of '<msg>', even
though the '<msg>' is ignored by the 'ok' status. After the extension,
both of them can take key-value pairs as extended-status, like:

    ok <reference> [msg]\0key=value ...
    ng <reference> [msg]\0key=value ...

We can use '<msg>' and the extended status to fit our needs in the future.

As for the design of status report from 'proc-receive' to 'receive-pack',
we should follow the same rules:

1. Never report status for non-exists commands.

2. Do not forget to report some of the commands.

3. After receive report for commands from 'proc-receive', we should have
   enough information to generate report to 'send-pack' from these commands.

For the above example, when user push to 'refs/for/master/topic',
'proc-receive' may report to 'receive-pack' like this:

    PKT-LINE(<old-oid> <new-oid> <ref> ok\0ref=refs/pull/123/head forced-update)

'send-pack' will save the key-value pairs into the cooresponding `ref->remote_status`.
If the report line has a different '<old-oid>' or '<new-oid>' for the '<ref>',
'send-pack' will substitude the oids of the ref, and append new key-value
pairs (old-oid=<hash> and new-oid=<hash>) to `ref->remote_status`.

> >     H: ... ...
> >     H: flush-pkt
> >
> > After receiving a command, the hook will execute the command, and may
> > create/update different reference.  For example, a command for a pseudo
> > reference "refs/for/master/topic" may create/update different reference
> > such as "refs/pull/123/head".  The alternate reference name and other
> > status are given in key-value pairs as extended status of the report
> > line.
> >
> > The list of commands returned from "proc-receive" will replace the
> > relevant commands that are sent from user to "receive-pack", and
> > "receive-pack" will continue to run the "execute_commands" function and
> > other routines.  Finally, the result of the execution of these commands
> > will be reported to end user.
>
> It is not clear if there needs to be any correspondence between the
> sequence of input commands and the sequence of output commands.  I
> am guessing that there is not any and the hook is allowed to do
> anything as it wants to.  For example:
>
>  - it is OK to execute them all and report them, but in a totally
>    random order.
>
>  - it is OK to ignore all input and report no update (not even
>    "ng").
>
>  - it is OK to be asked to update one ref, but update and report
>    updates to two refs that are not related to the requested ref.
>
> Is my understanding correct?
>
> Or does the design tie the set of input and output ref updates
> closely together, i.e. one input corresponds to one output and they
> are in order, so all the hook is allowed to do is
>
>  - to execute and update as it is told, and report "ok",
>
>  - to reject and report "ng", or
>
>  - to update *another* ref without updating the requested one, with
>    "ok\0ref=..." mechanism.
>
> I am not sure which one is more sensible, though.
>
> If we choose to use the "anything goes" approach, I do not think
> there is no need for the "ok\nref=..." mechanism---we can just give
> two output records, one "ok" for the original request, and a new
> "ok" that reports the "we updated this one instead".

I saw you have noticed the code for replacing the commands of 'receive-pack'
by comparing ref name.

> > +             } else if (strcmp("ok", status)) {
> > +                     strbuf_addf(errmsg, "proc-receive has bad status '%s' for '%s'\n",
> > +                                 status, reader->line);
> > +                     code = -1;
> > +                     /* Skip marking it as RUN_PROC_RECEIVE_RETURNED */
>
> But then shouldn't we be complaining if msg is not NULL here,
> instead of silently ignoring?  Also we didn't see what happened to
> the promised "ok\0ref=..." stuff here, or the passthru "ft".
> Puzzled...

I implement "ft" and "\0key=value..." features in patch 6/8, and it
depends another refactoring commit.  I can squash it to this commit
in next reroll, and rearrange the refactoring commmit and test cases.


> > +                     continue;
> > +             }
> > +             oidcpy(&hint->old_oid, &old_oid);
> > +             oidcpy(&hint->new_oid, &new_oid);
> > +             hint->run_proc_receive |= RUN_PROC_RECEIVE_RETURNED;
>
> Or, is this last one the catch all for "ft" (in other words, the
> hook does not have to say "ft", but as long as it says two non-SP
> letters that are not "ok" nor "ng", it is taken as "ft")?

"ft" is implemented in patch 6/8.  This code is used to help to find
missing reported references.

>
> > +     }
> > +
> > +     for (cmd = commands; cmd; cmd = cmd->next)
> > +             if (cmd->run_proc_receive && !cmd->error_string &&
> > +                 !(cmd->run_proc_receive & RUN_PROC_RECEIVE_RETURNED)) {
> > +                 cmd->error_string = "no report from proc-receive";
> > +                 code = -1;
> > +             }
> > +
> > +     return code;
> > +}
>
> OK, so this sort-of answers my earlier question.  But not quite...
>
> The output records and the input requests are tied by "<ref>" each
> input record wanted to udpate.  The order does not matter, but not
> having a corresponding report in the hook's output is like the hook
> reporting a "ng" failure.  It also means that the hook can update
> two refs in response to one request, but it is awkward.
>

Right sort order of the output commnands has better performace.
The hook can only update commands marked with 'run_proc_receive'.

"update two refs", do you mean duplicate report line from 'proc-receive'? 
If 'proc-receive' really want to update several references for one
command, it must report all of the updates in one report line by adding
message, or adding additional key-value pairs.

> > +static int run_proc_receive_hook(struct command *commands,
> > +                              const struct string_list *push_options)
> > +{
> > +     struct child_process proc = CHILD_PROCESS_INIT;
> > +     struct async muxer;
> > +     struct command *cmd;
> > +     const char *argv[2];
> > +     struct packet_reader reader;
> > +     struct strbuf cap = STRBUF_INIT;
> > +     struct strbuf errmsg = STRBUF_INIT;
> > +     int pr_use_push_options = 0;
> > +     int version = 0;
> > +     int code;
> > +
> > +     argv[0] = find_hook("proc-receive");
> > +     if (!argv[0]) {
> > +             rp_error("cannot find hook 'proc-receive'");
> > +             return -1;
> > +     }
> > +     argv[1] = NULL;
> > +
> > +     proc.argv = argv;
> > +     proc.in = -1;
> > +     proc.out = -1;
> > +     proc.trace2_hook_name = "proc-receive";
> > +
>
> Isn't this a brand new protocol between receive-pack and a hook?  I
> am puzzled why we want a choice between using and not using
> sideband.  It's not like you are maintaining compatibility with
> ancient version of Git that talked with proc-receive hook but
> without sideband, because it did not know how to multiplex sideband
> communication.  Shouldn't we just always assume that the sideband is
> available?

The sideband message stream (stderr of 'proc-receive') will be sent
directly to the client.  If the client is a very old version of git,
is is still safe?

> Or are we just letting the hook directly answer the push client and
> that is why this thing needs to know if we are using sideband
> between us and the client?  I kind of expected that you'd keep the
> two communication channels on both sides isolated so that the side
> that talks with the hook does not need to know how we are talking
> with the client.

In test cases for 'proc-receive', I output some debug info on stderr
of 'proc-receive' hook.  If the client (very old version of git)
does not support sideband, it's ok.

> > +     if (use_sideband) {
> > +             memset(&muxer, 0, sizeof(muxer));
> > +             muxer.proc = copy_to_sideband;
> > +             muxer.in = -1;
> > +             code = start_async(&muxer);
> > +             if (code)
> > +                     return code;
> > +             proc.err = muxer.in;
> > +     } else {
> > +             proc.err = 0;
> > +     }
> > +
> > +     code = start_command(&proc);
> > +     if (code) {
> > +             if (use_sideband)
> > +                     finish_async(&muxer);
> > +             return code;
> > +     }
> > +
> > +     sigchain_push(SIGPIPE, SIG_IGN);
> > +
> > +     /* Version negotiaton */
> > +     packet_reader_init(&reader, proc.out, NULL, 0,
> > +                        PACKET_READ_CHOMP_NEWLINE |
> > +                        PACKET_READ_DIE_ON_ERR_PACKET);
> > ...
> > @@ -1497,6 +1731,20 @@ static void execute_commands(struct command *commands,
> >
> >       reject_updates_to_hidden(commands);
> >
> > +     /* Try to find commands that have special prefix in their reference names,
>
> Style?

Will fix.
>
> > +      * and mark them to run an external "proc-receive" hook later.
> > +      */

Junio C Hamano April 13, 2020, 9:50 p.m. UTC | #3

Jiang Xin <[e-mail]> writes:

> Before making a decision, we must consider some limitations or backward
> compatibility issues.  See the limitations from following code snippet
> of "send-pack.c" and "transport.c":
> ...
> The sideband message stream (stderr of 'proc-receive') will be sent
> directly to the client.  If the client is a very old version of git,
> is is still safe?

These made your assumptions clear to me, I think.

I was expecting that the protocol was purely between receive-pack
and the hook and the communication back to the "push" side was done
by receive-pack, AFTER receive-pack finished talking with the hook,
reading and understanding what the hook did.  And with that
expectation, the protocol to the hook is free of "compatibility"
concern and does not have to be constrained to "one report packet,
which may have to carry multiple updates if the hook updates
multiple refs in response to a single update request packet".

But if you are letting the hook talk directly to the "push" side,
without "receive-pack" even snooping/understanding what is going on,
sure, I can understand why you have to put such a limitation to the
protocol to the hook or use/unuse of the side-band.

I actually think we would need some update (new capability, perhaps)
to the protocol between "push" and "receive-pack" when we want to
fully support things like "you tried to push commit A to
'refs/for/X'; 'refs/for/X' is not created nor updated, but instead
'refs/heads/X' has been updated to commit B, which got created using
info in A".  A partial support can be done by just pretending that
the proposed update for 'refs/for/X' succeeded and nothing else
happened, and that may be a perfectly usable initial version.  But I
would suspect that we eventually would want to be able to tell the
"git push" a bit more, and at that point, we'd probably want new
capabilities between "push" and "receive-pack".

I think we should retain fairly strict control over the vocabulary
used by "receive-pack" to explain what happened on "the server side"
to the client, to avoid fracturing the ecosystem.  With "hook's
output is sent pass-thru back to the client" design, you'd allow a
proprietary proc-receive hook to express what it did in a way that
only a matching proprietary variant of "git push" can understand,
which I do not think is a good thing.

Thanks.

Patch

diff --git a/Makefile b/Makefile
index ef1ff2228f..1c52c280ce 100644
--- a/Makefile
+++ b/Makefile
@@ -725,6 +725,7 @@  TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
 TEST_BUILTINS_OBJS += test-path-utils.o
 TEST_BUILTINS_OBJS += test-pkt-line.o
 TEST_BUILTINS_OBJS += test-prio-queue.o
+TEST_BUILTINS_OBJS += test-proc-receive.o
 TEST_BUILTINS_OBJS += test-progress.o
 TEST_BUILTINS_OBJS += test-reach.o
 TEST_BUILTINS_OBJS += test-read-cache.o
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 2cc18bbffd..1a57c5bb3b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -308,11 +308,14 @@  static void write_head_info(void)
 	packet_flush(1);
 }
 
+#define RUN_PROC_RECEIVE_SCHEDULED	1
+#define RUN_PROC_RECEIVE_RETURNED	2
 struct command {
 	struct command *next;
 	const char *error_string;
 	unsigned int skip_update:1,
-		     did_not_exist:1;
+		     did_not_exist:1,
+		     run_proc_receive:2;
 	int index;
 	struct object_id old_oid;
 	struct object_id new_oid;
@@ -817,6 +820,236 @@  static int run_update_hook(struct command *cmd)
 	return finish_command(&proc);
 }
 
+static struct command *find_command_by_refname(const struct command *list,
+					       const char *refname)
+{
+	for (; list; list = list->next)
+		if (!strcmp(list->ref_name, refname))
+			return (struct command *)list;
+	return NULL;
+}
+
+static int read_proc_receive_report(struct packet_reader *reader,
+				    struct command *commands,
+				    struct strbuf *errmsg)
+{
+	struct command *cmd;
+	struct command *hint = NULL;
+	int code = 0;
+
+	for (;;) {
+		struct object_id old_oid, new_oid;
+		const char *refname;
+		const char *p;
+		char *status;
+		char *msg = NULL;
+
+		if (packet_reader_read(reader) != PACKET_READ_NORMAL)
+			break;
+		if (parse_oid_hex(reader->line, &old_oid, &p) ||
+		    *p++ != ' ' ||
+		    parse_oid_hex(p, &new_oid, &p) ||
+		    *p++ != ' ') {
+			strbuf_addf(errmsg, "proc-receive expected 'old new ref status [msg]', got '%s'\n",
+				    reader->line);
+			code = -1;
+			continue;
+		}
+
+		refname = p;
+		status = strchr(p, ' ');
+		if (!status) {
+			strbuf_addf(errmsg, "proc-receive expected 'old new ref status [msg]', got '%s'\n",
+				    reader->line);
+			code = -1;
+			continue;
+		}
+		*status++ = '\0';
+		if (strlen(status) > 2 && *(status + 2) == ' ') {
+			msg = status + 2;
+			*msg++ = '\0';
+		}
+		if (strlen(status) != 2) {
+			strbuf_addf(errmsg, "proc-receive has bad status '%s' for '%s'\n",
+				    status, reader->line);
+			code = -1;
+			continue;
+		}
+
+		/* first try searching at our hint, falling back to all refs */
+		if (hint)
+			hint = find_command_by_refname(hint, refname);
+		if (!hint)
+			hint = find_command_by_refname(commands, refname);
+		if (!hint) {
+			strbuf_addf(errmsg, "proc-receive reported status on unknown ref: %s\n",
+				    refname);
+			continue;
+		}
+		if (!hint->run_proc_receive) {
+			strbuf_addf(errmsg, "proc-receive reported status on ref of builtin command: %s\n",
+				    refname);
+			continue;
+		}
+		if (!strcmp(status, "ng")) {
+			if (msg)
+				hint->error_string = xstrdup(msg);
+			else
+				hint->error_string = "failed";
+			code = -1;
+		} else if (strcmp("ok", status)) {
+			strbuf_addf(errmsg, "proc-receive has bad status '%s' for '%s'\n",
+				    status, reader->line);
+			code = -1;
+			/* Skip marking it as RUN_PROC_RECEIVE_RETURNED */
+			continue;
+		}
+		oidcpy(&hint->old_oid, &old_oid);
+		oidcpy(&hint->new_oid, &new_oid);
+		hint->run_proc_receive |= RUN_PROC_RECEIVE_RETURNED;
+	}
+
+	for (cmd = commands; cmd; cmd = cmd->next)
+		if (cmd->run_proc_receive && !cmd->error_string &&
+		    !(cmd->run_proc_receive & RUN_PROC_RECEIVE_RETURNED)) {
+		    cmd->error_string = "no report from proc-receive";
+		    code = -1;
+		}
+
+	return code;
+}
+
+static int run_proc_receive_hook(struct command *commands,
+				 const struct string_list *push_options)
+{
+	struct child_process proc = CHILD_PROCESS_INIT;
+	struct async muxer;
+	struct command *cmd;
+	const char *argv[2];
+	struct packet_reader reader;
+	struct strbuf cap = STRBUF_INIT;
+	struct strbuf errmsg = STRBUF_INIT;
+	int pr_use_push_options = 0;
+	int version = 0;
+	int code;
+
+	argv[0] = find_hook("proc-receive");
+	if (!argv[0]) {
+		rp_error("cannot find hook 'proc-receive'");
+		return -1;
+	}
+	argv[1] = NULL;
+
+	proc.argv = argv;
+	proc.in = -1;
+	proc.out = -1;
+	proc.trace2_hook_name = "proc-receive";
+
+	if (use_sideband) {
+		memset(&muxer, 0, sizeof(muxer));
+		muxer.proc = copy_to_sideband;
+		muxer.in = -1;
+		code = start_async(&muxer);
+		if (code)
+			return code;
+		proc.err = muxer.in;
+	} else {
+		proc.err = 0;
+	}
+
+	code = start_command(&proc);
+	if (code) {
+		if (use_sideband)
+			finish_async(&muxer);
+		return code;
+	}
+
+	sigchain_push(SIGPIPE, SIG_IGN);
+
+	/* Version negotiaton */
+	packet_reader_init(&reader, proc.out, NULL, 0,
+			   PACKET_READ_CHOMP_NEWLINE |
+			   PACKET_READ_DIE_ON_ERR_PACKET);
+	if (use_atomic)
+		strbuf_addstr(&cap, " atomic");
+	if (use_push_options)
+		strbuf_addstr(&cap, " push-options");
+	if (cap.len) {
+		packet_write_fmt(proc.in, "version=1%c%s\n", '\0', cap.buf + 1);
+		strbuf_release(&cap);
+	} else {
+		packet_write_fmt(proc.in, "version=1\n");
+	}
+	packet_flush(proc.in);
+
+	for (;;) {
+		int linelen;
+
+		if (packet_reader_read(&reader) != PACKET_READ_NORMAL)
+			break;
+
+		if (reader.pktlen > 8 && starts_with(reader.line, "version=")) {
+			version = atoi(reader.line + 8);
+			linelen = strlen(reader.line);
+			if (linelen < reader.pktlen) {
+				const char *feature_list = reader.line + linelen + 1;
+				if (parse_feature_request(feature_list, "push-options"))
+					pr_use_push_options = 1;
+			}
+		}
+	}
+
+	if (version != 1) {
+		strbuf_addf(&errmsg, "proc-receive version '%d' is not supported",
+			    version);
+		code = -1;
+		goto cleanup;
+	}
+
+	/* Send commands */
+	for (cmd = commands; cmd; cmd = cmd->next) {
+		if (!cmd->run_proc_receive || cmd->skip_update || cmd->error_string)
+			continue;
+		packet_write_fmt(proc.in, "%s %s %s",
+				 oid_to_hex(&cmd->old_oid),
+				 oid_to_hex(&cmd->new_oid),
+				 cmd->ref_name);
+	}
+	packet_flush(proc.in);
+
+	/* Send push options */
+	if (pr_use_push_options) {
+		struct string_list_item *item;
+
+		for_each_string_list_item(item, push_options)
+			packet_write_fmt(proc.in, "%s", item->string);
+		packet_flush(proc.in);
+	}
+
+	/* Read result from proc-receive */
+	code = read_proc_receive_report(&reader, commands, &errmsg);
+
+cleanup:
+	close(proc.in);
+	close(proc.out);
+	if (use_sideband)
+		finish_async(&muxer);
+	if (finish_command(&proc))
+		code = -1;
+	if (errmsg.len >0) {
+		char *p = errmsg.buf;
+
+		p += errmsg.len - 1;
+		if (*p == '\n')
+			*p = '\0';
+		rp_error("%s", errmsg.buf);
+		strbuf_release(&errmsg);
+	}
+	sigchain_pop(SIGPIPE);
+
+	return code;
+}
+
 static char *refuse_unconfigured_deny_msg =
 	N_("By default, updating the current branch in a non-bare repository\n"
 	   "is denied, because it will make the index and work tree inconsistent\n"
@@ -1392,7 +1625,7 @@  static void execute_commands_non_atomic(struct command *commands,
 	struct strbuf err = STRBUF_INIT;
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
-		if (!should_process_cmd(cmd))
+		if (!should_process_cmd(cmd) || cmd->run_proc_receive)
 			continue;
 
 		transaction = ref_transaction_begin(&err);
@@ -1432,7 +1665,7 @@  static void execute_commands_atomic(struct command *commands,
 	}
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
-		if (!should_process_cmd(cmd))
+		if (!should_process_cmd(cmd) || cmd->run_proc_receive)
 			continue;
 
 		cmd->error_string = update(cmd, si);
@@ -1468,6 +1701,7 @@  static void execute_commands(struct command *commands,
 	struct iterate_data data;
 	struct async muxer;
 	int err_fd = 0;
+	int run_proc_receive = 0;
 
 	if (unpacker_error) {
 		for (cmd = commands; cmd; cmd = cmd->next)
@@ -1497,6 +1731,20 @@  static void execute_commands(struct command *commands,
 
 	reject_updates_to_hidden(commands);
 
+	/* Try to find commands that have special prefix in their reference names,
+	 * and mark them to run an external "proc-receive" hook later.
+	 */
+	for (cmd = commands; cmd; cmd = cmd->next) {
+		if (!should_process_cmd(cmd))
+			continue;
+
+		/* TODO: replace the fixed prefix by looking up git config variables. */
+		if (!strncmp(cmd->ref_name, "refs/for/", 9)) {
+			cmd->run_proc_receive = RUN_PROC_RECEIVE_SCHEDULED;
+			run_proc_receive = 1;
+		}
+	}
+
 	if (run_receive_hook(commands, "pre-receive", 0, push_options)) {
 		for (cmd = commands; cmd; cmd = cmd->next) {
 			if (!cmd->error_string)
@@ -1523,6 +1771,12 @@  static void execute_commands(struct command *commands,
 	free(head_name_to_free);
 	head_name = head_name_to_free = resolve_refdup("HEAD", 0, NULL, NULL);
 
+	if (run_proc_receive &&
+	    run_proc_receive_hook(commands, push_options))
+		for (cmd = commands; cmd; cmd = cmd->next)
+			if (!cmd->error_string  && (cmd->run_proc_receive || use_atomic))
+				cmd->error_string = "fail to run proc-receive hook";
+
 	if (use_atomic)
 		execute_commands_atomic(commands, si);
 	else
diff --git a/t/helper/test-proc-receive.c b/t/helper/test-proc-receive.c
new file mode 100644
index 0000000000..8fb143d692
--- /dev/null
+++ b/t/helper/test-proc-receive.c
@@ -0,0 +1,168 @@ 
+#include "cache.h"
+#include "connect.h"
+#include "parse-options.h"
+#include "pkt-line.h"
+#include "sigchain.h"
+#include "string-list.h"
+#include "test-tool.h"
+
+static const char *proc_receive_usage[] = {
+	"test-tool proc-receive [<options>...]",
+	NULL
+};
+
+static int version = 1;
+static int verbose = 0;
+static int no_push_options = 0;
+static int use_atomic = 0;
+static int use_push_options = 0;
+static struct string_list returns = STRING_LIST_INIT_NODUP;
+
+struct command {
+	struct command *next;
+	const char *error_string;
+	unsigned int skip_update:1,
+		     did_not_exist:1;
+	int index;
+	struct object_id old_oid;
+	struct object_id new_oid;
+	char ref_name[FLEX_ARRAY]; /* more */
+};
+
+static void proc_receive_verison(struct packet_reader *reader) {
+	int server_version = 0;
+
+	for (;;) {
+		int linelen;
+
+		if (packet_reader_read(reader) != PACKET_READ_NORMAL)
+			break;
+
+		if (reader->pktlen > 8 && starts_with(reader->line, "version=")) {
+			server_version = atoi(reader->line+8);
+			linelen = strlen(reader->line);
+			if (linelen < reader->pktlen) {
+				const char *feature_list = reader->line + linelen + 1;
+				if (parse_feature_request(feature_list, "atomic"))
+					use_atomic= 1;
+				if (parse_feature_request(feature_list, "push-options"))
+					use_push_options = 1;
+			}
+		}
+	}
+
+	if (server_version != 1)
+		die("bad protocol version: %d", server_version);
+
+	packet_write_fmt(1, "version=%d%c%s\n",
+			 version, '\0',
+			 use_push_options && !no_push_options ? "push-options": "");
+	packet_flush(1);
+}
+
+static void proc_receive_read_commands(struct packet_reader *reader,
+				       struct command **commands)
+{
+	struct command **tail = commands;
+
+	for (;;) {
+		struct object_id old_oid, new_oid;
+		struct command *cmd;
+		const char *refname;
+		const char *p;
+
+		if (packet_reader_read(reader) != PACKET_READ_NORMAL)
+			break;
+
+		if (parse_oid_hex(reader->line, &old_oid, &p) ||
+		    *p++ != ' ' ||
+		    parse_oid_hex(p, &new_oid, &p) ||
+		    *p++ != ' ')
+			die("protocol error: expected 'old new ref', got '%s'",
+			    reader->line);
+		refname = p;
+		FLEX_ALLOC_STR(cmd, ref_name, refname);
+		oidcpy(&cmd->old_oid, &old_oid);
+		oidcpy(&cmd->new_oid, &new_oid);
+
+		*tail = cmd;
+		tail = &cmd->next;
+	}
+}
+
+static void proc_receive_read_push_options(struct packet_reader *reader,
+					   struct string_list *options)
+{
+
+	if (no_push_options || !use_push_options)
+	       return;
+
+	while (1) {
+		if (packet_reader_read(reader) != PACKET_READ_NORMAL)
+			break;
+
+		string_list_append(options, reader->line);
+	}
+}
+
+int cmd__proc_receive(int argc, const char **argv)
+{
+	struct packet_reader reader;
+	struct command *commands = NULL;
+	struct string_list push_options = STRING_LIST_INIT_DUP;
+	struct string_list_item *item;
+	struct option options[] = {
+		OPT_BOOL(0, "no-push-options", &no_push_options,
+			 "disable push options"),
+		OPT_STRING_LIST('r', "return", &returns, "old/new/ref/status/msg",
+				"return of results"),
+		OPT__VERBOSE(&verbose, "be verbose"),
+		OPT_INTEGER('V', "version", &version,
+			    "use this protocol version number"),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, "test-tools", options, proc_receive_usage, 0);
+	if (argc > 0)
+		usage_msg_opt("Too many arguments.", proc_receive_usage, options);
+
+	packet_reader_init(&reader, 0, NULL, 0,
+			   PACKET_READ_CHOMP_NEWLINE |
+			   PACKET_READ_DIE_ON_ERR_PACKET);
+
+	sigchain_push(SIGPIPE, SIG_IGN);
+	proc_receive_verison(&reader);
+	proc_receive_read_commands(&reader, &commands);
+	proc_receive_read_push_options(&reader, &push_options);
+
+	if (verbose) {
+		struct command *cmd;
+
+		if (use_push_options || use_atomic)
+			fprintf(stderr, "proc-receive:%s%s\n",
+				use_atomic? " atomic": "",
+				use_push_options ? " push_options": "");
+
+		for (cmd = commands; cmd; cmd = cmd->next)
+			fprintf(stderr, "proc-receive< %s %s %s\n",
+				oid_to_hex(&cmd->old_oid),
+				oid_to_hex(&cmd->new_oid),
+				cmd->ref_name);
+
+		if (push_options.nr > 0)
+			for_each_string_list_item(item, &push_options)
+				fprintf(stderr, "proc-receive< %s\n", item->string);
+
+		if (returns.nr)
+			for_each_string_list_item(item, &returns)
+				fprintf(stderr, "proc-receive> %s\n", item->string);
+	}
+
+	if (returns.nr)
+		for_each_string_list_item(item, &returns)
+			packet_write_fmt(1, "%s\n", item->string);
+	packet_flush(1);
+	sigchain_pop(SIGPIPE);
+
+	return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 31eedcd241..f865433a61 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -44,6 +44,7 @@  static struct test_cmd cmds[] = {
 	{ "path-utils", cmd__path_utils },
 	{ "pkt-line", cmd__pkt_line },
 	{ "prio-queue", cmd__prio_queue },
+	{ "proc-receive", cmd__proc_receive},
 	{ "progress", cmd__progress },
 	{ "reach", cmd__reach },
 	{ "read-cache", cmd__read_cache },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 4eb5e6609e..059008cf22 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -34,6 +34,7 @@  int cmd__parse_pathspec_file(int argc, const char** argv);
 int cmd__path_utils(int argc, const char **argv);
 int cmd__pkt_line(int argc, const char **argv);
 int cmd__prio_queue(int argc, const char **argv);
+int cmd__proc_receive(int argc, const char **argv);
 int cmd__progress(int argc, const char **argv);
 int cmd__reach(int argc, const char **argv);
 int cmd__read_cache(int argc, const char **argv);
diff --git a/t/t5411/common-functions.sh b/t/t5411/common-functions.sh
index 6e400c0625..ef3913bbb5 100644
--- a/t/t5411/common-functions.sh
+++ b/t/t5411/common-functions.sh
@@ -34,8 +34,11 @@  create_commits_in () {
 # Format the output of git-push, git-show-ref and other commands to make a
 # user-friendly and stable text.  We can easily prepare the expect text
 # without having to worry about future changes of the commit ID and spaces
-# of the output.  We also replce single quotes with double quotes, because
-# it is boring to prepare unquoted single quotes in expect txt.
+# of the output.  Single quotes are replaced with double quotes, because
+# it is boring to prepare unquoted single quotes in expect txt.  We also
+# remove some locale error messages, which break test if we turn on
+# `GIT_TEST_GETTEXT_POISON=true` in order to test unintentional translations
+# on plumbing commands.
 make_user_friendly_and_stable_output_common () {
 	sed \
 		-e "s/  *\$//" \
@@ -46,5 +49,6 @@  make_user_friendly_and_stable_output_common () {
 		-e "s/$TAG/<TAG-v123>/g" \
 		-e "s/$ZERO_OID/<ZERO-OID>/g" \
 		-e "s/$(echo $A | cut -c1-7)[0-9a-f]*/<OID-A>/g" \
-		-e "s/$(echo $B | cut -c1-7)[0-9a-f]*/<OID-B>/g"
+		-e "s/$(echo $B | cut -c1-7)[0-9a-f]*/<OID-B>/g" \
+		-e "/^error: / d"
 }
diff --git a/t/t5411/common-test-cases.sh b/t/t5411/common-test-cases.sh
index 23655846e4..8e30c8f31e 100644
--- a/t/t5411/common-test-cases.sh
+++ b/t/t5411/common-test-cases.sh
@@ -41,3 +41,579 @@  test_expect_success "normal git-push command" '
 	EOF
 	test_cmp expect actual
 '
+
+# Refs of upstream : master(A)  tags/v123  refs/review/master/topic(A)  a/b/c(A)
+# Refs of workbench: master(A)  tags/v123
+test_expect_success "cleanup" '
+	(
+		cd "$upstream" &&
+		git update-ref -d refs/review/master/topic &&
+		git update-ref -d refs/tags/v123 &&
+		git update-ref -d refs/heads/a/b/c
+	)
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       next(A)  refs/for/master/topic(A)
+test_expect_success "no proc-receive hook, fail to push special ref" '
+	test_must_fail git -C workbench push origin \
+		HEAD:next \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: error: cannot find hook "proc-receive"
+	remote: # post-receive hook
+	remote: post-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	To <URL/of/upstream.git>
+	 * [new branch] HEAD -> next
+	 ! [remote rejected] HEAD -> refs/for/master/topic (fail to run proc-receive hook)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	<COMMIT-A> refs/heads/next
+	EOF
+	test_cmp expect actual
+'
+
+# Refs of upstream : master(A)             next(A)
+# Refs of workbench: master(A)  tags/v123
+test_expect_success "cleanup" '
+	git -C "$upstream" update-ref -d refs/heads/next
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push --atomic: (B)                   next(A)  refs/for/master/topic(A)
+test_expect_success "no proc-receive hook, fail all for atomic push" '
+	test_must_fail git -C workbench push --atomic origin \
+		$B:master \
+		HEAD:next \
+		HEAD:refs/for/master/topic >out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <COMMIT-A> <COMMIT-B> refs/heads/master
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: error: cannot find hook "proc-receive"
+	To <URL/of/upstream.git>
+	 ! [remote rejected] <COMMIT-B> -> master (fail to run proc-receive hook)
+	 ! [remote rejected] HEAD -> next (fail to run proc-receive hook)
+	 ! [remote rejected] HEAD -> refs/for/master/topic (fail to run proc-receive hook)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "setup proc-receive hook (bad version)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF &&
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v --version 2
+	EOF
+	chmod a+x "$upstream/hooks/proc-receive"
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/master/topic(A)
+test_expect_success "proc-receive bad protocol: unknown version" '
+	test_must_fail git -C workbench push origin \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+
+	# Check status report for git-push
+	sed -n \
+		-e "/^To / { s/   */ /g; p; }" \
+		-e "/^ / { s/   */ /g; p; }" \
+		<actual >actual-report &&
+	cat >expect <<-EOF &&
+	To <URL/of/upstream.git>
+	 ! [remote rejected] HEAD -> refs/for/master/topic (fail to run proc-receive hook)
+	EOF
+	test_cmp expect actual-report &&
+
+	# Check error message from "receive-pack", but ignore unstable fatal error
+	# message ("remote: fatal: the remote end hung up unexpectedly") which
+	# is different from the remote HTTP server with different locale settings.
+	sed -n -e "/^remote: error:/ { s/   */ /g; p; }" \
+		<actual >actual-error &&
+	cat >expect <<-EOF &&
+	remote: error: proc-receive version "2" is not supported
+	EOF
+	test_cmp expect actual-error &&
+
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "setup proc-receive hook (no report)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       next(A)  refs/for/master/topic(A)
+test_expect_success "proc-receive bad protocol: no report" '
+	test_must_fail git -C workbench push origin \
+		HEAD:refs/heads/next \
+		HEAD:refs/for/master/topic >out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # post-receive hook
+	remote: post-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	To <URL/of/upstream.git>
+	 * [new branch] HEAD -> next
+	 ! [remote rejected] HEAD -> refs/for/master/topic (no report from proc-receive)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	<COMMIT-A> refs/heads/next
+	EOF
+	test_cmp expect actual
+'
+
+# Refs of upstream : master(A)             next(A)
+# Refs of workbench: master(A)  tags/v123
+test_expect_success "cleanup" '
+	git -C "$upstream" update-ref -d refs/heads/next
+
+'
+
+test_expect_success "setup proc-receive hook (bad oid)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v \
+		-r "bad-id new-id ref ok"
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/master/topic
+test_expect_success "proc-receive bad protocol: bad oid" '
+	test_must_fail git -C workbench push origin \
+		HEAD:refs/for/master/topic\
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive> bad-id new-id ref ok
+	remote: error: proc-receive expected "old new ref status [msg]", got "bad-id new-id ref ok"
+	To <URL/of/upstream.git>
+	 ! [remote rejected] HEAD -> refs/for/master/topic (no report from proc-receive)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "setup proc-receive hook (no status)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v \
+		-r "$ZERO_OID $A refs/for/master/topic"
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/master/topic
+test_expect_success "proc-receive bad protocol: no status" '
+	test_must_fail git -C workbench push origin \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: error: proc-receive expected "old new ref status [msg]", got "<ZERO-OID> <COMMIT-A> refs/for/master/topic"
+	To <URL/of/upstream.git>
+	 ! [remote rejected] HEAD -> refs/for/master/topic (no report from proc-receive)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "setup proc-receive hook (unknown status)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v \
+		-r "$ZERO_OID $A refs/for/master/topic xx msg"
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/master/topic
+test_expect_success "proc-receive bad protocol: unknown status" '
+	test_must_fail git -C workbench push origin \
+			HEAD:refs/for/master/topic \
+			>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/for/master/topic xx msg
+	remote: error: proc-receive has bad status "xx" for "<ZERO-OID> <COMMIT-A> refs/for/master/topic"
+	To <URL/of/upstream.git>
+	 ! [remote rejected] HEAD -> refs/for/master/topic (no report from proc-receive)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "setup proc-receive hook (bad status)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v \
+		-r "$ZERO_OID $A refs/for/master/topic bad status"
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/master/topic
+test_expect_success "proc-receive bad protocol: bad status" '
+	test_must_fail git -C workbench push origin \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/for/master/topic bad status
+	remote: error: proc-receive has bad status "bad status" for "<ZERO-OID> <COMMIT-A> refs/for/master/topic"
+	To <URL/of/upstream.git>
+	 ! [remote rejected] HEAD -> refs/for/master/topic (no report from proc-receive)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "setup proc-receive hook (ng)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v \
+		-r "$ZERO_OID $A refs/for/master/topic ng"
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/master/topic
+test_expect_success "proc-receive: fail to update (no message)" '
+	test_must_fail git -C workbench push origin \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/for/master/topic ng
+	To <URL/of/upstream.git>
+	 ! [remote rejected] HEAD -> refs/for/master/topic (failed)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "setup proc-receive hook (ng message)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v \
+		-r "$ZERO_OID $A refs/for/master/topic ng error msg"
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/master/topic
+test_expect_success "proc-receive: fail to update (has message)" '
+	test_must_fail git -C workbench push origin \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/for/master/topic ng error msg
+	To <URL/of/upstream.git>
+	 ! [remote rejected] HEAD -> refs/for/master/topic (error msg)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "setup proc-receive hook (report status on builtin command)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v \
+		-r "$ZERO_OID $A refs/heads/master ok"
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         : (B)                   refs/for/master/topic
+test_expect_success "proc-receive: warning on report for builtin command" '
+	test_must_fail git -C workbench push origin \
+		$B:refs/heads/master \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <COMMIT-A> <COMMIT-B> refs/heads/master
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/heads/master ok
+	remote: error: proc-receive reported status on ref of builtin command: refs/heads/master
+	remote: # post-receive hook
+	remote: post-receive< <COMMIT-A> <COMMIT-B> refs/heads/master
+	To <URL/of/upstream.git>
+	 <OID-A>..<OID-B> <COMMIT-B> -> master
+	 ! [remote rejected] HEAD -> refs/for/master/topic (no report from proc-receive)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-B> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "cleanup" '
+	git -C "$upstream" update-ref refs/heads/master $A
+'
+
+test_expect_success "setup proc-receive hook (ok)" '
+	cat >"$upstream/hooks/proc-receive" <<-EOF
+	#!/bin/sh
+
+	printf >&2 "# proc-receive hook\n"
+
+	test-tool proc-receive -v \
+		-r "$ZERO_OID $A refs/for/master/topic ok"
+	EOF
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/master/topic
+test_expect_success "proc-receive: ok" '
+	git -C workbench push origin \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/for/master/topic ok
+	remote: # post-receive hook
+	remote: post-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	To <URL/of/upstream.git>
+	 * [new reference] HEAD -> refs/for/master/topic
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push         :                       refs/for/a/b/c/my/topic
+test_expect_success "proc-receive: no report from proc-receive" '
+	test_must_fail git -C workbench push origin \
+		HEAD:refs/for/a/b/c/my/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/a/b/c/my/topic
+	remote: # proc-receive hook
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/a/b/c/my/topic
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/for/master/topic ok
+	remote: error: proc-receive reported status on unknown ref: refs/for/master/topic
+	To <URL/of/upstream.git>
+	 ! [remote rejected] HEAD -> refs/for/a/b/c/my/topic (no report from proc-receive)
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push -o ...  :                       refs/for/master/topic
+test_expect_success "not support push options" '
+	test_must_fail git -C workbench push \
+		-o issue=123 \
+		-o reviewer=user1 \
+		origin \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	test_i18ngrep "fatal: the receiving end does not support push options" \
+		actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success "enable push options" '
+	git -C "$upstream" config receive.advertisePushOptions true
+'
+
+# Refs of upstream : master(A)
+# Refs of workbench: master(A)  tags/v123
+# git push -o ...  :                       next(A)  refs/for/master/topic
+test_expect_success "push with options" '
+	git -C workbench push \
+		--atomic \
+		-o issue=123 \
+		-o reviewer=user1 \
+		origin \
+		HEAD:refs/heads/next \
+		HEAD:refs/for/master/topic \
+		>out 2>&1 &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	remote: # pre-receive hook
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: # proc-receive hook
+	remote: proc-receive: atomic push_options
+	remote: proc-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	remote: proc-receive< issue=123
+	remote: proc-receive< reviewer=user1
+	remote: proc-receive> <ZERO-OID> <COMMIT-A> refs/for/master/topic ok
+	remote: # post-receive hook
+	remote: post-receive< <ZERO-OID> <COMMIT-A> refs/heads/next
+	remote: post-receive< <ZERO-OID> <COMMIT-A> refs/for/master/topic
+	To <URL/of/upstream.git>
+	 * [new branch] HEAD -> next
+	 * [new reference] HEAD -> refs/for/master/topic
+	EOF
+	test_cmp expect actual &&
+	git -C "$upstream" show-ref >out &&
+	make_user_friendly_and_stable_output <out >actual &&
+	cat >expect <<-EOF &&
+	<COMMIT-A> refs/heads/master
+	<COMMIT-A> refs/heads/next
+	EOF
+	test_cmp expect actual
+'