Re: [PATCH 2/2] sequencer: keep auto maintenance out of the commands a rebase spawns

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Thomas

On 04/09/2026 08:53, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
> 
> The commands a rebase with the merge backend spawns, the "git commit"
> for a resolved, reworded or squashed pick, the "git merge" of a
> "rebase -r" for an octopus merge or with a strategy, and whatever an
> exec command runs, each kick off "git maintenance run --auto --detach",
> a background process the rebase then races for the repository: the
> "rerere gc" spawned by the commit of one "git rebase --continue" holds
> MERGE_RR.lock while the next pick wants it, and a repack wants to
> delete packs the sequencer still had open, which 65cda10d5b
> (sequencer: release the ODB before spawning git commit, 2026-08-12)
> had to fix for Windows.

This entire paragraph is a single sentence and is very hard to understand.
> Nothing a rebase creates is old enough to be pruned by the time it

s/Nothing/The objects/?

> ends, and repacking what it created can wait until then,

That's what we'll find out when this is merged. In principle it is 
possible someone is doing enormous rebases where the number of loose 
objects impacts the performance if we don't repack mid-rebase but I 
don't think we can know that without disabling auto maintenance and 
seeing if anyone complians.

> so
> maintenance in the middle of a rebase has nothing to do that a run at
> its end cannot, and a rebase to get in the way of.

That last clause is hard to parse.

> Pass
> maintenance.auto=false and gc.auto=0 to the commands a rebase spawns,
> through GIT_CONFIG_PARAMETERS so that the shell of an exec command
> passes them on too, appended to whatever -c the user gave, since the
> last entry wins. What the user runs while the rebase is stopped, say
> "git commit --amend" at an edit, is not the rebase's to control and
> still runs it. 

s/runs/run/

> "git commit" and "git merge" could skip it themselves
> while a rebase is in progress, which would cover that too, but that
> spreads the rebase's business over every command that runs
> maintenance and defers theirs for as long as a rebase is left lying
> around, so keep the decision with the rebase, in what it spawns. Both
> backends run maintenance once the rebase is done, the merge backend
> since the previous commit, so nothing is lost.
> 
> Cherry-pick and revert are left alone: they never ran maintenance at
> the end of a sequence, and the "git commit" they spawn for a
> --continue or an edited message is the only place they run it at all.

I'm inclined to think that the reasoning for running maintenance at the 
end of a rebase applies to cherry-pick and probably revert as well.

> 
> Assisted-by: Claude Fable 5.1
> Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
> ---
>   sequencer.c                | 27 +++++++++++++++++++++++++++
>   t/t3418-rebase-continue.sh | 18 ++++++++++++++++++
>   2 files changed, 45 insertions(+)
> 
> diff --git a/sequencer.c b/sequencer.c
> index f58ad254be..30c1a799cc 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1107,6 +1107,29 @@ static int run_command_silent_on_success(struct child_process *cmd)
>   	return rc;
>   }
>   
> +/*
> + * A rebase runs auto maintenance once it is done, not from every command
> + * it spawns along the way: their background "rerere gc" or repack would
> + * race the rebase for locks and files it still holds.
> + */
> +static void disable_auto_maintenance(struct child_process *cmd)
> +{
> +	struct strbuf value = STRBUF_INIT;
> +	const char *old = getenv(CONFIG_DATA_ENVIRONMENT);
> +
> +	if (old && *old)
> +		strbuf_addf(&value, "%s ", old);
> +	sq_quote_buf(&value, "maintenance.auto");
> +	strbuf_addch(&value, '=');
> +	sq_quote_buf(&value, "false");
> +	strbuf_addch(&value, ' ');
> +	sq_quote_buf(&value, "gc.auto");
> +	strbuf_addch(&value, '=');
> +	sq_quote_buf(&value, "0");
> +	strvec_pushf(&cmd->env, "%s=%s", CONFIG_DATA_ENVIRONMENT, value.buf);
> +	strbuf_release(&value);
> +}

We already have a function in config.c to append parameters, but it sets 
them in the callers environment which we don't want to do here. I wonder 
if we could factor out a helper append the parameters to an strbuf 
passed by the caller so we don't need to know about the quoting scheme 
here. Also it would be nice to cache this in replay_ctx so we don't have 
to construct the string each time we want to disable auto maintenance.

Other than that this looks good

Thanks

Phillip


> +
>   /*
>    * If we are cherry-pick, and if the merge did not result in
>    * hand-editing, we will hit this commit and inherit the original
> @@ -1148,6 +1171,8 @@ static int run_git_commit(const char *defmsg,
>   			     author_date_from_env(&cmd.env));
>   	if (opts->ignore_date)
>   		strvec_push(&cmd.env, "GIT_AUTHOR_DATE=");
> +	if (is_rebase_i(opts))
> +		disable_auto_maintenance(&cmd);
>   
>   	strvec_push(&cmd.args, "commit");
>   
> @@ -3934,6 +3959,7 @@ static int do_exec(struct repository *r, const char *command_line, int quiet)
>   	cmd.use_shell = 1;
>   	strvec_push(&cmd.args, command_line);
>   	strvec_push(&cmd.env, "GIT_CHERRY_PICK_HELP");
> +	disable_auto_maintenance(&cmd);
>   	status = run_command(&cmd);
>   
>   	/* force re-reading of the cache */
> @@ -4342,6 +4368,7 @@ static int do_merge(struct repository *r,
>   				     author_date_from_env(&cmd.env));
>   		if (opts->ignore_date)
>   			strvec_push(&cmd.env, "GIT_AUTHOR_DATE=");
> +		disable_auto_maintenance(&cmd);
>   
>   		cmd.git_cmd = 1;
>   		strvec_push(&cmd.args, "merge");
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index 2c34cf8a01..cf6d20ce79 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -403,4 +403,22 @@ test_expect_success 'rebase runs auto maintenance at its end' '
>   	test_subcommand_flex git maintenance run --auto <finish.txt
>   '
>   
> +test_expect_success 'rebase spawns no auto maintenance before its end' '
> +	git checkout -b two-conflicts topic &&
> +	test_commit F2-again F2 222 &&
> +	test_must_fail git rebase -x "git commit --allow-empty -m exec" main &&
> +	echo resolved >F2 &&
> +	git add F2 &&
> +	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/mid.txt" \
> +		git rebase --continue &&
> +	test_subcommand_flex git commit <mid.txt &&
> +	test_subcommand_flex ! git maintenance run --auto <mid.txt &&
> +	echo resolved >F2 &&
> +	git add F2 &&
> +	GIT_TRACE2_EVENT="$(pwd)/end.txt" git rebase --continue &&
> +	test_subcommand_flex git maintenance run --auto <end.txt &&
> +	grep "\"child_start\".*\"maintenance\"" end.txt >maintenance &&
> +	test_line_count = 1 maintenance
> +'
> +
>   test_done