@matheusbsbs / matheus-bagliano / commits / 46701bde69
Fork 0

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>

Jiang Xin committed Apr 17, 2020 at 05:45 UTC 46701bde690f94fb1532bce110eae93d5f6b68a1
5 files changed +7 -19
send-pack.c
+2
@@ -332,6 +332,8 @@ static int atomic_push_failure(struct send_pack_args *args,
332 continue;
333
334 switch (ref->status) {
335 + case REF_STATUS_NONE:
336 + case REF_STATUS_OK:
337 case REF_STATUS_EXPECTING_REPORT:
338 ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
339 continue;
t/t5541-http-push-smart.sh
+1 -1
@@ -177,7 +177,7 @@ test_expect_success 'push (chunked)' '
177 test $HEAD = $(git rev-parse --verify HEAD))
178 '
179
180 -test_expect_success 'push --atomic also prevents branch creation, reports collateral' '
180 +test_expect_failure 'push --atomic also prevents branch creation, reports collateral' '
181 # Setup upstream repo - empty for now
182 d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&
183 git init --bare "$d" &&
t/t5543-atomic-push.sh
+3 -3
@@ -200,7 +200,7 @@ test_expect_success 'atomic push is not advertised if configured' '
200 # References in upstream : master(1) one(1) foo(1)
201 # References in workbench: master(2) foo(1) two(2) bar(2)
202 # Atomic push : master(2) two(2) bar(2)
203 -test_expect_failure 'atomic push reports (reject by update hook)' '
203 +test_expect_success 'atomic push reports (reject by update hook)' '
204 mk_repo_pair &&
205 (
206 cd workbench &&
@@ -241,7 +241,7 @@ test_expect_failure 'atomic push reports (reject by update hook)' '
241
242 # References in upstream : master(1) one(1) foo(1)
243 # References in workbench: master(2) foo(1) two(2) bar(2)
244 -test_expect_failure 'atomic push reports (mirror, but reject by update hook)' '
244 +test_expect_success 'atomic push reports (mirror, but reject by update hook)' '
245 (
246 cd workbench &&
247 git remote remove up &&
@@ -262,7 +262,7 @@ test_expect_failure 'atomic push reports (mirror, but reject by update hook)' '
262
263 # References in upstream : master(2) one(1) foo(1)
264 # References in workbench: master(1) foo(1) two(2) bar(2)
265 -test_expect_failure 'atomic push reports (reject by non-ff)' '
265 +test_expect_success 'atomic push reports (reject by non-ff)' '
266 rm upstream/.git/hooks/update &&
267 (
268 cd workbench &&
t/t5548-push-porcelain.sh
+1 -1
@@ -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_success "atomic push failed ($PROTOCOL)" '
139 + test_expect_failure "atomic push failed ($PROTOCOL)" '
140 (
141 cd workbench &&
142 git update-ref refs/heads/master $B &&
transport.c
-14
@@ -1248,20 +1248,6 @@ int transport_push(struct repository *r,
1248 err = push_had_errors(remote_refs);
1249 ret = push_ret | err;
1250
1251 - if ((flags & TRANSPORT_PUSH_ATOMIC) && err) {
1252 - struct ref *it;
1253 - for (it = remote_refs; it; it = it->next)
1254 - switch (it->status) {
1255 - case REF_STATUS_NONE:
1256 - case REF_STATUS_UPTODATE:
1257 - case REF_STATUS_OK:
1258 - it->status = REF_STATUS_ATOMIC_PUSH_FAILED;
1259 - break;
1260 - default:
1261 - break;
1262 - }
1263 - }
1264 -
1251 if (!quiet || err)
1252 transport_print_push_status(transport->url, remote_refs,
1253 verbose | porcelain, porcelain,