send-pack: mark failure of atomic push properly

When pushing with SSH or other smart protocol, references are validated
by function `check_to_send_update()` before they are sent in commands
to `send_pack()` of "receve-pack".  For atomic push, if a reference is
rejected after the validation, only references pushed by user should be
marked as failure, instead of report failure on all remote references.

Commit v2.22.0-1-g3bca1e7f9f (transport-helper: enforce atomic in
push_refs_with_push, 2019-07-11) wanted to fix report issue of HTTP
protocol, but marked all remote references failure for atomic push.

In order to fix the issue of status report for SSH or other built-in
smart protocol, revert part of that commit and add additional status
for function `atomic_push_failure()`.  The additional status for it
except the "REF_STATUS_EXPECTING_REPORT" status are:

- REF_STATUS_NONE : Not marked as "REF_STATUS_EXPECTING_REPORT" yet.
- REF_STATUS_OK   : Assume OK for dryrun or status_report is disabled.

This fix won't resolve the issue of status report in transport-helper
for HTTP or other protocols, and breaks test case in t5541.  Will fix
it in additional commit.

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

diff --git a/send-pack.c b/send-pack.c
index 1835cd5582342db7a8c82161f9a9787838c7d948..efefb687b2da1329679eed01d62b5db3990ad907 100644 (file)
--- a/send-pack.c
+++ b/send-pack.c
@@ -332,6 +332,8 @@ static int atomic_push_failure(struct send_pack_args *args,
                        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;

diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
index 23be8ce92d67824166156cf0c5ca380022b79efc..2c2c3fb0f51974dfb72e22c2233a010989093835 100755 (executable)
--- a/t/t5541-http-push-smart.sh
+++ b/t/t5541-http-push-smart.sh
@@ -177,7 +177,7 @@ test_expect_success 'push (chunked)' '
         test $HEAD = $(git rev-parse --verify HEAD))
 '
 
-test_expect_success 'push --atomic also prevents branch creation, reports collateral' '
+test_expect_failure '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" &&

diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
index 001240eec717d68c7c9805353cc8d7655d5f8ab3..620c30d58f00b978bd20753acbf12181d5de2abe 100755 (executable)
--- a/t/t5543-atomic-push.sh
+++ b/t/t5543-atomic-push.sh
@@ -200,7 +200,7 @@ test_expect_success 'atomic push is not advertised if configured' '
 # References in upstream : master(1) one(1) foo(1)
 # References in workbench: master(2)        foo(1) two(2) bar(2)
 # Atomic push            : master(2)               two(2) bar(2)
-test_expect_failure 'atomic push reports (reject by update hook)' '
+test_expect_success 'atomic push reports (reject by update hook)' '
        mk_repo_pair &&
        (
                cd workbench &&
@@ -241,7 +241,7 @@ test_expect_failure 'atomic push reports (reject by update hook)' '
 
 # References in upstream : master(1) one(1) foo(1)
 # References in workbench: master(2)        foo(1) two(2) bar(2)
-test_expect_failure 'atomic push reports (mirror, but reject by update hook)' '
+test_expect_success 'atomic push reports (mirror, but reject by update hook)' '
        (
                cd workbench &&
                git remote remove up &&
@@ -262,7 +262,7 @@ test_expect_failure 'atomic push reports (mirror, but reject by update hook)' '
 
 # References in upstream : master(2) one(1) foo(1)
 # References in workbench: master(1)        foo(1) two(2) bar(2)
-test_expect_failure 'atomic push reports (reject by non-ff)' '
+test_expect_success 'atomic push reports (reject by non-ff)' '
        rm upstream/.git/hooks/update &&
        (
                cd workbench &&

diff --git a/t/t5548-push-porcelain.sh b/t/t5548-push-porcelain.sh
index 96ba449a383d49c188d7e14129c33b147b0c7c6b..9f4b7de74b141e8c83c44db37c605d3f096e6321 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_success "atomic push failed ($PROTOCOL)" '
+       test_expect_failure "atomic push failed ($PROTOCOL)" '
                (
                        cd workbench &&
                        git update-ref refs/heads/master $B &&

diff --git a/transport.c b/transport.c
index 13d638d5febf1824d90838afb9516db71a33d141..a07e39564c01570dac2803aef5872b2a8e7840b8 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -1248,20 +1248,6 @@ int transport_push(struct repository *r,
                err = push_had_errors(remote_refs);
                ret = push_ret | err;
 
-               if ((flags & TRANSPORT_PUSH_ATOMIC) && err) {
-                       struct ref *it;
-                       for (it = remote_refs; it; it = it->next)
-                               switch (it->status) {
-                               case REF_STATUS_NONE:
-                               case REF_STATUS_UPTODATE:
-                               case REF_STATUS_OK:
-                                       it->status = REF_STATUS_ATOMIC_PUSH_FAILED;
-                                       break;
-                               default:
-                                       break;
-                               }
-               }
-
                if (!quiet || err)
                        transport_print_push_status(transport->url, remote_refs,
                                        verbose | porcelain, porcelain,