[PATCH v3 0/3] sequencer: leave auto maintenance to the end of a sequence
From: Thomas Bachem via GitGitGadget <hidden>
Date: 2026-09-08 10:28:09
Changes since v2: * Auto maintenance now runs from builtin/rebase.c and builtin/revert.c, once the command is done, instead of from three places inside the sequencer. That follows the apply backend, where "git am" leaves it to rebase.c (Patrick, Phillip). * gc.auto=0 dropped, maintenance.auto=false is enough (Patrick). * config_parameters is a char * built once with strbuf_detach() (Junio). * Patch 3 extends the tests of patch 2 instead of adding its own. They also assert that nothing runs before a stop, and cover a single pick with --edit and a sequence ending in --skip (Phillip). * Commit messages rewritten: what GIT_CONFIG_PARAMETERS is and looks like (Patrick), what patch 2 changes for the user and why the call moved (Patrick, Phillip), and the comment on the helper (Phillip). Based on master. Independent of the rerere lock fix in [1]. [1] [ref] Thomas Bachem (3): config: add git_config_append_parameter() rebase, cherry-pick, revert: run auto maintenance when done sequencer: disable auto maintenance in spawned commands builtin/rebase.c | 13 ++++++++--- builtin/revert.c | 19 +++++++++++------ config.c | 20 +++++++++++------ config.h | 13 +++++++++++ sequencer.c | 38 ++++++++++++++++++++++++++++++--- t/t3418-rebase-continue.sh | 17 +++++++++++++++ t/t3510-cherry-pick-sequence.sh | 31 +++++++++++++++++++++++++++ 7 files changed, 131 insertions(+), 20 deletions(-) base-commit: 3cb9185f65410273787f74333cc027d2ea5daada Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2217%2Fthomasbachem%2Frebase-auto-maintenance-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2217/thomasbachem/rebase-auto-maintenance-v3 Pull-Request: https://github.com/gitgitgadget/git/pull/2217 Range-diff vs v2: 1: ef8087e80d ! 1: 70db5ad084 config: add git_config_append_parameter() @@ Metadata ## Commit message ## config: add git_config_append_parameter() - Split the part of git_config_push_split_parameter() that formats one - GIT_CONFIG_PARAMETERS entry into a helper that appends it to a strbuf, - so that a caller can build a value for a child's environment without - knowing the quoting. The sequencer is about to do that. + "git -c key=value" passes its settings on to the git commands it + spawns through the environment variable GIT_CONFIG_PARAMETERS. The + value is a space separated list of 'key'='value' pairs with both + sides single quoted, which git_config_from_parameters() reads back in + the child. The only place we write such an entry is + git_config_push_split_parameter(), and it writes straight into our + own environment. + + Split the formatting out into git_config_append_parameter(), which + appends one entry to a strbuf, so that we can build such a value for + a child's environment without repeating the quoting. The sequencer + will use it in a later commit to pass settings to the commands it + spawns. Assisted-by: Claude Fable 5.1 Signed-off-by: Thomas Bachem [off-list ref] @@ config.h: int git_config_from_blob_oid(config_fn_t fn, const char *name, 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. ++ * Append a "-c key=value" setting to a GIT_CONFIG_PARAMETERS value in ++ * `env`. The variable carries such settings from a git process to the ++ * git commands it spawns, as a space separated list of 'key'='value' ++ * pairs with both sides single quoted, which git_config_from_parameters() ++ * reads back. A NULL `value` appends 'key'= with nothing after the equals ++ * sign, which stands for a boolean true, like "-c key" on the command ++ * line. + */ +void git_config_append_parameter(struct strbuf *env, const char *key, + const char *value); 2: baab8d4876 ! 2: 68a728c5f4 sequencer: run auto maintenance once a sequence is done @@ Metadata Author: Thomas Bachem [off-list ref] ## Commit message ## - sequencer: run auto maintenance once a sequence is done + rebase, cherry-pick, revert: run auto maintenance when done - The apply backend of "git rebase" runs "git maintenance run --auto" - from finish_rebase() once it has applied its patches. The merge - backend, "git cherry-pick" and "git revert" do not run it when they - finish. They create their commits in process, and only the "git - commit" they spawn for an edited message or a resolved conflict, the - "git merge" a "rebase -r" spawns and an exec command start it, in the - middle of the sequence. + "git commit", "git merge", "git fetch" and "git am" run "git + maintenance run --auto" when they are done, and so does the apply + backend of "git rebase". That repacks the loose objects they wrote + once there are enough of them, expires old rerere entries and does + whatever other housekeeping is due. - Run it where the sequencer finishes, so that every sequence ends the - way the apply backend does, and so that the next commit can keep it - out of the commands a sequence spawns. + The merge backend of "git rebase", "git cherry-pick" and "git revert" + do not. They create their commits in process, so auto maintenance + runs only when they spawn a command that runs it on its own. That is + the "git commit" for a resolved conflict or an edited message, the + "git merge" that "--rebase-merges" spawns for an octopus merge, a + strategy other than ort or any strategy option, and whatever an exec + runs. A sequence that needs none of these never runs auto + maintenance. One that stops for conflicts runs it after each + resolution, in the middle of the sequence. + + Run it once when the sequence is done, like the apply backend does. + + The apply backend leaves that to builtin/rebase.c: "git am" skips + auto maintenance in rebasing mode, and finish_rebase() runs it once + the patches are applied. Do the same for the sequencer, from + builtin/rebase.c and builtin/revert.c, because the sequencer itself + has no single place where every sequence ends. A sequence of several + commits ends inside pick_commits(). A single cherry-pick or revert + never creates the sequencer's state directory and returns to its + caller as soon as its commit is made. "--continue" and "--skip" have + entry points of their own. Nothing but those two builtins starts or + continues a sequence, so that is where we run auto maintenance. + run_specific_rebase() runs it for the merge backend once the + sequencer has returned successfully and removed its state directory, + which it keeps while the rebase is stopped. run_sequencer() runs it + for cherry-pick and revert when a pick, a "--continue" or a "--skip" + returns successfully. + + For the user, a sequence that never stops now runs auto maintenance + once when it is done, where it never ran it before. That is the same + "git maintenance run --auto --detach" as after "git commit": it + detaches into the background by default and does nothing unless one + of its tasks is due. The runs from the commands a sequence spawns + stay for now. The next commit removes them, so that a sequence runs + auto maintenance exactly once. Assisted-by: Claude Fable 5.1 Signed-off-by: Thomas Bachem [off-list ref] - ## sequencer.c ## -@@ sequencer.c: cleanup_head_ref: - return -1; - } + ## builtin/rebase.c ## +@@ builtin/rebase.c: static int run_specific_rebase(struct rebase_options *opts) -+ /* -+ * We ignore errors in 'git maintenance run --auto', since the -+ * user should see them. -+ */ -+ run_auto_maintenance(r, opts->quiet); + if (opts->dont_finish_rebase) + ; /* do nothing */ +- else if (opts->type == REBASE_MERGE) +- ; /* merge backend cleans up after itself */ +- else if (status == 0) { ++ else if (opts->type == REBASE_MERGE) { ++ int quiet = !(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)); + - /* - * Sequence of picks finished successfully; cleanup by - * removing the .git/sequencer directory -@@ sequencer.c: 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; -+ } ++ /* ++ * The sequencer cleans up after itself. Its state directory ++ * is gone once it is done, and stays while it is stopped. ++ */ ++ if (status == 0 && !is_directory(opts->state_dir)) ++ run_auto_maintenance(the_repository, quiet); ++ } else if (status == 0) { + if (!file_exists(state_dir_path("stopped-sha", opts))) + finish_rebase(opts); + } else if (status == 2) { + + ## builtin/revert.c ## +@@ + #include "gettext.h" + #include "revision.h" + #include "rerere.h" ++#include "run-command.h" + #include "sequencer.h" + #include "branch.h" - if (!is_rebase_i(opts)) { - /* Verify that the conflict has been resolved */ -@@ sequencer.c: int sequencer_pick_revisions(struct repository *r, - BUG("unexpected extra commit from walk"); +@@ builtin/revert.c: static int run_sequencer(int argc, const char **argv, const char *prefix, + const char *strategy = &sentinel_value; + const char *gpg_sign = &sentinel_value; + enum empty_action empty_opt = EMPTY_COMMIT_UNSPECIFIED; +- int cmd = 0; ++ int cmd = 0, ret; + struct option base_options[] = { + OPT_CMDMODE(0, "quit", &cmd, N_("end revert or cherry-pick sequence"), 'q'), + OPT_CMDMODE(0, "continue", &cmd, N_("resume revert or cherry-pick sequence"), 'c'), +@@ builtin/revert.c: static int run_sequencer(int argc, const char **argv, const char *prefix, + free(options); - res = single_pick(r, cmit, opts); -+ if (!res) -+ run_auto_maintenance(r, opts->quiet); - goto out; + if (cmd == 'q') { +- int ret = sequencer_remove_state(opts); ++ ret = sequencer_remove_state(opts); + if (!ret) + remove_branch_state(the_repository, 0); + return ret; } +- if (cmd == 'c') +- return sequencer_continue(the_repository, opts); + if (cmd == 'a') + return sequencer_rollback(the_repository, opts); +- if (cmd == 's') +- return sequencer_skip(the_repository, opts); +- return sequencer_pick_revisions(the_repository, opts); ++ if (cmd == 'c') ++ ret = sequencer_continue(the_repository, opts); ++ else if (cmd == 's') ++ ret = sequencer_skip(the_repository, opts); ++ else ++ ret = sequencer_pick_revisions(the_repository, opts); ++ if (!ret) ++ run_auto_maintenance(the_repository, opts->quiet); ++ return ret; + } + int cmd_revert(int argc, ## t/t3418-rebase-continue.sh ## @@ t/t3418-rebase-continue.sh: 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 runs auto maintenance once it is done' ' ++ git checkout -b auto-maintenance topic && ++ test_must_fail env GIT_TRACE2_EVENT="$(pwd)/stop.txt" \ ++ git rebase -x false main && ++ test_subcommand_flex ! git maintenance run --auto <stop.txt && ++ echo resolved >F2 && ++ git add F2 && ++ test_must_fail git rebase --continue && ++ GIT_TRACE2_EVENT="$(pwd)/end.txt" git rebase --continue && ++ test_subcommand_flex git maintenance run --auto <end.txt +' + test_done @@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'commit descriptions in ins + test_subcommand_flex git maintenance run --auto <single.txt && + GIT_TRACE2_EVENT="$(pwd)/sequence.txt" \ + git cherry-pick anotherpick yetanotherpick && ++ test_subcommand_flex git maintenance run --auto <sequence.txt && + grep "\"child_start\".*\"maintenance\"" sequence.txt >maintenance && + test_line_count = 1 maintenance +' ++ ++test_expect_success 'cherry-pick runs auto maintenance once a stopped sequence is done' ' ++ pristine_detach initial && ++ test_must_fail env GIT_TRACE2_EVENT="$(pwd)/stop.txt" \ ++ git cherry-pick base..anotherpick && ++ test_subcommand_flex ! git maintenance run --auto <stop.txt && ++ echo resolved >foo && ++ git add foo && ++ test_must_fail git cherry-pick --continue && ++ GIT_TRACE2_EVENT="$(pwd)/end.txt" git cherry-pick --skip && ++ test_subcommand_flex git maintenance run --auto <end.txt ++' + test_done 3: 9a6fc0427a ! 3: 7a353df3d9 sequencer: keep auto maintenance out of the commands a sequence spawns @@ Metadata Author: Thomas Bachem [off-list ref] ## Commit message ## - sequencer: keep auto maintenance out of the commands a sequence spawns + sequencer: disable auto maintenance in spawned commands - 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. 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. + The "git commit" and "git merge" the sequencer spawns run "git + maintenance run --auto --detach" as they finish, and so does any + such command an exec runs. That maintenance then works in the + background while the sequencer goes on with the sequence, and the + two get in each other's way. With rerere enabled, the maintenance + started by the "git commit" of a "git rebase --continue" runs + "rerere gc", which can still hold MERGE_RR.lock when the next pick + conflicts. The rebase then dies with "Unable to create + '.../MERGE_RR.lock': File exists" instead of stopping for the user + to resolve the conflict. And a repack can delete a pack the + sequencer still has open, which 65cda10d5b (sequencer: release the + ODB before spawning git commit, 2026-08-12) works around. - The loose objects a sequence creates wait for the run at its end that - the previous commit added. 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, 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. + Pass maintenance.auto=false to these commands through + GIT_CONFIG_PARAMETERS, as "git -c" would. We build the value once + from the one we inherited and append our setting after the user's + own -c settings so that it wins. The environment also reaches + everything the command spawns in turn, so a git command run from an + exec is covered as well. The sequencer also spawns "git stash", "git + reset" and "git notes", which never run auto maintenance. + + With the previous commit, rebase, cherry-pick and revert run auto + maintenance once when they are done, so a sequence now runs it + exactly once, at the end. A sequence that stops for conflicts used + to run it at every resolution and now piles up its loose objects + until the end, as a sequence without conflicts always has. + + A command the user runs while the sequence is stopped, like "git + commit --amend" at an edit, still runs auto maintenance. The + sequencer does not spawn it and has no say in it. Assisted-by: Claude Fable 5.1 Signed-off-by: Thomas Bachem [off-list ref] @@ sequencer.c: struct replay_ctx { */ unsigned have_message :1; + /* -+ * The GIT_CONFIG_PARAMETERS value that keeps auto maintenance out -+ * of the commands we spawn, built on first use. ++ * GIT_CONFIG_PARAMETERS for the commands we spawn, with auto ++ * maintenance turned off. Built on first use. + */ -+ struct strbuf config_parameters; ++ char *config_parameters; }; struct replay_ctx* replay_ctx_new(void) -@@ sequencer.c: 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; - } @@ sequencer.c: static void replay_ctx_release(struct replay_ctx *ctx) { strbuf_release(&ctx->current_fixups); strbuf_release(&ctx->message); -+ strbuf_release(&ctx->config_parameters); ++ free(ctx->config_parameters); } void replay_opts_release(struct replay_opts *opts) @@ sequencer.c: static int run_command_silent_on_success(struct child_process *cmd) } +/* -+ * 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. ++ * 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. Our caller ++ * runs it once the sequence is done. + */ +static void disable_auto_maintenance(struct replay_opts *opts, + struct child_process *cmd) +{ -+ struct strbuf *params = &opts->ctx->config_parameters; -+ -+ if (!params->len) { ++ if (!opts->ctx->config_parameters) { + const char *old = getenv(CONFIG_DATA_ENVIRONMENT); ++ struct strbuf buf = STRBUF_INIT; + + if (old && *old) -+ strbuf_addstr(params, old); -+ git_config_append_parameter(params, "maintenance.auto", "false"); -+ git_config_append_parameter(params, "gc.auto", "0"); ++ strbuf_addstr(&buf, old); ++ git_config_append_parameter(&buf, "maintenance.auto", "false"); ++ opts->ctx->config_parameters = strbuf_detach(&buf, NULL); + } -+ strvec_pushf(&cmd->env, "%s=%s", CONFIG_DATA_ENVIRONMENT, params->buf); ++ strvec_pushf(&cmd->env, "%s=%s", CONFIG_DATA_ENVIRONMENT, ++ opts->ctx->config_parameters); +} + /* @@ sequencer.c: static int continue_single_pick(struct repository *r, struct replay /* ## t/t3418-rebase-continue.sh ## -@@ t/t3418-rebase-continue.sh: 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 && +@@ t/t3418-rebase-continue.sh: test_orig_head --merge + test_expect_success 'rebase runs auto maintenance once it is done' ' + git checkout -b auto-maintenance topic && + test_must_fail env GIT_TRACE2_EVENT="$(pwd)/stop.txt" \ +- git rebase -x false main && ++ git rebase -x "git commit --allow-empty -m exec && false" main && + test_subcommand_flex ! git maintenance run --auto <stop.txt && + echo resolved >F2 && + git add F2 && +- test_must_fail git rebase --continue && + 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 && + GIT_TRACE2_EVENT="$(pwd)/end.txt" git rebase --continue && +- test_subcommand_flex git maintenance run --auto <end.txt + test_subcommand_flex git maintenance run --auto <end.txt && + grep "\"child_start\".*\"maintenance\"" end.txt >maintenance && + test_line_count = 1 maintenance -+' -+ + ' + test_done ## t/t3510-cherry-pick-sequence.sh ## -@@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'cherry-pick runs auto maintenance once it is done' ' - 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 && +@@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'commit descriptions in insn sheet are optional' ' + + 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 && ++ GIT_TRACE2_EVENT="$(pwd)/single.txt" git cherry-pick --edit picked && ++ test_subcommand_flex git commit <single.txt && + test_subcommand_flex git maintenance run --auto <single.txt && ++ grep "\"child_start\".*\"maintenance\"" single.txt >maintenance && ++ test_line_count = 1 maintenance && + GIT_TRACE2_EVENT="$(pwd)/sequence.txt" \ + git cherry-pick anotherpick yetanotherpick && + test_subcommand_flex git maintenance run --auto <sequence.txt && +@@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'cherry-pick runs auto maintenance once a stopped sequence i + test_subcommand_flex ! git maintenance run --auto <stop.txt && + echo resolved >foo && + git add foo && +- test_must_fail git cherry-pick --continue && + 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 && + GIT_TRACE2_EVENT="$(pwd)/end.txt" git cherry-pick --skip && +- test_subcommand_flex git maintenance run --auto <end.txt ++ test_subcommand_flex git maintenance run --auto <end.txt && + grep "\"child_start\".*\"maintenance\"" end.txt >maintenance && + test_line_count = 1 maintenance -+' -+ + ' + test_done -- gitgitgadget