Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Thomas Bachem <mail@thomasbachem.com>, phillip.wood@dunelm.org.uk
Cc: ben.knoble@gmail.com, gitster@pobox.com, git@vger.kernel.org,
	eli@barzilay.org, ps@pks.im
Subject: Re: [PATCH v3 0/5] stash: clean up index-mode test merge
Date: Mon, 28 Sep 2026 16:40:02 +0100	[thread overview]
Message-ID: <ef5e507f-9e26-4e7e-887a-403cf7f282a7@gmail.com> (raw)
In-Reply-To: <CAA0xjtpzaWH10pHOQ5j-5Hp1yHEKTDFbsicG6E4w=5nxb_irWw@mail.gmail.com>

Hi Thomas

On 28/09/2026 15:50, Thomas Bachem wrote:
> On Mon, Sep 28, 2026 at 3:45 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> 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",

Thanks for tracking that down, I couldn't see where we'd be calling "git 
stash apply" with "--index" but builtin/merge.c:restore_state() calls 
"git stash apply --index --quiet" rather than calling one of the 
autostash helper functions which do not use "--index".

> 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.

That accounts for the difference in the number of reflog entries. It's 
good to have an explanation for why we're expiring the reflog entries at 
a slightly different time.

Thanks

Phillip
> 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.
> 
>> git config maintenance.reflog-expire.auto 0
>>
>> to the 'setup' test fixes the test failure, but it would be good to try
>> and understand why this topic triggers the reflog to be expired in case
>> there is something nasty happening that we've not thought of.
> 
> I'd pin the expiry itself instead, as ea7d894f44 (t34xx: don't expire
> reflogs where it matters, 2026-02-24) did for the rebase tests:
> 
> git config set gc.reflogExpire never &&
> git config set gc.reflogExpireUnreachable never &&
> 
> That covers a "git gc" as well, which expires reflogs on its own. With
> it, 'seen' plus Ben's series passes t5520 here and no expiry runs
> during the script at all. I sent it as a patch on master:
> <pull.2243.git.1790606282769.gitgitgadget@gmail.com>
> 
> FWIW, any script that reads a reflog after a hundred HEAD updates can
> fall into the same hole. I have not looked further than t5520.
> 
> Thomas


  parent reply	other threads:[~2026-09-28 15:40 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19 21:26 [PATCH 0/2] Hi all, D. Ben Knoble
2026-09-19 21:26 ` [PATCH 1/2] builtin/stash: remove unused header D. Ben Knoble
2026-09-21 15:10   ` Junio C Hamano
2026-09-19 21:26 ` [PATCH 2/2] builtin/stash: merge index in-core D. Ben Knoble
2026-09-21 13:17   ` Phillip Wood
2026-09-22 12:43     ` D. Ben Knoble
2026-09-22 12:51       ` D. Ben Knoble
2026-09-22 13:57       ` Phillip Wood
2026-09-22 20:34         ` D. Ben Knoble
2026-09-19 21:32 ` [PATCH 0/2] Hi all, D. Ben Knoble
2026-09-23 12:58 ` [PATCH v2 0/4] stash: clean up index-mode test merge D. Ben Knoble
2026-09-23 12:58   ` [PATCH v2 1/4] builtin/stash: remove unused header D. Ben Knoble
2026-09-23 12:58   ` [PATCH v2 2/4] stash: prepare merge options earlier D. Ben Knoble
2026-09-23 12:58   ` [PATCH v2 3/4] t: test failed "stash apply --index" D. Ben Knoble
2026-09-24  9:42     ` Phillip Wood
2026-09-25 13:36       ` D. Ben Knoble
2026-09-25 15:45         ` Phillip Wood
2026-09-26  9:53           ` Phillip Wood
2026-09-26 12:07             ` D. Ben Knoble
2026-09-23 12:58   ` [PATCH v2 4/4] builtin/stash: merge index in-core D. Ben Knoble
2026-09-24  9:42     ` Phillip Wood
2026-09-25 12:55       ` D. Ben Knoble
2026-09-25 15:58         ` Phillip Wood
2026-09-25 16:16           ` D. Ben Knoble
2026-09-24 21:59     ` Junio C Hamano
2026-09-25  4:12       ` Junio C Hamano
2026-09-25 13:00       ` D. Ben Knoble
2026-09-25 16:24         ` Junio C Hamano
2026-09-26  9:51           ` Phillip Wood
2026-09-26 12:04             ` D. Ben Knoble
2026-09-25 16:04       ` Phillip Wood
2026-09-25 16:17         ` D. Ben Knoble
2026-09-25 16:49           ` Junio C Hamano
2026-09-26 12:16   ` [PATCH v3 0/5] stash: clean up index-mode test merge D. Ben Knoble
2026-09-26 12:16     ` [PATCH v3 1/5] builtin/stash: remove unused header D. Ben Knoble
2026-09-26 12:16     ` [PATCH v3 2/5] stash: prepare merge options earlier D. Ben Knoble
2026-09-26 12:16     ` [PATCH v3 3/5] t3903: test stash --index merges D. Ben Knoble
2026-09-28 15:44       ` Phillip Wood
2026-09-28 15:55         ` D. Ben Knoble
2026-09-29  9:41           ` Phillip Wood
2026-09-26 12:16     ` [PATCH v3 4/5] t3903: test failed "stash apply --index" D. Ben Knoble
2026-09-26 12:16     ` [PATCH v3 5/5] builtin/stash: merge index in-core D. Ben Knoble
2026-09-27 18:59       ` Junio C Hamano
2026-09-28 12:02         ` D. Ben Knoble
2026-09-28  9:40       ` Junio C Hamano
2026-09-28 12:03         ` D. Ben Knoble
2026-09-28 15:32           ` Junio C Hamano
2026-09-26 12:20     ` [PATCH v3 0/5] stash: clean up index-mode test merge D. Ben Knoble
2026-09-27 19:21     ` Junio C Hamano
2026-09-28  9:50       ` Phillip Wood
2026-09-28 12:05         ` D. Ben Knoble
2026-09-28 12:33           ` D. Ben Knoble
2026-09-28 13:00             ` D. Ben Knoble
2026-09-28 13:45               ` Phillip Wood
2026-09-28 14:50                 ` Thomas Bachem
2026-09-28 15:36                   ` D. Ben Knoble
2026-09-29 11:38                     ` D. Ben Knoble
2026-09-29 15:54                     ` Phillip Wood
2026-09-28 15:40                   ` Phillip Wood [this message]
2026-09-29 12:18 ` [PATCH v4 " D. Ben Knoble
2026-09-29 12:18   ` [PATCH v4 1/5] builtin/stash: remove unused header D. Ben Knoble
2026-09-29 12:18   ` [PATCH v4 2/5] stash: prepare merge options earlier D. Ben Knoble
2026-09-29 12:18   ` [PATCH v4 3/5] t3903: test failed "stash apply --index" D. Ben Knoble
2026-09-29 12:18   ` [PATCH v4 4/5] t5520: don't expire reflogs where it matters D. Ben Knoble
2026-09-29 15:46     ` Phillip Wood
2026-09-29 12:18   ` [PATCH v4 5/5] builtin/stash: merge index in-core D. Ben Knoble
2026-09-29 20:07     ` Junio C Hamano
2026-09-30  1:29       ` D. Ben Knoble
2026-09-29 15:48   ` [PATCH v4 0/5] stash: clean up index-mode test merge Phillip Wood
2026-09-29 17:31     ` Ben Knoble
2026-09-30 21:26       ` D. Ben Knoble
2026-09-30 21:24 ` [PATCH v5 0/4] " D. Ben Knoble
2026-09-30 21:24   ` [PATCH v5 1/4] builtin/stash: remove unused header D. Ben Knoble
2026-09-30 21:24   ` [PATCH v5 2/4] stash: prepare merge options earlier D. Ben Knoble
2026-09-30 21:24   ` [PATCH v5 3/4] t3903: test failed "stash apply --index" D. Ben Knoble
2026-09-30 21:24   ` [PATCH v5 4/4] builtin/stash: merge index in-core D. Ben Knoble
2026-10-01 15:52   ` [PATCH v5 0/4] stash: clean up index-mode test merge Phillip Wood
2026-10-01 17:47     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ef5e507f-9e26-4e7e-887a-403cf7f282a7@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=ben.knoble@gmail.com \
    --cc=eli@barzilay.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mail@thomasbachem.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox