Skip to content

sequencer: leave auto maintenance to the end of a sequence #2217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thomasbachem wants to merge 3 commits into
base: master
Choose a base branch
from
Open

sequencer: leave auto maintenance to the end of a sequence #2217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef8087e
config: add git_config_append_parameter()
thomasbachem Sep 4, 2026
baab8d4
sequencer: run auto maintenance once a sequence is done
thomasbachem Sep 4, 2026
9a6fc04
sequencer: keep auto maintenance out of the commands a sequence spawns
thomasbachem Sep 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,18 +450,24 @@ static int git_config_include(const char *var, const char *value,
return ret;
}

void git_config_append_parameter(struct strbuf *env, const char *key,
const char *value)
{
if (env->len)
strbuf_addch(env, ' ');
sq_quote_buf(env, key);
strbuf_addch(env, '=');
if (value)
sq_quote_buf(env, value);
}

static void git_config_push_split_parameter(const char *key, const char *value)
{
struct strbuf env = STRBUF_INIT;
const char *old = getenv(CONFIG_DATA_ENVIRONMENT);
if (old && *old) {
if (old && *old)
strbuf_addstr(&env, old);
strbuf_addch(&env, ' ');
}
sq_quote_buf(&env, key);
strbuf_addch(&env, '=');
if (value)
sq_quote_buf(&env, value);
git_config_append_parameter(&env, key, value);
setenv(CONFIG_DATA_ENVIRONMENT, env.buf, 1);
strbuf_release(&env);
}
Expand Down
10 changes: 10 additions & 0 deletions config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*/

struct object_id;
struct strbuf;

/* git_config_parse_key() returns these negated: */
#define CONFIG_INVALID_KEY 1
Expand Down Expand Up @@ -186,6 +187,15 @@ int git_config_from_blob_oid(config_fn_t fn, const char *name,
enum config_scope scope);
void git_config_push_parameter(const char *text);
void git_config_push_env(const char *spec);

/*
* Append `key=value` to the GIT_CONFIG_PARAMETERS value in `env`, quoted
* the way git_config_from_parameters() reads it, so that a child can be
* given configuration on top of what this process was given. A NULL
* `value` appends a boolean entry.
*/
void git_config_append_parameter(struct strbuf *env, const char *key,
const char *value);
int git_config_from_parameters(config_fn_t fn, void *data);

/*
Expand Down
57 changes: 51 additions & 6 deletions sequencer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@ struct replay_ctx {
* Whether message contains a commit message.

@gitgitgadget gitgitgadget Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Thomas Bachem via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +	/*
> +	 * The GIT_CONFIG_PARAMETERS value that keeps auto maintenance out
> +	 * of the commands we spawn, built on first use.
> +	 */
> +	struct strbuf config_parameters;

Does this have to be a "struct strbuf", not "const char *"?  The
latter makes it clear that it will never change its value once you
built it in disable_auto_maintenance().

> +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");
> +	}
> +	strvec_pushf(&cmd->env, "%s=%s", CONFIG_DATA_ENVIRONMENT, params->buf);
> +}

This would then become something like

	if (!opts->ctx->config_parameters) {
		const char *old = getenv(CONFIG_DATA_ENVIRONMENT);
		struct strbuf params = STRBUF_INIT;

                if (old && *old)
			strbuf_addstr(&params, old);
		git_config_append_parameter(&params, "maintenance.auto", "0");
		git_config_append_parameter(&params, "gc.auto", "0");
		opts->ctx->config_parameters = strbuf_detach(&params, NULL);
	}
	strbuf_pushf(..., opts->ctx->config_parameters);

@gitgitgadget gitgitgadget Bot Sep 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thomas Bachem wrote on the Git mailing list (how to reply to this email):

Hi Junio,

On 04/09/2026 23:21, Junio C Hamano wrote:
> Does this have to be a "struct strbuf", not "const char *"?  The
> latter makes it clear that it will never change its value once you
> built it in disable_auto_maintenance().

It doesn't. I'll make it a char * that replay_ctx_release() frees,
built the way you sketch, in the next version.

Thanks,
Thomas

*/
unsigned have_message :1;
/*
* The GIT_CONFIG_PARAMETERS value that keeps auto maintenance out
* of the commands we spawn, built on first use.
*/
struct strbuf config_parameters;
};

struct replay_ctx* replay_ctx_new(void)
Expand All @@ -242,6 +247,7 @@ struct replay_ctx* replay_ctx_new(void)

strbuf_init(&ctx->current_fixups, 0);
strbuf_init(&ctx->message, 0);
strbuf_init(&ctx->config_parameters, 0);

return ctx;
}
Expand Down Expand Up @@ -407,6 +413,7 @@ static void replay_ctx_release(struct replay_ctx *ctx)
{
strbuf_release(&ctx->current_fixups);
strbuf_release(&ctx->message);
strbuf_release(&ctx->config_parameters);
}

void replay_opts_release(struct replay_opts *opts)
Expand Down Expand Up @@ -1107,6 +1114,27 @@ static int run_command_silent_on_success(struct child_process *cmd)
return rc;

@gitgitgadget gitgitgadget Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phillip Wood wrote on the Git mailing list (how to reply to this email):

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

@gitgitgadget gitgitgadget Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thomas Bachem wrote on the Git mailing list (how to reply to this email):

Hi Phillip,

On 04/09/2026 16:03, Phillip Wood wrote:
> That's what we'll find out when this is merged.

Agreed, and the message now says so instead of claiming it.

> 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.

Agreed. In v2 all three end with one run where the sequencer finishes,
and none of the commands they spawn run it, the "git commit" of a
"cherry-pick --continue" included.

