[PATCH v5 3/3] push: --force-if-includes should allow fast-forward

[Tyler Cipriani <tyler@tylercipriani.com>]

In set_ref_status_for_push, we verify --force-if-includes's reflog
reachability checks before fast-forward rules. As a result, valid
fast-forward pushes may be rejected when a force push is unneeded; like
when the reflog is expired:

    git clone repo.git repo
    git commit --allow-empty -m 1
    git reflog expire --expire=all --all
    git push --force-with-lease --force-if-includes origin main
    ! [rejected]    main -> main (remote ref updated since checkout)

Rejecting fast-forwards is a mismatch with the --force-if-includes
documentation "Force an update only if the tip of the remote-tracking
ref has been integrated locally."

Instead, defer check for --force-if-includes until after determining if
a push force is needed.

Opted to create a deferred_reject_reason in set_ref_status_for_push
rather than move the computation of reachability or verifiability to
winnow scope of changes in this patch. Lazily checking for reachability
or verifiability is a valid followup.

Signed-off-by: Tyler Cipriani <tyler@tylercipriani.com>
---
 remote.c            | 16 +++++++++++++---
 t/t5533-push-cas.sh | 27 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/remote.c b/remote.c
index b7b5ac0d28..db0b50b030 100644
--- a/remote.c
+++ b/remote.c
@@ -1669,6 +1669,7 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 	for (ref = remote_refs; ref; ref = ref->next) {
 		int force_ref_update = ref->force || force_update;
 		int reject_reason = 0;
+		int deferred_reject_reason = 0;
 
 		if (ref->peer_ref)
 			oidcpy(&ref->new_oid, &ref->peer_ref->new_oid);
@@ -1693,16 +1694,17 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		 *
 		 * If the tip of the remote-tracking ref is unreachable
 		 * from any reflog entry of its local ref indicating a
-		 * possible update since checkout; reject the push.
+		 * possible update since checkout, then remember the
+		 * rejection in case the push is non-fast-forward.
 		 */
 		if (ref->expect_old_sha1) {
 			if (!oideq(&ref->old_oid, &ref->old_oid_expect))
 				reject_reason = REF_STATUS_REJECT_STALE;
 			else if (ref->check_reachable && ref->unreachable)
-				reject_reason =
+				deferred_reject_reason =
 					REF_STATUS_REJECT_REMOTE_UPDATED;
 			else if (ref->check_reachable && ref->unverifiable)
-				reject_reason =
+				deferred_reject_reason =
 					REF_STATUS_REJECT_UNVERIFIABLE;
 			else
 				/*
@@ -1746,6 +1748,14 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 				reject_reason = REF_STATUS_REJECT_NONFASTFORWARD;
 		}
 
+		/*
+		 * If push is non-fast-forward and we were asked to
+		 * verify the reflog but were unable to, then reflog
+		 * verification is the right reject_reason.
+		 */
+		if (deferred_reject_reason && reject_reason)
+			reject_reason = deferred_reject_reason;
+
 		/*
 		 * "--force" will defeat any rejection implemented
 		 * by the rules above.
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index 38576917e4..53e241c5b1 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -421,6 +421,33 @@ test_expect_success '"--force-if-includes" should allow forced update from HEAD'
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow fast-forward push without local reflog' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch main &&
+		git reset --hard origin/main &&
+		test_commit I &&
+		git reflog expire --expire=all --all &&
+		git push --force-with-lease --force-if-includes origin main
+	)
+'
+
+test_expect_success '"--force-if-includes" should allow fast-forward push from tag' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git fetch &&
+		git switch -c newbranch origin/main &&
+		test_commit I &&
+		git tag T &&
+		git push --force-with-lease --force-if-includes origin T:main
+	)
+'
+
 test_expect_success '"--force-if-includes" should reject forced update from differently named branches when local lacks remote ref' '
 	setup_src_dup_dst &&
 	test_when_finished "rm -fr dst src dup" &&
-- 
2.47.3