Merge branch 'jx/atomic-push'

"git push --atomic" used to show failures for refs that weren't
even pushed, which has been corrected.

* jx/atomic-push:
  transport-helper: new method reject_atomic_push()
  transport-helper: mark failure for atomic push
  send-pack: mark failure of atomic push properly
  t5543: never report what we do not push
  send-pack: fix inconsistent porcelain output

send-pack.c

@@ -190,10 +190,8 @@ static int receive_status(struct packet_reader *reader, struct ref *refs)
 
 		if (reader->line[0] == 'o' && reader->line[1] == 'k')
 			hint->status = REF_STATUS_OK;
-		else {
+		else
 			hint->status = REF_STATUS_REMOTE_REJECT;
-			ret = -1;
-		}
 		hint->remote_status = xstrdup_or_null(msg);
 		/* start our next search from the next ref */
 		hint = hint->next;
@@ -322,29 +320,6 @@ static int generate_push_cert(struct strbuf *req_buf,
 	return update_seen;
 }
 
-
-static int atomic_push_failure(struct send_pack_args *args,
-			       struct ref *remote_refs,
-			       struct ref *failing_ref)
-{
-	struct ref *ref;
-	/* Mark other refs as failed */
-	for (ref = remote_refs; ref; ref = ref->next) {
-		if (!ref->peer_ref && !args->send_mirror)
-			continue;
-
-		switch (ref->status) {
-		case REF_STATUS_EXPECTING_REPORT:
-			ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
-			continue;
-		default:
-			break; /* do nothing */
-		}
-	}
-	return error("atomic push failed for ref %s. status: %d\n",
-		     failing_ref->name, failing_ref->status);
-}
-
 #define NONCE_LEN_LIMIT 256
 
 static void reject_invalid_nonce(const char *nonce, int len)
@@ -489,7 +464,10 @@ int send_pack(struct send_pack_args *args,
 			if (use_atomic) {
 				strbuf_release(&req_buf);
 				strbuf_release(&cap_buf);
-				return atomic_push_failure(args, remote_refs, ref);
+				reject_atomic_push(remote_refs, args->send_mirror);
+				error("atomic push failed for ref %s. status: %d\n",
+				      ref->name, ref->status);
+				return args->porcelain ? 0 : -1;
 			}
 			/* else fallthrough */
 		default:

t/t5504-fetch-receive-strict.sh

@@ -65,6 +65,7 @@ test_expect_success 'fetch with transfer.fsckobjects' '
 cat >exp <<EOF
 To dst
 !	refs/heads/master:refs/heads/test	[remote rejected] (missing necessary objects)
+Done
 EOF
 
 test_expect_success 'push without strict' '

t/t5516-fetch-push.sh

@@ -1066,6 +1066,7 @@ test_expect_success 'push --porcelain rejected' '
 
 	echo >.git/foo  "To testrepo"  &&
 	echo >>.git/foo "!	refs/heads/master:refs/heads/master	[remote rejected] (branch is currently checked out)" &&
+	echo >>.git/foo "Done" &&
 
 	test_must_fail git push >.git/bar --porcelain  testrepo refs/heads/master:refs/heads/master &&
 	test_cmp .git/foo .git/bar

t/t5541-http-push-smart.sh

@@ -177,6 +177,9 @@ test_expect_success 'push (chunked)' '
 	 test $HEAD = $(git rev-parse --verify HEAD))
 '
 
+## References of remote: atomic1(1)            master(2) collateral(2) other(2)
+## References of local :            atomic2(2) master(1) collateral(3) other(2) collateral1(3) atomic(1)
+## Atomic push         :                       master(1) collateral(3)                         atomic(1)
 test_expect_success 'push --atomic also prevents branch creation, reports collateral' '
 	# Setup upstream repo - empty for now
 	d=$HTTPD_DOCUMENT_ROOT_PATH/atomic-branches.git &&
@@ -189,7 +192,8 @@ test_expect_success 'push --atomic also prevents branch creation, reports collat
 	test_commit atomic2 &&
 	git branch collateral &&
 	git branch other &&
-	git push "$up" master collateral other &&
+	git push "$up" atomic1 master collateral other &&
+	git tag -d atomic1 &&
 
 	# collateral is a valid push, but should be failed by atomic push
 	git checkout collateral &&
