Skip to content

Commit

Permalink
transport-helper: mark failure for atomic push
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jiangxin authored and gitster committed Apr 17, 2020
1 parent 46701bd commit f38b168
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 6 deletions.
14 changes: 11 additions & 3 deletions t/t5541-http-push-smart.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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" &&
Expand All @@ -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 &&
Expand Down Expand Up @@ -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' '
Expand Down
5 changes: 2 additions & 3 deletions t/t5548-push-porcelain.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand All @@ -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 &&
Expand All @@ -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
Expand Down
15 changes: 15 additions & 0 deletions transport-helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit f38b168

Please sign in to comment.