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

Aleksei Sviridkin [off-list ref] writes:

 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;

This gives a known value to the "date" variable, solving the issue
of using an uninitialized variable.  But how do we know if "0" a
reasonable fall-back value?  Why is it better than "now" or perhaps
"2 weeks ago"?

We pretend that the latest entry of the remote-tracking ref was from
year 1970.  And then that timestamp is used as a cut-off time for
check_and_collect_until().  What's the ramification of that?

Since 99a1f9ae10 (push: add reflog check for "--force-if-includes",
2020-10-03), is_reachable_in_reflog() stops walking the reflog of the
local branch at entries older than the newest reflog entry of the
remote-tracking ref. That timestamp is read by a callback of
refs_for_each_reflog_ent_reverse() into a variable that is never
initialized, so when the remote-tracking ref has no reflog the walk
is cut off at whatever happens to be on the stack.

This is almost good as-is.  I'd end the above with "... has no reflog,
the variable that holds the timestamp stays uninitialized".

With the files backend a remote-tracking ref created by "git clone"
has no reflog and does not get one until it moves. On my machine the
leftover value exceeds any real timestamp: the walk stops at the very
first entry, never reaches the "Created from" entry that "checkout
--track" wrote, and the push is rejected with "remote ref updated
since checkout" although nothing on the remote has changed.

That describes what happens (eh, rather, what does not happen) when
that uninitialized timestamp is more recent than the current time.

It does not explain why it is sensible to set it to year 1970, which
would force everything to be inspected.