From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 29 Sep 2026 15:46:03 GMT
Subject: Re: [PATCH v4 4/5] t5520: don't expire reflogs where it matters
Message-ID: <3547f4aa-649a-4f46-868c-0e50dfa69466@gmail.com>
In-Reply-To: <2ac371d2dc1425cc47bf369e88b321d3c0c8c605.1790684309.git.ben.knoble@gmail.com>

On 29/09/2026 13:18, D. Ben Knoble wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
> 
> The "--rebase -f with rebased upstream" test computes its fork point
> from the reflog of refs/remotes/me/copy, and the entry it needs is
> the one that the fetch of the test before it wrote. Like every reflog
> entry the suite writes after test_tick, it is dated 2005, so the
> first "git reflog expire --all" after that fetch removes it. Pull
> then finds no fork point and rebases onto the merge head with the
> merge head as the upstream, and the rewound commits come back as a
> conflict.
> 
> Since 452b12c2e0 (builtin/maintenance: use "geometric" strategy by
> default, 2026-02-24) auto maintenance runs that expiry once the reflog
> of HEAD holds a hundred entries it would remove, the default of
> maintenance.reflog-expire.auto. Which run crosses the threshold
> depends on the entries and maintenance runs before it, so the script
> passed by chance: a stash topic that no longer runs "git reset" from
> "stash apply --index" and a rebase topic that runs auto maintenance
> at the end of "git rebase" together move the expiry between the two
> tests.
> 
> Pin the expiry as ea7d894f44 (t34xx: don't expire reflogs where it
> matters, 2026-02-24) did for the rebase tests. That covers a "git gc"
> as well, which expires reflogs on its own, where turning off the auto
> trigger of the reflog-expire task alone would not.

I find this commit message quite hard to understand. From my perspective 
the important points are

  - "git merge" uses "git stash" to clear any uncommitted changes from
    the worktree before it tries each strategy. The stashed changes are
    popped with "--index".

  - switching "git stash pop --index" to use merge_incore_nonrecursive()
    causes "git merge" to stop writing the reflog entries that came from
    "git stash pop --index" running "git reset"

  - that combined with "git rebase" starting to run "git maintenance
    --auto" changed when we expire the reflogs which breaks the fork-
    point detection.

The changes themselves look good

Thanks

Phillip

> 
> Reported-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: D. Ben Knoble <ben.knoble@gmail.com>
> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Assisted-by: Claude Fable 5.1
> Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
> Signed-off-by: D. Ben Knoble <ben.knoble@gmail.com>
> ---
>   t/t5520-pull.sh | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 27f38ab3c8..bc818605a5 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -35,6 +35,12 @@ test_pull_autostash_fail () {
>   }
>   
>   test_expect_success setup '
> +	# Commit dates are hardcoded to 2005, and the reflog entries will have
> +	# a matching timestamp. Maintenance may thus immediately expire
> +	# reflogs if it was running.
> +	git config set gc.reflogExpire never &&
> +	git config set gc.reflogExpireUnreachable never &&
> +
>   	echo file >file &&
>   	git add file &&
>   	git commit -a -m original