@@ -224,7 +228,11 @@ test_expect_success 'push --atomic also prevents branch creation, reports collat
 
 	# the collateral failure refs should be indicated to the user
 	grep "^ ! .*rejected.* atomic -> atomic .*atomic push failed" output &&
-	grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output
+	grep "^ ! .*rejected.* collateral -> collateral .*atomic push failed" output &&
+
+	# never report what we do not push
+	! grep "^ ! .*rejected.* atomic1 " output &&
+	! grep "^ ! .*rejected.* other " output
 '
 
 test_expect_success 'push --atomic fails on server-side errors' '

t/t5543-atomic-push.sh

@@ -27,6 +27,12 @@ test_refs () {
 	test_cmp expect actual
 }
 
+fmt_status_report () {
+	sed -n \
+		-e "/^To / { s/   */ /g; p; }" \
+		-e "/^ ! / { s/   */ /g; p; }"
+}
+
 test_expect_success 'atomic push works for a single branch' '
 	mk_repo_pair &&
 	(
@@ -191,4 +197,87 @@ test_expect_success 'atomic push is not advertised if configured' '
 	test_refs master HEAD@{1}
 '
 
+# References in upstream : master(1) one(1) foo(1)
+# References in workbench: master(2)        foo(1) two(2) bar(2)
+# Atomic push            : master(2)               two(2) bar(2)
+test_expect_success 'atomic push reports (reject by update hook)' '
+	mk_repo_pair &&
+	(
+		cd workbench &&
+		test_commit one &&
+		git branch foo &&
+		git push up master one foo &&
+		git tag -d one
+	) &&
+	(
+		mkdir -p upstream/.git/hooks &&
+		cat >upstream/.git/hooks/update <<-EOF &&
+		#!/bin/sh
+
+		if test "\$1" = "refs/heads/bar"
+		then
+			echo >&2 "Pusing to branch bar is prohibited"
+			exit 1
+		fi
+		EOF
+		chmod a+x upstream/.git/hooks/update
+	) &&
+	(
+		cd workbench &&
+		test_commit two &&
+		git branch bar
+	) &&
+	test_must_fail git -C workbench \
+		push --atomic up master two bar >out 2>&1 &&
+	fmt_status_report <out >actual &&
+	cat >expect <<-EOF &&
+	To ../upstream
+	 ! [remote rejected] master -> master (atomic push failure)
+	 ! [remote rejected] two -> two (atomic push failure)
+	 ! [remote rejected] bar -> bar (hook declined)
+	EOF
+	test_cmp expect actual
+'
+
+# References in upstream : master(1) one(1) foo(1)
+# References in workbench: master(2)        foo(1) two(2) bar(2)
+test_expect_success 'atomic push reports (mirror, but reject by update hook)' '
+	(
+		cd workbench &&
+		git remote remove up &&
+		git remote add up ../upstream
+	) &&
+	test_must_fail git -C workbench \
+		push --atomic --mirror up >out 2>&1 &&
+	fmt_status_report <out >actual &&
+	cat >expect <<-EOF &&
+	To ../upstream
+	 ! [remote rejected] master -> master (atomic push failure)
+	 ! [remote rejected] one (atomic push failure)
+	 ! [remote rejected] bar -> bar (hook declined)
+	 ! [remote rejected] two -> two (atomic push failure)
+	EOF
+	test_cmp expect actual
+'
+
+# References in upstream : master(2) one(1) foo(1)
+# References in workbench: master(1)        foo(1) two(2) bar(2)
+test_expect_success 'atomic push reports (reject by non-ff)' '
+	rm upstream/.git/hooks/update &&
+	(
+		cd workbench &&
+		git push up master &&
+		git reset --hard HEAD^
+	) &&
+	test_must_fail git -C workbench \
+		push --atomic up master foo bar >out 2>&1 &&
+	fmt_status_report <out >actual &&
+	cat >expect <<-EOF &&
+	To ../upstream
+	 ! [rejected] master -> master (non-fast-forward)
+	 ! [rejected] bar -> bar (atomic push failed)
+	EOF
+	test_cmp expect actual
+'
+
 test_done