[v3,0/5] fix git-push porcelain output and atomic report issue

Message

Jiang Xin April 16, 2020, 4:24 p.m. UTC

From: Jiang Xin <[e-mail]>

Find a new issue on porcelain output for git-push, add a new patch 1/5
to fix this issue.  This fix overlaps with and the topic `jx/atomic-push`,
so merge them together.

I find this issue when adding test cases of porcelain output for topic
`jx/proc-receive-hook`, which depends on this fix.

## Details of this new issue

The porcelain output of a failed `git-push` command is inconsistent for
different protocols.  For example, the following `git-push` command
failed because of the failure of `pre-receive` hook.

    git push --porcelain origin HEAD:refs/heads/master

For SSH protocol, the porcelain output does not end with a "Done"
message:

	To <URL/of/upstream.git>
	!  HEAD:refs/heads/master  [remote rejected] (pre-receive hook declined)

While for HTTP protocol, the porcelain output does end with a "Done"
message;

	To <URL/of/upstream.git>
	!  HEAD:refs/heads/master  [remote rejected] (pre-receive hook declined)
	Done

The following code at the end of function `send_pack()` indicates that
`send_pack()` should not return an error if some references are rejected
in porcelain mode.

    int send_pack(...)
        ... ...

        if (args->porcelain)
            return 0;

        for (ref = remote_refs; ref; ref = ref->next) {
            switch (ref->status) {
            case REF_STATUS_NONE:
            case REF_STATUS_UPTODATE:
            case REF_STATUS_OK:
                break;
            default:
                return -1;
            }
        }
        return 0;
    }

So if atomic push failed, must check the porcelain mode before return
an error.  And `receive_status()` should not return an error for a
failed updated reference, because `send_pack()` will check them instead.


## Range-diff v2...v3


-:  ---------- > 1:  d9ea3c35a3 send-pack: fix inconsistent porcelain output
1:  7a0579ba13 = 2:  bb07d5c330 t5543: never report what we do not push
2:  9b4bca8f4c ! 3:  1aa917b097 send-pack: mark failure of atomic push properly
    @@ t/t5543-atomic-push.sh: test_expect_failure 'atomic push reports (mirror, but re
      	(
      		cd workbench &&
     
    + ## t/t5548-push-porcelain.sh ##
    +@@ t/t5548-push-porcelain.sh: run_git_push_porcelain_output_test() {
    + 	# Refs of upstream : master(A)  bar(B)  baz(A)  next(A)
    + 	# Refs of workbench: master(B)  bar(A)  baz(A)  next(A)
    + 	# git-push         : master(B)  bar(A)  NULL    next(A)
    +-	test_expect_success "atomic push failed ($PROTOCOL)" '
    ++	test_expect_failure "atomic push failed ($PROTOCOL)" '
    + 		(
    + 			cd workbench &&
    + 			git update-ref refs/heads/master $B &&
    +
      ## transport.c ##
     @@ transport.c: int transport_push(struct repository *r,
      		err = push_had_errors(remote_refs);
3:  a7e8d7c893 ! 4:  2848088852 transport-helper: mark failure for atomic push
    @@ t/t5541-http-push-smart.sh: test_expect_failure 'push --atomic also prevents bra
      
      test_expect_success 'push --atomic fails on server-side errors' '
     
    + ## t/t5548-push-porcelain.sh ##
    +@@ t/t5548-push-porcelain.sh: run_git_push_porcelain_output_test() {
    + 	# Refs of upstream : master(A)  bar(B)  baz(A)  next(A)
    + 	# Refs of workbench: master(B)  bar(A)  baz(A)  next(A)
    + 	# git-push         : master(B)  bar(A)  NULL    next(A)
    +-	test_expect_failure "atomic push failed ($PROTOCOL)" '
    ++	test_expect_success "atomic push failed ($PROTOCOL)" '
    + 		(
    + 			cd workbench &&
    + 			git update-ref refs/heads/master $B &&
    +@@ t/t5548-push-porcelain.sh: run_git_push_porcelain_output_test() {
    + 		make_user_friendly_and_stable_output <out >actual &&
    + 		cat >expect <<-EOF &&
    + 		To <URL/of/upstream.git>
    ++		=    refs/heads/next:refs/heads/next    [up to date]
    + 		!    refs/heads/bar:refs/heads/bar    [rejected] (non-fast-forward)
    + 		!    (delete):refs/heads/baz    [rejected] (atomic push failed)
    + 		!    refs/heads/master:refs/heads/master    [rejected] (atomic push failed)
    +-		!    refs/heads/next:refs/heads/next    [rejected] (atomic push failed)
    + 		Done
    + 		EOF
    + 		test_cmp expect actual &&
    +@@ t/t5548-push-porcelain.sh: run_git_push_porcelain_output_test() {
    + 		EOF
    + 		test_cmp expect actual
    + 	'
    +-
    + 	test_expect_success "prepare pre-receive hook ($PROTOCOL)" '
    + 		write_script "$upstream/hooks/pre-receive" <<-EOF
    + 		exit 1
    +
      ## transport-helper.c ##
     @@ transport-helper.c: static int push_refs_with_push(struct transport *transport,
      		case REF_STATUS_REJECT_STALE:
4:  94b13f5dcd ! 5:  d2f0b50395 transport-helper: new method reject_atomic_push()
    @@ send-pack.c: int send_pack(struct send_pack_args *args,
      			if (use_atomic) {
      				strbuf_release(&req_buf);
      				strbuf_release(&cap_buf);
    --				return atomic_push_failure(args, remote_refs, ref);
    +-				atomic_push_failure(args, remote_refs, ref);
     +				reject_atomic_push(remote_refs, args->send_mirror);
    -+				return error("atomic push failed for ref %s. status: %d\n",
    -+					     ref->name, ref->status);
    ++				error("atomic push failed for ref %s. status: %d\n",
    ++				      ref->name, ref->status);
    + 				return args->porcelain ? 0 : -1;
      			}
      			/* else fallthrough */
    - 		default:
     
      ## transport-helper.c ##
     @@ transport-helper.c: static int push_refs_with_push(struct transport *transport,

---

Jiang Xin (5):
  send-pack: fix inconsistent porcelain output
  t5543: never report what we do not push
  send-pack: mark failure of atomic push properly
  transport-helper: mark failure for atomic push
  transport-helper: new method reject_atomic_push()

 send-pack.c                |  32 +---
 t/t5541-http-push-smart.sh |  12 +-
 t/t5543-atomic-push.sh     |  89 +++++++++++
 t/t5548-push-porcelain.sh  | 300 +++++++++++++++++++++++++++++++++++++
 transport-helper.c         |  23 +++
 transport.c                |  24 ++-
 transport.h                |   3 +
 7 files changed, 439 insertions(+), 44 deletions(-)
 create mode 100755 t/t5548-push-porcelain.sh