Re: [PATCH v2 3/3] sequencer: keep auto maintenance out of the commands a sequence spawns

From: Thomas Bachem <mail@thomasbachem.com>
Date: Sep 7, 2026, 16:37 UTC
Message-ID: <CAA0xjtoUBHcEJA6_EGgiy2eXghb3duMpTi=KBkxt08fV0c5Arw@mail.gmail.com>
In-Reply-To: <7493f0b7-a6cb-4b7d-bfd4-f4a318ff7e32@gmail.com>

Hi Phillip,

On 07/09/2026 15:24, Phillip Wood wrote:

Show 7 quoted lines

> I don't think maintenance is actively working against other commands, it
> just creates lock contention. Maybe something like
>
>      When the sequencer runs "git commit" or "git merge", either directly
>      or via a user supplied exec command, those commands run "git
>      maintenance --auto --detach" which can cause lock contention with
>      the sequencer.

I'll use that. The repack case is a bit different, though. It can delete a pack the sequencer still has open, which 65cda10d5b had to work around, so I'll keep one sentence on it.

> This is pretty hard to understand. What does 'the commit of one "git
> rebase --continue"' mean? Also whether the next pick needs to take
> MERGE_RR.lock is conditional on there being conflicts which isn't at all
> clear.

I meant the "git commit" that "git rebase --continue" spawns for a resolved conflict. Its maintenance run can still hold MERGE_RR.lock when the next pick conflicts and rerere needs it. I'll write it like that.

> What does that mean?

Once the spawned commands no longer run maintenance, a long sequence can pile up loose objects, and nothing packs them before the run at the end. I don't know whether a sequence can get long enough for that to matter. I'll say it like this, or drop it.

> Talking about the shell here is unnecessarily confusing as the command
> is not necessarily run by the shell: if it is a single word that does
> not contain any shell metacharacters it is passed directly to exec()

Right, the environment reaches the command either way. I'll drop the shell from the message.

> This comment isn't wrong but sounds like an LLM, rather than something a
> person would write.

I've rewritten it:

/*
* Don't let the commands we spawn run auto maintenance. It would
* race us for MERGE_RR.lock or delete packs we still have open,
* so it runs once at the end of the sequence instead.
*/

> Shouldn't this just extend the test added in the previous patch, rather
> than duplicating the coverage for auto maintenance being run at the end
> of a rebase?

Yes, I'll extend both tests from the previous patch instead.

Thanks, Thomas