Re: [PATCH v3 0/5] stash: clean up index-mode test merge

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Ben

On 28/09/2026 16:36, D. Ben Knoble wrote:
> Let me see if I understand correctly…
> 
> On Mon, Sep 28, 2026 at 10:50 AM Thomas Bachem <mail@thomasbachem.com> wrote:
>>
>> On Mon, Sep 28, 2026 at 3:45 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>> Oh, when I was thinking about this over lunch I did wonder if that might
>>> be the culprit. Previously we didn't run "git maintenance --auto" after
>>> a rebase with the 'merge' backend but with that topic we do, and because
>>> we set GIT_COMMITTER_DATE to sometime in 2005, if 'git reflog expire'
>>> gets triggered it will expire the reflog entries that 'git pull
>>> --rebase' relies on. As you suggested in another mail, I assume this
> 
>> "git pull --rebase" computes the fork point before it fetches, from
>> the reflog of refs/remotes/me/copy,
> 
> This is described by the manual for git-rebase under --fork-point,
> which is on unless we have an <upstream> or --keep-base (modulo
> config). Put a pin in this.
> 
>> and test 69 needs the entry that
>> test 68's fetch wrote there, copy-orig (f29aa66) to ae98574. With the
>> reflog empty, "merge-base --fork-point" falls back to the ref itself,
>> ae98574 is no ancestor of to-rebase, and pull hands the merge head to
>> rebase as the upstream. That is your "--onto ae98... ae98...", and the
>> four commits from copy-orig up come back, the first of them
>> conflicting with "conflict".
>>
>>> topic has changed something in one of the '--autostash' tests that come
>>> before the failing test triggers which the new behavior. What that
>>> something is I'm not sure; off the top of my head I'd expect the number
>>> of reflog entries in HEAD to be the same but maybe I'm missing
>>> something. Adding
>>
>> It is eight entries fewer, and they come from the failed merges, not
>> from the autostash tests. "git merge" restores a dirty tree with
>> "stash apply --index --quiet", and until Ben's series that spawned
>> "git reset --quiet --refresh", which writes "reset: moving to HEAD"
>> to the reflog. That happens eight times in t5520 before test 68.
>>
>> Auto maintenance expires reflogs once HEAD's reflog holds a hundred
>> entries that the policy would remove, the default of
>> maintenance.reflog-expire.auto, and after the first test_tick that is
>> every entry. Which run crosses the hundred depends on how many entries
>> and maintenance runs came before it. On 'seen' the expiry lands on
>> "git commit -m conflict" in test 68, before the fetch writes the entry.
>> Eight entries fewer move the crossing past that commit, and the
>> maintenance run my topic adds at the end of the rebase in test 68 is
>> the next one: after the fetch, before test 69 reads the reflog. Either
>> change alone leaves it somewhere harmless, and nothing else is going
>> on. The expiry is the usual 90 days applied to entries dated 2005, and
>> the only new thing is one more maintenance run per rebase, the same
>> one "git commit" and "git fetch" run.
> 
> In short, expiry used to happen prior to .68, so the reflog entry
> created in that test which is used by "pull --rebase" in .69 is picked
> up. With fewer reflog entries, expiry happens later, and it just so
> happens to drop the important entry. Darn!

Yes, it is incredibly bad luck that the test broke, though I guess it is 
also fortunate as it means we can fix the latent bug in the test.

> 
> But here's what I can't figure out, returning to that pin from
> earlier: I was a bit surprised to see mention of rebase reading
> reflogs! When I remembered --fork-point, I was even more curious (but
> at least it's obvious that rebase will read the reflogs in some
> scenarios).
> 
> What confuses me is that builtin/pull.c:run_rebase() sure looks like
> it provides an <upstream> to the command invocation, so shouldn't
> --fork-point and reflog use be disabled????
> 
> I'll try tracing that test myself later, I suppose. It's nice to know
> we have a fix available (thanks for the patch), but it sure feels like
> a hack :) oh well?

It is a bit confusing that "git pull --rebase" does not use "git rebase 
--fork-point", instead it calls "git merge-base --fork-point" (which is 
where we read the reflog of the remote branch) itself and then passes 
that as the upstream revision to "git rebase". I think this is because 
fork-point handling was added to "git pull" before "--fork-point" 
existed in "git rebase". "git rebase" only looks for a fork-point if its 
upstream argument is a ref, so as "git pull" passes an object id, the 
fork-point detection in rebase is bypassed.

Thanks

Phillip