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

On 04/09/2026 16:51, Thomas Bachem via GitGitGadget wrote:

From: Thomas Bachem <redacted>

The "git commit" and "git merge" the sequencer spawns, and the git
commands an exec runs, each start "git maintenance run --auto
--detach", which then works in the background against the sequence
itself. 

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.

A "rerere gc" started by the commit of one "git rebase
--continue" holds MERGE_RR.lock when the next pick needs it, and a
repack deletes packs the sequencer still has open, which 65cda10d5b
(sequencer: release the ODB before spawning git commit, 2026-08-12)
had to work around.

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.

The loose objects a sequence creates wait for the run at its end that
the previous commit added.

What does that mean?

Whether a sequence can be long enough to
suffer from them before that remains to be seen. Pass
maintenance.auto=false and gc.auto=0 to the spawned commands through
GIT_CONFIG_PARAMETERS, which the shell of an exec command hands on to
whatever it runs,

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()

appended after the user's own -c settings so that
ours win, and built once per run. A command the user runs while the
sequence is stopped, like "git commit --amend" at an edit, is not the
sequencer's to control and still runs maintenance.

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

+static void disable_auto_maintenance(struct replay_opts *opts,
+				     struct child_process *cmd)
+{
+	struct strbuf *params = &opts->ctx->config_parameters;
+
+	if (!params->len) {
+		const char *old = getenv(CONFIG_DATA_ENVIRONMENT);
+
+		if (old && *old)
+			strbuf_addstr(params, old);
+		git_config_append_parameter(params, "maintenance.auto", "false");
+		git_config_append_parameter(params, "gc.auto", "0");

This is much nicer now we have the helper function and the calls to 
disable_auto_maintenance() that I've trimmed all look good.

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?

Again why do we need a separate test, rather than extending the one 
we've just added in the previous commit?

Thanks

Phillip

+'
+
  test_done