Merge branch 'ps/send-pack-unhide-error-in-atomic-push' into seen

gitster · gitster · commit 3c6761e96955 · 2024-11-20T14:50:52.000+09:00
"git push --atomic --porcelain" used to ignore failures from the
other side, losing the error status from the child process, which
has been corrected.

* ps/send-pack-unhide-error-in-atomic-push:
  push: not send push-options to server with --dry-run
  push: only ignore finish_connect() for dry-run mode
  t5543: atomic push reports exit code failure
  t5504: modernize test by moving heredocs into test bodies
  push: fix the behavior of the Done message for porcelain
  t5548: new test cases for push --porcelain and --dry-run
diff --git a/send-pack.c b/send-pack.c
@@ -630,7 +630,7 @@ int send_pack(struct send_pack_args *args,
 				reject_atomic_push(remote_refs, args->send_mirror);
 				error("atomic push failed for ref %s. status: %d",
 				      ref->name, ref->status);
-				ret = args->porcelain ? 0 : -1;
+				ret = (args->porcelain && args->dry_run) ? 0 : -1;
 				goto out;
 			}
 			/* else fallthrough */
@@ -678,7 +678,7 @@ int send_pack(struct send_pack_args *args,
 		}
 	}
 
-	if (use_push_options) {
+	if (use_push_options && !args->dry_run) {
 		struct string_list_item *item;
 
 		packet_buf_flush(&req_buf);
@@ -760,11 +760,12 @@ int send_pack(struct send_pack_args *args,
 
 	if (ret < 0)
 		goto out;
-
-	if (args->porcelain) {
-		ret = 0;
+	else if (args->porcelain && args->dry_run)
+		/*
+		 * Knowing a ref will be rejected in a --dry-run does not
+		 * count as an error.
+		 */
 		goto out;
-	}
 
 	for (ref = remote_refs; ref; ref = ref->next) {
 		switch (ref->status) {
diff --git a/t/t5411/test-0001-standard-git-push--porcelain.sh b/t/t5411/test-0001-standard-git-push--porcelain.sh
@@ -73,7 +73,6 @@ test_expect_success "non-fast-forward git-push ($PROTOCOL/porcelain)" '
 	> To <URL/of/upstream.git>
 	>  	<COMMIT-B>:refs/heads/next	<COMMIT-A>..<COMMIT-B>
 	> !	refs/heads/main:refs/heads/main	[rejected] (non-fast-forward)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
diff --git a/t/t5411/test-0003-pre-receive-declined--porcelain.sh b/t/t5411/test-0003-pre-receive-declined--porcelain.sh
@@ -18,7 +18,6 @@ test_expect_success "git-push is declined ($PROTOCOL/porcelain)" '
 	> To <URL/of/upstream.git>
 	> !	<COMMIT-B>:refs/heads/main	[remote rejected] (pre-receive hook declined)
 	> !	HEAD:refs/heads/next	[remote rejected] (pre-receive hook declined)
-	Done
 	EOF
 	test_cmp expect actual &&
 
diff --git a/t/t5411/test-0012-no-hook-error--porcelain.sh b/t/t5411/test-0012-no-hook-error--porcelain.sh
@@ -17,7 +17,6 @@ test_expect_success "proc-receive: no hook, fail to push special ref ($PROTOCOL/
 	> To <URL/of/upstream.git>
 	> *	HEAD:refs/heads/next	[new branch]
 	> !	HEAD:refs/for/main/topic	[remote rejected] (fail to run proc-receive hook)
-	Done
 	EOF
 	test_cmp expect actual &&
 
@@ -52,7 +51,6 @@ test_expect_success "proc-receive: no hook, all failed for atomic push ($PROTOCO
 	> !	<COMMIT-B>:refs/heads/main	[remote rejected] (fail to run proc-receive hook)
 	> !	HEAD:refs/heads/next	[remote rejected] (fail to run proc-receive hook)
 	> !	HEAD:refs/for/main/topic	[remote rejected] (fail to run proc-receive hook)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
diff --git a/t/t5411/test-0014-bad-protocol--porcelain.sh b/t/t5411/test-0014-bad-protocol--porcelain.sh
@@ -21,7 +21,6 @@ test_expect_success "proc-receive: bad protocol (unknown version, $PROTOCOL/porc
 	cat >expect <<-EOF &&
 	To <URL/of/upstream.git>
 	!	HEAD:refs/for/main/topic	[remote rejected] (fail to run proc-receive hook)
-	Done
 	EOF
 	test_cmp expect actual-report &&
 
@@ -59,7 +58,6 @@ test_expect_success "proc-receive: bad protocol (hook --die-read-version, $PROTO
 	cat >expect <<-EOF &&
 	To <URL/of/upstream.git>
 	!	HEAD:refs/for/main/topic	[remote rejected] (fail to run proc-receive hook)
-	Done
 	EOF
 	test_cmp expect actual &&
 	grep "remote: fatal: die with the --die-read-version option" out-$test_count &&
@@ -90,7 +88,6 @@ test_expect_success "proc-receive: bad protocol (hook --die-write-version, $PROT
 	cat >expect <<-EOF &&
 	To <URL/of/upstream.git>
 	!	HEAD:refs/for/main/topic	[remote rejected] (fail to run proc-receive hook)
-	Done
 	EOF
 	test_cmp expect actual &&
 	grep "remote: fatal: die with the --die-write-version option" out-$test_count &&
@@ -121,7 +118,6 @@ test_expect_success "proc-receive: bad protocol (hook --die-read-commands, $PROT
 	cat >expect <<-EOF &&
 	To <URL/of/upstream.git>
 	!	HEAD:refs/for/main/topic	[remote rejected] (fail to run proc-receive hook)
-	Done
 	EOF
 	test_cmp expect actual &&
 	grep "remote: fatal: die with the --die-read-commands option" out-$test_count &&
@@ -153,7 +149,6 @@ test_expect_success "proc-receive: bad protocol (hook --die-read-push-options, $
 	cat >expect <<-EOF &&
 	To <URL/of/upstream.git>
 	!	HEAD:refs/for/main/topic	[remote rejected] (fail to run proc-receive hook)
-	Done
 	EOF
 	test_cmp expect actual &&
 	grep "remote: fatal: die with the --die-read-push-options option" out-$test_count &&
@@ -183,7 +178,6 @@ test_expect_success "proc-receive: bad protocol (hook --die-write-report, $PROTO
 	cat >expect <<-EOF &&
 	To <URL/of/upstream.git>
 	!	HEAD:refs/for/main/topic	[remote rejected] (fail to run proc-receive hook)
-	Done
 	EOF
 	test_cmp expect actual &&
 	grep "remote: fatal: die with the --die-write-report option" out-$test_count &&
@@ -219,7 +213,6 @@ test_expect_success "proc-receive: bad protocol (no report, $PROTOCOL/porcelain)
 	> To <URL/of/upstream.git>
 	> *	HEAD:refs/heads/next	[new branch]
 	> !	HEAD:refs/for/main/topic	[remote rejected] (proc-receive failed to report status)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
@@ -260,7 +253,6 @@ test_expect_success "proc-receive: bad protocol (no ref, $PROTOCOL/porcelain)" '
 	> remote: error: proc-receive reported incomplete status line: "ok"        Z
 	> To <URL/of/upstream.git>
 	> !	HEAD:refs/for/main/topic	[remote rejected] (proc-receive failed to report status)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
@@ -294,7 +286,6 @@ test_expect_success "proc-receive: bad protocol (unknown status, $PROTOCOL/porce
 	> remote: error: proc-receive reported bad status "xx" on ref "refs/for/main/topic"        Z
 	> To <URL/of/upstream.git>
 	> !	HEAD:refs/for/main/topic	[remote rejected] (proc-receive failed to report status)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
diff --git a/t/t5411/test-0021-report-ng--porcelain.sh b/t/t5411/test-0021-report-ng--porcelain.sh
@@ -22,7 +22,6 @@ test_expect_success "proc-receive: fail to update (ng, no message, $PROTOCOL/por
 	> remote: proc-receive> ng refs/for/main/topic        Z
 	> To <URL/of/upstream.git>
 	> !	HEAD:refs/for/main/topic	[remote rejected] (failed)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
@@ -55,7 +54,6 @@ test_expect_success "proc-receive: fail to update (ng, with message, $PROTOCOL/p
 	> remote: proc-receive> ng refs/for/main/topic error msg        Z
 	> To <URL/of/upstream.git>
 	> !	HEAD:refs/for/main/topic	[remote rejected] (error msg)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
diff --git a/t/t5411/test-0023-report-unexpect-ref--porcelain.sh b/t/t5411/test-0023-report-unexpect-ref--porcelain.sh
@@ -28,7 +28,6 @@ test_expect_success "proc-receive: report unexpected ref ($PROTOCOL/porcelain)"
 	> To <URL/of/upstream.git>
 	>  	<COMMIT-B>:refs/heads/main	<COMMIT-A>..<COMMIT-B>
 	> !	HEAD:refs/for/main/topic	[remote rejected] (proc-receive failed to report status)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
diff --git a/t/t5411/test-0025-report-unknown-ref--porcelain.sh b/t/t5411/test-0025-report-unknown-ref--porcelain.sh
@@ -23,7 +23,6 @@ test_expect_success "proc-receive: report unknown reference ($PROTOCOL/porcelain
 	> remote: error: proc-receive reported status on unknown ref: refs/for/main/topic        Z
 	> To <URL/of/upstream.git>
 	> !	HEAD:refs/for/a/b/c/my/topic	[remote rejected] (proc-receive failed to report status)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
diff --git a/t/t5411/test-0033-report-with-options--porcelain.sh b/t/t5411/test-0033-report-with-options--porcelain.sh
@@ -25,7 +25,6 @@ test_expect_success "proc-receive: report option without matching ok ($PROTOCOL/
 	> remote: error: proc-receive reported "option" without a matching "ok/ng" directive        Z
 	> To <URL/of/upstream.git>
 	> !	HEAD:refs/for/main/topic	[remote rejected] (proc-receive failed to report status)
-	> Done
 	EOF
 	test_cmp expect actual
 '
diff --git a/t/t5411/test-0039-report-mixed-refs--porcelain.sh b/t/t5411/test-0039-report-mixed-refs--porcelain.sh
@@ -63,7 +63,6 @@ test_expect_success "proc-receive: report update of mixed refs ($PROTOCOL/porcel
 	>  	HEAD:refs/for/main/topic	<COMMIT-A>..<COMMIT-B>
 	> !	HEAD:refs/for/next/topic1	[remote rejected] (fail to call Web API)
 	> !	HEAD:refs/for/next/topic3	[remote rejected] (proc-receive failed to report status)
-	> Done
 	EOF
 	test_cmp expect actual &&
 
diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh
@@ -64,12 +64,6 @@ test_expect_success 'fetch with transfer.fsckobjects' '
 	)
 '
 
-cat >exp <<EOF
-To dst
-!	refs/heads/main:refs/heads/test	[remote rejected] (missing necessary objects)
-Done
-EOF
-
 test_expect_success 'push without strict' '
 	rm -rf dst &&
 	git init dst &&
@@ -78,6 +72,10 @@ test_expect_success 'push without strict' '
 		git config fetch.fsckobjects false &&
 		git config transfer.fsckobjects false
 	) &&
+	cat >exp <<-EOF &&
+	To dst
+	!	refs/heads/main:refs/heads/test	[remote rejected] (missing necessary objects)
+	EOF
 	test_must_fail git push --porcelain dst main:refs/heads/test >act &&
 	test_cmp exp act
 '
@@ -94,11 +92,6 @@ test_expect_success 'push with !receive.fsckobjects' '
 	test_cmp exp act
 '
 
-cat >exp <<EOF
-To dst
-!	refs/heads/main:refs/heads/test	[remote rejected] (unpacker error)
-EOF
-
 test_expect_success 'push with receive.fsckobjects' '
 	rm -rf dst &&
 	git init dst &&
@@ -107,6 +100,10 @@ test_expect_success 'push with receive.fsckobjects' '
 		git config receive.fsckobjects true &&
 		git config transfer.fsckobjects false
 	) &&
+	cat >exp <<-EOF &&
+	To dst
+	!	refs/heads/main:refs/heads/test	[remote rejected] (unpacker error)
+	EOF
 	test_must_fail git push --porcelain dst main:refs/heads/test >act &&
 	test_cmp exp act
 '
@@ -129,15 +126,14 @@ test_expect_success 'repair the "corrupt or missing" object' '
 	git fsck
 '
 
-cat >bogus-commit <<EOF
-tree $EMPTY_TREE
-author Bugs Bunny 1234567890 +0000
-committer Bugs Bunny <bugs@bun.ni> 1234567890 +0000
-
-This commit object intentionally broken
-EOF
-
 test_expect_success 'setup bogus commit' '
+	cat >bogus-commit <<-EOF &&
+	tree $EMPTY_TREE
+	author Bugs Bunny 1234567890 +0000
+	committer Bugs Bunny <bugs@bun.ni> 1234567890 +0000
+
+	This commit object intentionally broken
+	EOF
 	commit="$(git hash-object --literally -t commit -w --stdin <bogus-commit)"
 '
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
@@ -1189,7 +1189,6 @@ test_expect_success 'push --porcelain rejected' '
 
 	echo >.git/foo  "To testrepo"  &&
 	echo >>.git/foo "!	refs/heads/main:refs/heads/main	[remote rejected] (branch is currently checked out)" &&
-	echo >>.git/foo "Done" &&
 
 	test_must_fail git push >.git/bar --porcelain  testrepo refs/heads/main:refs/heads/main &&
 	test_cmp .git/foo .git/bar
@@ -1206,7 +1205,7 @@ test_expect_success 'push --porcelain --dry-run rejected' '
 	echo >>.git/foo "!	refs/heads/main^:refs/heads/main	[rejected] (non-fast-forward)" &&
 	echo >>.git/foo "Done" &&
 
-	test_must_fail git push >.git/bar --porcelain  --dry-run testrepo refs/heads/main^:refs/heads/main &&
+	git push >.git/bar --porcelain  --dry-run testrepo refs/heads/main^:refs/heads/main &&
 	test_cmp .git/foo .git/bar
 '
 
diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
@@ -384,7 +384,6 @@ test_expect_success GPG 'failed atomic push does not execute GPG' '
 	=	refs/heads/noop:refs/heads/noop	[up to date]
 	!	refs/heads/ff:refs/heads/ff	[rejected] (atomic push failed)
 	!	refs/heads/noff:refs/heads/noff	[rejected] (non-fast-forward)
-	Done
 	EOF
 	test_cmp expect out
 '
diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh
@@ -498,7 +498,6 @@ test_expect_success 'report error server does not provide ref status' '
 	cat >expect <<-EOF &&
 	To $HTTPD_URL/smart/no_report
 	!	HEAD:refs/tags/will-fail	[remote failure] (remote failed to report status)
-	Done
 	EOF
 	test_cmp expect actual
 '
diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
@@ -280,4 +280,34 @@ test_expect_success 'atomic push reports (reject by non-ff)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'atomic push reports exit code failure' '
+	write_script receive-pack-wrapper <<-\EOF &&
+	git-receive-pack "$@"
+	exit 1
+	EOF
+	test_must_fail git -C workbench push --atomic \
+		--receive-pack="${SQ}$(pwd)${SQ}/receive-pack-wrapper" \
+		up HEAD:refs/heads/no-conflict 2>err &&
+	cat >expect <<-EOF &&
+	To ../upstream
+	 * [new branch]      HEAD -> no-conflict
+	error: failed to push some refs to ${SQ}../upstream${SQ}
+	EOF
+	test_cmp expect err
+'
+
+test_expect_success 'atomic push reports exit code failure with porcelain' '
+	write_script receive-pack-wrapper <<-\EOF &&
+	git-receive-pack "$@"
+	exit 1
+	EOF
+	test_must_fail git -C workbench push --atomic --porcelain \
+		--receive-pack="${SQ}$(pwd)${SQ}/receive-pack-wrapper" \
+		up HEAD:refs/heads/no-conflict-porcelain 2>err &&
+	cat >expect <<-EOF &&
+	error: failed to push some refs to ${SQ}../upstream${SQ}
+	EOF
+	test_cmp expect err
+'
+
 test_done
diff --git a/t/t5548-push-porcelain.sh b/t/t5548-push-porcelain.sh
diff --git a/transport.c b/transport.c

Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,6 @@ test_expect_success "proc-receive: report option without matching ok ($PROTOCOL/`
`25`	`25`	`> remote: error: proc-receive reported "option" without a matching "ok/ng" directive Z`
`26`	`26`	`> To <URL/of/upstream.git>`
`27`	`27`	`> ! HEAD:refs/for/main/topic [remote rejected] (proc-receive failed to report status)`
`28`		`- > Done`
`29`	`28`	`EOF`
`30`	`29`	`test_cmp expect actual`
`31`	`30`	`'`