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>
Jiang Xin committed
Apr 17, 2020 at 05:45 UTC
f38b16843dcbb575803dfeab39a4172b7469411d
3 files changed
+28
-6
t/t5541-http-push-smart.sh
+11
-3
@@ -177,7 +177,10 @@ test_expect_success 'push (chunked)' '
177
test $HEAD = $(git rev-parse --verify HEAD))
178
'
179
180
-test_expect_failure 'push --atomic also prevents branch creation, reports collateral' '
180
+## References of remote: atomic1(1) master(2) collateral(2) other(2)
181
+## References of local : atomic2(2) master(1) collateral(3) other(2) collateral1(3) atomic(1)
182
+## Atomic push : master(1) collateral(3) atomic(1)
183
+test_expect_success 'push --atomic also prevents branch creation, reports collateral' '
184
# Setup upstream repo - empty for now
185
d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&
186
git init --bare "$d" &&
@@ -189,7 +192,8 @@ test_expect_failure 'push --atomic also prevents branch creation, reports collat
192
test_commit atomic2 &&
193
git branch collateral &&
194
git branch other &&
192
- git push "$up" master collateral other &&
195
+ git push "$up" atomic1 master collateral other &&
196
+ git tag -d atomic1 &&
197
198
# collateral is a valid push, but should be failed by atomic push
199
git checkout collateral &&
@@ -224,7 +228,11 @@ test_expect_failure 'push --atomic also prevents branch creation, reports collat
228
229
# the collateral failure refs should be indicated to the user
230
grep "^ ! .*rejected.* atomic -> atomic .*atomic push failed" output &&
227
- grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output
231
+ grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output &&
232
+
233
+ # never report what we do not push
234
+ ! grep "^ ! .*rejected.* atomic1 " output &&
235
+ ! grep "^ ! .*rejected.* other " output
236
'
237
238
test_expect_success 'push --atomic fails on server-side errors' '
t/t5548-push-porcelain.sh
+2
-3
@@ -136,7 +136,7 @@ run_git_push_porcelain_output_test() {
136
# Refs of upstream : master(A) bar(B) baz(A) next(A)
137
# Refs of workbench: master(B) bar(A) baz(A) next(A)
138
# git-push : master(B) bar(A) NULL next(A)
139
- test_expect_failure "atomic push failed ($PROTOCOL)" '
139
+ test_expect_success "atomic push failed ($PROTOCOL)" '
140
(
141
cd workbench &&
142
git update-ref refs/heads/master $B &&
@@ -150,10 +150,10 @@ run_git_push_porcelain_output_test() {
150
make_user_friendly_and_stable_output <out >actual &&
151
cat >expect <<-EOF &&
152
To <URL/of/upstream.git>
153
+ = refs/heads/next:refs/heads/next [up to date]
154
! refs/heads/bar:refs/heads/bar [rejected] (non-fast-forward)
155
! (delete):refs/heads/baz [rejected] (atomic push failed)
156
! refs/heads/master:refs/heads/master [rejected] (atomic push failed)
156
- ! refs/heads/next:refs/heads/next [rejected] (atomic push failed)
157
Done
158
EOF
159
test_cmp expect actual &&
@@ -168,7 +168,6 @@ run_git_push_porcelain_output_test() {
168
EOF
169
test_cmp expect actual
170
'
171
-
171
test_expect_success "prepare pre-receive hook ($PROTOCOL)" '
172
write_script "$upstream/hooks/pre-receive" <<-EOF
173
exit 1
transport-helper.c
+15
@@ -894,6 +894,21 @@ static int push_refs_with_push(struct transport *transport,
894
case REF_STATUS_REJECT_STALE:
895
case REF_STATUS_REJECT_ALREADY_EXISTS:
896
if (atomic) {
897
+ /* Mark other refs as failed */
898
+ for (ref = remote_refs; ref; ref = ref->next) {
899
+ if (!ref->peer_ref && !mirror)
900
+ continue;
901
+
902
+ switch (ref->status) {
903
+ case REF_STATUS_NONE:
904
+ case REF_STATUS_OK:
905
+ case REF_STATUS_EXPECTING_REPORT:
906
+ ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
907
+ continue;
908
+ default:
909
+ break; /* do nothing */
910
+ }
911
+ }
912
string_list_clear(&cas_options, 0);
913
return 0;
914
} else