[PATCH v4] push: fix --force-if-includes when remote-tracking ref has no reflog

[Aleksei Sviridkin <f@lex.la>]

Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
2020-10-03), is_reachable_in_reflog() looks for the remote tip in the
local branch's reflog and stops at entries older than the newest entry
of the remote-tracking ref's reflog. That timestamp comes from a
callback of refs_for_each_reflog_ent_reverse(), which never runs when
the remote-tracking ref has no reflog, so the variable stays
uninitialized.

With the files backend a remote-tracking ref that "git clone" created
has no reflog until it moves. On my machine the leftover value exceeded
any real timestamp, so the walk stopped at the first entry and the push
was rejected with "remote ref updated since checkout" though nothing on
the remote had changed.

That stopping point assumes an entry older than the last recorded move
of the remote-tracking ref cannot be the one we want. The record itself
can be missing: never written, deleted, or expired by gc. Initialize
the timestamp to zero for a missing record. timestamp_t is unsigned, so
nothing compares older and the walk stops only at the remote tip or at
the end of the local reflog. "Now" brings the bug straight back. A
fixed age narrows it: the push is rejected when the remote tip is
recorded only past the first entry older than that age and nothing
collected reaches it.

When the remote tip is not in the local reflog at all, a stopped walk
and a full one fall back to the same merge-base check, and the full one
hands it more entries.

Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
Changes since v3:

- log message rewritten. Two things in it were wrong, not just
  unclear: it read as if a walk that stops at the cut-off skips the
  merge-base check, and it said there is "no such moment" when what is
  missing is the record of it.
- test uses setup_src_dup_dst and expires only the remote-tracking
  reflog.

t5533 passes 24/24 with the fix on files and on reftable, and the new
test fails on both without it. That failure is only reliable when built
with

	make CFLAGS_APPEND=-ftrivial-auto-var-init=pattern

otherwise the stack may hold a small number, as on your machine. So CI
would not catch this going uninitialized again.

One detail the expire hides: on files it leaves no reflog at all, on
reftable an empty one. The callback does not run either way.

The batch size growth looks worth its own patch. Not touched here.

 remote.c            |  2 +-
 t/t5533-push-cas.sh | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 00723b385e..6d301698ca 100644
--- a/remote.c
+++ b/remote.c
@@ -2751,7 +2751,7 @@ static int check_and_collect_until(const char *refname UNUSED,
  */
 static int is_reachable_in_reflog(const char *local, const struct ref *remote)
 {
-	timestamp_t date;
+	timestamp_t date = 0;
 	struct commit *commit;
 	struct commit **chunk;
 	struct check_and_collect_until_cb_data cb;
diff --git a/t/t5533-push-cas.sh b/t/t5533-push-cas.sh
index cba26a872d..c9aaeec8d1 100755
--- a/t/t5533-push-cas.sh
+++ b/t/t5533-push-cas.sh
@@ -396,4 +396,20 @@ test_expect_success '"--force-if-includes" should allow deletes' '
 	)
 '
 
+test_expect_success '"--force-if-includes" should allow forced update when remote-tracking ref has no reflog' '
+	setup_src_dup_dst &&
+	test_when_finished "rm -fr dst src dup" &&
+	(
+		cd src &&
+		git switch branch &&
+		git pull --rebase origin branch &&
+		# the bug needs a remote-tracking ref with no reflog, and
+		# the fetch above wrote one
+		git reflog expire --expire=all refs/remotes/origin/branch &&
+		git reset --hard HEAD^ &&
+		test_commit I &&
+		git push --force-if-includes --force-with-lease="branch"
+	)
+'
+
 test_done
-- 
2.55.0