[PATCH v4 0/5] fix git-push porcelain output and atomic report issue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx>

Patch 1/5 fixes format of porcelain output for `git-push`, and it should
belong to a new topic, but it has a overlap with topic `jx/atomic-push`.

So I rebased `jx/atomic-push` on top of it, and resolved conflicts
between these two features.  Patch 2/5 ~ 5/5 are belong to the original
topic `jx/atomic-push`.

Note: Some patches of `jx/proc-receive-hook` also have overlap with this
      patch series, so v13 of `jx/proc-receive-hook` will base on this
      topic, instead of master branch of Git.

## Changes since `jx/atomic-push` v3:

* Fix broken test cases in t5504, and t5516, which is instroduced by
  patch 1/5..

## Changes since `jx/atomic-push` v2:

Add new patch 1/5 which fixes new issue found in git-push porcelain output.
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
may fail due to the failure of the `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...v4

-:  ---------- > 1:  96028ebf7d send-pack: fix inconsistent porcelain output
1:  7a0579ba13 = 2:  77f66ebfd2 t5543: never report what we do not push
2:  9b4bca8f4c ! 3:  cb63c4a9f4 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:  3fb57569da 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:  1c73ddff0d 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/t5504-fetch-receive-strict.sh |   1 +
 t/t5516-fetch-push.sh           |   1 +
 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 +
 9 files changed, 441 insertions(+), 44 deletions(-)
 create mode 100755 t/t5548-push-porcelain.sh

-- 
2.24.1.15.g448c31058d.agit.4.5




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux