Re: [PATCH v2 2/3] sequencer: run auto maintenance once a sequence is done
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Sep 7, 2026, 13:25 UTC
- Message-ID
- <d09ef622-1398-4e38-8a04-8542e7347a98@gmail.com>
- In-Reply-To
- <baab8d4876441ea883044c34bb5584631e30e1ec.1788537086.git.gitgitgadget@gmail.com>
Hi Thomas
On 04/09/2026 16:51, Thomas Bachem via GitGitGadget wrote:
Show 5 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
The merge backend which is also used by "git cherry-pick" and "git
revert" does not run it when it finishes.would be clearer to me
> 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.
Like Patrick I cannot understand what this is saying, let alone whether it is saying anything useful.
> 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.
Run "git maintenace --auto" at the end of all sequencer operations,
...would be clearer to me
Show 5 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:
This makes me think we should change the fragment re for c files to exclude ':', but that's a digression and would mean having different expressions for c and c++.
Anyway this change is at the end of pick_commits(), just before we finish so looks like the right place to call run_auto_maintenance()
Show 21 quoted lines
> 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);It is a shame the single pick variants of "git cherry-pick" and "git revert" do not share the same code path as the multiple pick variants. continue_single_pick() runs "git commit" without calling run_git_commit() which is also unfortunate, but means that we could just rely and "git commit" to call run_auto_maintenance() for us.
Show 30 quoted lines
> + 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;
> }
>
> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index cb5c3a1cb5..2c34cf8a01 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -395,4 +395,12 @@ 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 &&Do we want to assert that we don't run auto maintenance up to this point?
Show 20 quoted lines
> + GIT_TRACE2_EVENT="$(pwd)/finish.txt" git rebase --continue && > + test_subcommand_flex git maintenance run --auto <finish.txt > +' > + > test_done > diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh > index 5777dff496..304981ccd6 100755 > --- a/t/t3510-cherry-pick-sequence.sh > +++ b/t/t3510-cherry-pick-sequence.sh > @@ -721,4 +721,14 @@ 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 &&
Using test_grep here would mean we get some useful test output if there are not matches in the file. Without that test_line_count just says the line count didn't match and prints an empty file.
Thanks
Phillip
> + test_line_count = 1 maintenance > +' > + > test_done