Re: [PATCH v2 2/3] sequencer: run auto maintenance once a sequence is done
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 7, 2026, 08:14 UTC
- Message-ID
- <ap5yVFNEFm2vdP1B@pks.im>
- In-Reply-To
- <baab8d4876441ea883044c34bb5584631e30e1ec.1788537086.git.gitgitgadget@gmail.com>
On Fri, Sep 04, 2026 at 03:51:25PM +0000, Thomas Bachem via GitGitGadget wrote:
Show 9 quoted lines
> From: Thomas Bachem <mail@thomasbachem.com> > > 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.
This paragraph just doesn't parse for me, it's really hard to tell what it even wants to say.
> 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.
Besides moving stuff around to prep for the next commit, what does this change? Like, do we now run the command in cases where we didn't before? And if so, what are the consequences of doing so?
Show 44 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index 65afd100d9..67e1c38762 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -5313,6 +5313,12 @@ cleanup_head_ref:
> 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
> @@ -5577,10 +5583,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 */
> @@ -5698,6 +5708,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;
> }
> It's surprisingly many sites where you add the call to `run_auto_maintenance()`. My hope was that there is a single exit path somewhere that is used by both the "apply" and "merge" strategy that we could adapt to unify when exactly we run auto-maintenance across both backends.
Patrick