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

[Junio C Hamano <gitster@pobox.com>]

Aleksei Sviridkin <f@lex.la> writes:

> 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.
>
> 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.
>
> Initialize the timestamp to zero, so that a remote-tracking ref
> without reflog makes the walk cover the whole reflog of the local
> branch, as documented.
>
> Signed-off-by: Aleksei Sviridkin <f@lex.la>
> Assisted-by: LLM

The last line adds no useful information, though.  Besides, you are
fully responsible for whatever LLM emitted and contributed into this
patch, so your sign-off must be the last line in the trailers.

> ---
> The new test fails without the fix on my machine (macOS, arm64). As
> the value read is uninitialized, other platforms may pass it by luck.

The code change looks good.

It is a bit surprising to see the fallout from a change 6 years ago
to be addressed now, and makes me wonder what else changed recently.
Certainly year 2026 is not the first year in which macOS on arm64
started becoming widely used, or you are not the only user of Git on
that platform.

>  remote.c            |  2 +-
>  t/t5533-push-cas.sh | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index 00723b3..6d30169 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 cba26a8..77f46f3 100755
> --- a/t/t5533-push-cas.sh
> +++ b/t/t5533-push-cas.sh
> @@ -396,4 +396,22 @@ 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' '
> +	rm -fr dst src &&
> +	git init --bare dst &&
> +	git push dst main main:branch &&
> +	git clone --no-local dst src &&
> +	test_when_finished "rm -fr dst src" &&

You'd want to move "test_when_finished" immediately before "git init
--bare dst", no?  That way, you can clean things up after any or the
"init", "push", "clone" fails (as well as the main part of the test
that is done in the subdirectory).

> +	(
> +		cd src &&
> +		# a clone leaves the remote-tracking refs without reflog
> +		# entries with the files backend, but not with reftable
> +		git reflog expire --all --expire=all &&
> +		git switch -c branch --track origin/branch &&
> +		git reset --hard HEAD^ &&
> +		test_commit D &&
> +		git push --force-if-includes --force-with-lease="branch"
> +	)
> +'
> +
>  test_done
>
> base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc