Re: [PATCH v2 2/3] sequencer: run auto maintenance once a sequence is done

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

Hi Thomas

On 04/09/2026 16:51, Thomas Bachem via GitGitGadget wrote:
> 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

> 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()

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

> +		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?

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