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

Hi Phillip,

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

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