transport-helper: mark failure for atomic push

Commit v2.22.0-1-g3bca1e7f9f (transport-helper: enforce atomic in
push_refs_with_push, 2019-07-11) noticed the incomplete report of
failure of an atomic push for HTTP protocol.  But the implementation
has a flaw that mark all remote references as failure.

Only mark necessary references as failure in `push_refs_with_push()` of
transport-helper.

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 2c2c3fb0f51974dfb72e22c2233a010989093835..afc680d5e3d3940ee63a1564b9ef5aa1ca39c9a4 100755 (executable)
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -177,7 +177,10 @@ test_expect_success 'push (chunked)' '
         test $HEAD = $(git rev-parse --verify HEAD))
 '
 
-test_expect_failure 'push --atomic also prevents branch creation, reports collateral' '
+## References of remote: atomic1(1)            master(2) collateral(2) other(2)
+## References of local :            atomic2(2) master(1) collateral(3) other(2) collateral1(3) atomic(1)
+## Atomic push         :                       master(1) collateral(3)                         atomic(1)
+test_expect_success 'push --atomic also prevents branch creation, reports collateral' '
        # Setup upstream repo - empty for now
        d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&
        git init --bare "$d" &&
@@ -189,7 +192,8 @@ test_expect_failure 'push --atomic also prevents branch creation, reports collat
        test_commit atomic2 &&
        git branch collateral &&
        git branch other &&
-       git push "$up" master collateral other &&
+       git push "$up" atomic1 master collateral other &&
+       git tag -d atomic1 &&
 
        # collateral is a valid push, but should be failed by atomic push
        git checkout collateral &&
@@ -224,7 +228,11 @@ test_expect_failure 'push --atomic also prevents branch creation, reports collat
 
        # the collateral failure refs should be indicated to the user
        grep "^ ! .*rejected.* atomic -> atomic .*atomic push failed" output &&
-       grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output
+       grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output &&
+
+       # never report what we do not push
+       ! grep "^ ! .*rejected.* atomic1 " output &&
+       ! grep "^ ! .*rejected.* other " output
 '
 
 test_expect_success 'push --atomic fails on server-side errors' '

diff --git a/t/t5548-push-porcelain.sh b/t/t5548-push-porcelain.sh
index 9f4b7de74b141e8c83c44db37c605d3f096e6321..1b19b3ef559a3a0b74c421ab5de1b8da182d113a 100755 (executable)
--- a/t/t5548-push-porcelain.sh
+++ b/t/t5548-push-porcelain.sh
@@ -136,7 +136,7 @@ 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 &&
@@ -150,10 +150,10 @@ 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 &&
@@ -168,7 +168,6 @@ 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

diff --git a/transport-helper.c b/transport-helper.c
index 20a7185ec40e1cf4612c259019f325208632373b..ab3b52eb1451131e8af84857f7e4bcda829670a9 100644 (file)
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -894,6 +894,21 @@ static int push_refs_with_push(struct transport *transport,
                case REF_STATUS_REJECT_STALE:
                case REF_STATUS_REJECT_ALREADY_EXISTS:
                        if (atomic) {
+                               /* Mark other refs as failed */
+                               for (ref = remote_refs; ref; ref = ref->next) {
+                                       if (!ref->peer_ref && !mirror)
+                                               continue;
+
+                                       switch (ref->status) {
+                                       case REF_STATUS_NONE:
+                                       case REF_STATUS_OK:
+                                       case REF_STATUS_EXPECTING_REPORT:
+                                               ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
+                                               continue;
+                                       default:
+                                               break; /* do nothing */
+                                       }
+                               }
                                string_list_clear(&cas_options, 0);
                                return 0;
                        } else