> 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.

Done, as a small config.c patch in front of the two, and a strbuf in
replay_ctx built on first use.

The messages are rewritten from scratch and much shorter, the sentence
you could not parse included.

Thanks,
Thomas

}

/*
* A sequence 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 sequencer for locks and files it still holds.
*/
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");
}
strvec_pushf(&cmd->env, "%s=%s", CONFIG_DATA_ENVIRONMENT, params->buf);
}

/*
* If we are cherry-pick, and if the merge did not result in
* hand-editing, we will hit this commit and inherit the original
Expand Down Expand Up @@ -1148,6 +1176,7 @@ 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=");
disable_auto_maintenance(opts, &cmd);

strvec_push(&cmd.args, "commit");

Expand Down Expand Up @@ -3924,16 +3953,18 @@ static int error_failed_squash(struct repository *r,
return error_with_patch(r, commit, subject, subject_len, opts, 1, 1);
}

static int do_exec(struct repository *r, const char *command_line, int quiet)
static int do_exec(struct repository *r, const char *command_line,
struct replay_opts *opts)
{
struct child_process cmd = CHILD_PROCESS_INIT;
int dirty, status;

if (!quiet)
if (!opts->quiet)
fprintf(stderr, _("Executing: %s\n"), command_line);
cmd.use_shell = 1;
strvec_push(&cmd.args, command_line);
strvec_push(&cmd.env, "GIT_CHERRY_PICK_HELP");
disable_auto_maintenance(opts, &cmd);
status = run_command(&cmd);

/* force re-reading of the cache */
Expand Down Expand Up @@ -4342,6 +4373,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(opts, &cmd);

cmd.git_cmd = 1;
strvec_push(&cmd.args, "merge");
Expand Down Expand Up @@ -5158,7 +5190,7 @@ static int pick_commits(struct repository *r,
if (!opts->verbose)
term_clear_line();
*end_of_arg = '\0';
res = do_exec(r, arg, opts->quiet);
res = do_exec(r, arg, opts);
*end_of_arg = saved;

if (res) {
Expand Down Expand Up @@ -5313,6 +5345,12 @@ static int pick_commits(struct repository *r,
return -1;
}

/*
* We ignore errors in 'git maintenance run --auto', since the
* user should see them.
*/
run_auto_maintenance(r, opts->quiet);

/*
* Sequence of picks finished successfully; cleanup by
* removing the .git/sequencer directory
Expand All @@ -5329,6 +5367,7 @@ static int continue_single_pick(struct repository *r, struct replay_opts *opts)
return error(_("no cherry-pick or revert in progress"));

cmd.git_cmd = 1;
disable_auto_maintenance(opts, &cmd);
strvec_push(&cmd.args, "commit");

/*
Expand Down Expand Up @@ -5577,10 +5616,14 @@ int sequencer_continue(struct repository *r, struct replay_opts *opts)
res = -1;
goto release_todo_list;
}
} else if (!file_exists(get_todo_path(opts)))
return continue_single_pick(r, opts);
else if ((res = read_populate_todo(r, &todo_list, opts)))
} else if (!file_exists(get_todo_path(opts))) {
res = continue_single_pick(r, opts);
if (!res)
run_auto_maintenance(r, opts->quiet);
return res;
} else if ((res = read_populate_todo(r, &todo_list, opts))) {
goto release_todo_list;
}

if (!is_rebase_i(opts)) {
/* Verify that the conflict has been resolved */
Expand Down Expand Up @@ -5698,6 +5741,8 @@ int sequencer_pick_revisions(struct repository *r,
BUG("unexpected extra commit from walk");

res = single_pick(r, cmit, opts);
if (!res)
run_auto_maintenance(r, opts->quiet);
goto out;
}

Expand Down
26 changes: 26 additions & 0 deletions t/t3418-rebase-continue.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,4 +395,30 @@ test_orig_head () {
test_orig_head --apply
test_orig_head --merge

test_expect_success 'rebase runs auto maintenance at its end' '
git checkout -b one-exec main^ &&
test_commit F4 &&
test_must_fail git rebase -x false main &&
GIT_TRACE2_EVENT="$(pwd)/finish.txt" git rebase --continue &&
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
27 changes: 27 additions & 0 deletions t/t3510-cherry-pick-sequence.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,4 +721,31 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
test_line_count = 4 commits
'

test_expect_success 'cherry-pick runs auto maintenance once it is done' '
pristine_detach base &&
GIT_TRACE2_EVENT="$(pwd)/single.txt" git cherry-pick picked &&
test_subcommand_flex git maintenance run --auto <single.txt &&
GIT_TRACE2_EVENT="$(pwd)/sequence.txt" \
git cherry-pick anotherpick yetanotherpick &&
grep "\"child_start\".*\"maintenance\"" sequence.txt >maintenance &&
test_line_count = 1 maintenance
'

test_expect_success 'cherry-pick spawns no auto maintenance before it is done' '
pristine_detach initial &&
test_must_fail git cherry-pick base..anotherpick &&
echo resolved >foo &&
git add foo &&
test_must_fail env GIT_TRACE2_EVENT="$(pwd)/mid.txt" \
git cherry-pick --continue &&
test_subcommand_flex git commit <mid.txt &&
test_subcommand_flex ! git maintenance run --auto <mid.txt &&
echo d >foo &&
git add foo &&
GIT_TRACE2_EVENT="$(pwd)/end.txt" git cherry-pick --continue &&
test_subcommand_flex git commit <end.txt &&
grep "\"child_start\".*\"maintenance\"" end.txt >maintenance &&
test_line_count = 1 maintenance
'

test_done
Loading