fixup

Signed-off-by: Jiang Xin <[email protected]>

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) {

t/t5548-push-porcelain.sh

@@ -110,7 +110,7 @@ run_git_push_porcelain_output_test() {
 	# Refs of workbench: main(A)                  baz(A)  next(A)
 	# git-push         : main(A)  NULL    (B)     baz(A)  next(A)
 	test_expect_success "git-push --porcelain --dry-run ($PROTOCOL)" '
-		test_must_fail git -C workbench push --porcelain --dry-run origin \
+		git -C workbench push --porcelain --dry-run origin \
 			main \
 			:refs/heads/foo \
 			$B:bar \
@@ -176,7 +176,7 @@ run_git_push_porcelain_output_test() {
 	# Refs of workbench: main(A)                  baz(A)  next(A)
 	# git-push         : main(A)  NULL    (B)     baz(A)  next(A)
 	test_expect_success "git-push --porcelain --dry-run --atomic ($PROTOCOL)" '
-		test_must_fail git -C workbench push --porcelain --dry-run --atomic origin \
+		git -C workbench push --porcelain --dry-run --atomic origin \
 			main \
 			:refs/heads/foo \
 			$B:bar \
@@ -257,7 +257,6 @@ run_git_push_porcelain_output_test() {
 		> -	:refs/heads/foo	[deleted]
 		> *	refs/heads/next:refs/heads/next	[new branch]
 		> !	refs/heads/main:refs/heads/main	[rejected] (non-fast-forward)
-		> Done
 		EOF
 		test_cmp expect actual &&
 
@@ -325,7 +324,6 @@ run_git_push_porcelain_output_test() {
 		> !	(delete):refs/heads/foo	[rejected] (atomic push failed)
 		> !	refs/heads/main:refs/heads/main	[rejected] (non-fast-forward)
 		> !	refs/heads/next:refs/heads/next	[rejected] (atomic push failed)
-		> Done
 		EOF
 		test_cmp expect actual &&
 
@@ -396,7 +394,6 @@ run_git_push_porcelain_output_test() {
 		> !	:refs/heads/foo	[remote rejected] (pre-receive hook declined)
 		> !	refs/heads/main:refs/heads/main	[remote rejected] (pre-receive hook declined)
 		> !	refs/heads/next:refs/heads/next	[remote rejected] (pre-receive hook declined)
-		> Done
 		EOF
 		test_cmp expect actual &&
 

transport.c

@@ -914,7 +914,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
 	 * which may cause an error for `finish_connect()`. Ignore this error
 	 * for atomic git-push.
 	 */
-	if (ret || args.atomic)
+	if (ret || (args.atomic && args.dry_run))
 		finish_connect(data->conn);
 	else
 		ret = finish_connect(data->conn);
@@ -1462,7 +1462,17 @@ int transport_push(struct repository *r,
 	} else
 		push_ret = 0;
 	err = push_had_errors(remote_refs);
-	ret = push_ret | err;
+	/*
+	 * The return codes of push_refs() from different vtable are
+	 * different. For the HTTP protocol, push_ret is zero if there
+	 * are no connection errors or no protocol errors, we should
+	 * reference the return code of push_had_errors() to find out
+	 * whether there are refs fail to update.
+	 * For dry-run mode, we should ignore refs update errors, i.e.,
+	 * ignore the return code of push_had_errors(), but cannot ignore
+	 * the error found in vtable->push_refs().
+	 */
+	ret = (porcelain && pretend) ? push_ret : (push_ret | err);
 
 	if (!quiet || err)
 		transport_print_push_status(transport->url, remote_refs,
@@ -1479,7 +1489,7 @@ int transport_push(struct repository *r,
 			transport_update_tracking_ref(transport->remote, ref, verbose);
 	}
 
-	if (porcelain && !push_ret)
+	if (porcelain && !ret)
 		puts("Done");
 	else if (!quiet && !ret && !transport_refs_pushed(remote_refs))
 		/* stable plumbing output; do not modify or localize */