From: Thomas Bachem Date: Mon, 07 Sep 2026 16:37:10 GMT Subject: Re: [PATCH v2 3/3] sequencer: keep auto maintenance out of the commands a sequence spawns Message-ID: In-Reply-To: <7493f0b7-a6cb-4b7d-bfd4-f4a318ff7e32@gmail.com> 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