send-pack: run GPG after atomic push checking
The refs update commands can be sent to the server side in two different ways: GPG-signed or unsigned. We should run these two operations in the same "Finally, tell the other end!" code block, but they are seperated by the "Clear the status for each ref" code block. This will result in a slight performance loss, because the failed atomic push will still perform unnecessary preparations for shallow advertise and GPG-signed commands buffers, and user may have to be bothered by the (possible) GPG passphrase input when there is nothing to sign. Add a new test case to t5534 to ensure GPG will not be called when the GPG-signed atomic push fails. Signed-off-by: Han Xin <hanxin.hx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Han Xin committed
Sep 19, 2020 at 22:47 UTC
a4f324a423ddfe3680bf6105f8c113d0e76305a0
2 files changed
+53
-28
send-pack.c
+30
-28
@@ -244,7 +244,12 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar
244
return CHECK_REF_STATUS_REJECTED;
245
case REF_STATUS_UPTODATE:
246
return CHECK_REF_UPTODATE;
247
+
248
default:
249
+ case REF_STATUS_EXPECTING_REPORT:
250
+ /* already passed checks on the local side */
251
+ case REF_STATUS_OK:
252
+ /* of course this is OK */
253
return 0;
254
}
255
}
@@ -447,13 +452,6 @@ int send_pack(struct send_pack_args *args,
452
if (ref->deletion && !allow_deleting_refs)
453
ref->status = REF_STATUS_REJECT_NODELETE;
454
450
- if (!args->dry_run)
451
- advertise_shallow_grafts_buf(&req_buf);
452
-
453
- if (!args->dry_run && push_cert_nonce)
454
- cmds_sent = generate_push_cert(&req_buf, remote_refs, args,
455
- cap_buf.buf, push_cert_nonce);
456
-
455
/*
456
* Clear the status for each ref and see if we need to send
457
* the pack data.
@@ -489,31 +487,35 @@ int send_pack(struct send_pack_args *args,
487
ref->status = REF_STATUS_EXPECTING_REPORT;
488
}
489
490
+ if (!args->dry_run)
491
+ advertise_shallow_grafts_buf(&req_buf);
492
+
493
/*
494
* Finally, tell the other end!
495
*/
495
- for (ref = remote_refs; ref; ref = ref->next) {
496
- char *old_hex, *new_hex;
497
-
498
- if (args->dry_run || push_cert_nonce)
499
- continue;
500
-
501
- if (check_to_send_update(ref, args) < 0)
502
- continue;
503
-
504
- old_hex = oid_to_hex(&ref->old_oid);
505
- new_hex = oid_to_hex(&ref->new_oid);
506
- if (!cmds_sent) {
507
- packet_buf_write(&req_buf,
508
- "%s %s %s%c%s",
509
- old_hex, new_hex, ref->name, 0,
510
- cap_buf.buf);
511
- cmds_sent = 1;
512
- } else {
513
- packet_buf_write(&req_buf, "%s %s %s",
514
- old_hex, new_hex, ref->name);
496
+ if (!args->dry_run && push_cert_nonce)
497
+ cmds_sent = generate_push_cert(&req_buf, remote_refs, args,
498
+ cap_buf.buf, push_cert_nonce);
499
+ else if (!args->dry_run)
500
+ for (ref = remote_refs; ref; ref = ref->next) {
501
+ char *old_hex, *new_hex;
502
+
503
+ if (check_to_send_update(ref, args) < 0)
504
+ continue;
505
+
506
+ old_hex = oid_to_hex(&ref->old_oid);
507
+ new_hex = oid_to_hex(&ref->new_oid);
508
+ if (!cmds_sent) {
509
+ packet_buf_write(&req_buf,
510
+ "%s %s %s%c%s",
511
+ old_hex, new_hex, ref->name, 0,
512
+ cap_buf.buf);
513
+ cmds_sent = 1;
514
+ } else {
515
+ packet_buf_write(&req_buf, "%s %s %s",
516
+ old_hex, new_hex, ref->name);
517
+ }
518
}
516
- }
519
520
if (use_push_options) {
521
struct string_list_item *item;
t/t5534-push-signed.sh
+23
@@ -273,4 +273,27 @@ test_expect_success GPGSM 'fail without key and heed user.signingkey x509' '
273
test_cmp expect dst/push-cert-status
274
'
275
276
+test_expect_success GPG 'failed atomic push does not execute GPG' '
277
+ prepare_dst &&
278
+ git -C dst config receive.certnonceseed sekrit &&
279
+ write_script gpg <<-EOF &&
280
+ # should check atomic push locally before running GPG.
281
+ exit 1
282
+ EOF
283
+ test_must_fail env PATH="$TRASH_DIRECTORY:$PATH" git push \
284
+ --signed --atomic --porcelain \
285
+ dst noop ff noff >out 2>&1 &&
286
+
287
+ test_i18ngrep ! "gpg failed to sign" out &&
288
+ sed -n -e "/^To dst/,$ p" out >actual &&
289
+ cat >expect <<-EOF &&
290
+ To dst
291
+ = refs/heads/noop:refs/heads/noop [up to date]
292
+ ! refs/heads/ff:refs/heads/ff [rejected] (atomic push failed)
293
+ ! refs/heads/noff:refs/heads/noff [rejected] (non-fast-forward)
294
+ Done
295
+ EOF
296
+ test_i18ncmp expect actual
297
+'
298
+
299
test_done