Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Thomas Bachem via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: Phillip Wood <phillip.wood@dunelm.org.uk>,
	Patrick Steinhardt <ps@pks.im>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Thomas Bachem <mail@thomasbachem.com>
Subject: Re: [PATCH 1/2] sequencer: run auto maintenance once a rebase is done
Date: Fri, 4 Sep 2026 16:03:28 +0100	[thread overview]
Message-ID: <845f8476-3105-4174-af51-9dd74147ea33@gmail.com> (raw)
In-Reply-To: <3415a4dcbf3c83f85aed3d806d897f5f4906715d.1788508426.git.gitgitgadget@gmail.com>

Hi Thomas

On 04/09/2026 08:53, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
> 
> The apply backend runs "git maintenance run --auto" from
> finish_rebase() once it has applied its patches, and so does "git am"
> on its own. The merge backend reaches finish_rebase() only on the
> paths both backends share in builtin/rebase.c: an abort, a branch
> that is already up to date, and a fast-forward. A rebase that
> replays commits never runs maintenance at its end. It creates most
> of its commits in process, and only the "git commit" it spawns for a
> resolved, reworded or squashed pick, the "git merge" a "rebase -r"
> spawns for an octopus merge or with a strategy, and whatever an exec
> command runs kick maintenance off, in the middle of the rebase. Run
> it where the sequencer finishes a rebase, after the autostash is
> applied, as finish_rebase() does, so that both backends end a rebase
> the same way, and so that the next commit can keep it out of the
> commands a rebase spawns. builtin/rebase.c could run it instead once
> run_sequencer_rebase() returns, but the sequencer is where the rebase
> finishes, and the autostash and the state cleanup that surround the
> run in finish_rebase() are there as well. prepare_auto_maintenance()
> closes the object database before the spawn, so the sequencer holds
> nothing a repack would need to replace.

It would be nice if the two backends shared more of the cleanup code, I 
don't think there is a good reason for them not to share the code that 
copies notes, applies the autostash and switches HEAD back to the 
branch, but that is outside the scope of this change. So adding a 
separate call to the sequencer code seems reasonable. I wonder if we 
should do this for cherry-pick and revert as well; certainly cherry-pick 
can create a lot of loose objects and does not necessarily call "git 
commit". If cherry-pick does call "git commit" and trigger background 
maintenance it is susceptible to the same problems as rebase, so we 
should probably treat them the same.

Thanks

Phillip

> 
> Assisted-by: Claude Fable 5.1
> Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
> ---
>   sequencer.c                | 5 +++++
>   t/t3418-rebase-continue.sh | 8 ++++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 65afd100d9..f58ad254be 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -5297,6 +5297,11 @@ cleanup_head_ref:
>   			run_hooks_opt(r, "post-rewrite", &hook_opt);
>   		}
>   		apply_autostash(rebase_path_autostash());
> +		/*
> +		 * We ignore errors in 'git maintenance run --auto', since the
> +		 * user should see them.
> +		 */
> +		run_auto_maintenance(r, opts->quiet);
>   
>   		if (!opts->quiet) {
>   			if (!opts->verbose)
> 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 &&
> +	GIT_TRACE2_EVENT="$(pwd)/finish.txt" git rebase --continue &&
> +	test_subcommand_flex git maintenance run --auto <finish.txt
> +'
> +
>   test_done


  reply	other threads:[~2026-09-04 15:03 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04  7:53 [PATCH 0/2] sequencer: leave auto maintenance to the end of a rebase Thomas Bachem via GitGitGadget
2026-09-04  7:53 ` [PATCH 1/2] sequencer: run auto maintenance once a rebase is done Thomas Bachem via GitGitGadget
2026-09-04 15:03   ` Phillip Wood [this message]
2026-09-04  7:53 ` [PATCH 2/2] sequencer: keep auto maintenance out of the commands a rebase spawns Thomas Bachem via GitGitGadget
2026-09-04 15:03   ` Phillip Wood
2026-09-04 15:55     ` Thomas Bachem
2026-09-04 15:51 ` [PATCH v2 0/3] sequencer: leave auto maintenance to the end of a sequence Thomas Bachem via GitGitGadget
2026-09-04 15:51   ` [PATCH v2 1/3] config: add git_config_append_parameter() Thomas Bachem via GitGitGadget
2026-09-07  8:14     ` Patrick Steinhardt
2026-09-07 13:24       ` Phillip Wood
2026-09-07 14:47         ` Patrick Steinhardt
2026-09-07 16:37       ` Thomas Bachem
2026-09-04 15:51   ` [PATCH v2 2/3] sequencer: run auto maintenance once a sequence is done Thomas Bachem via GitGitGadget
2026-09-07  8:14     ` Patrick Steinhardt
2026-09-07 16:35       ` Thomas Bachem
2026-09-08  5:50         ` Patrick Steinhardt
2026-09-08  7:46           ` Thomas Bachem
2026-09-07 13:25     ` Phillip Wood
2026-09-07 16:36       ` Thomas Bachem
2026-09-07 16:40         ` Phillip Wood
2026-09-04 15:51   ` [PATCH v2 3/3] sequencer: keep auto maintenance out of the commands a sequence spawns Thomas Bachem via GitGitGadget
2026-09-04 21:21     ` Junio C Hamano
2026-09-05  5:42       ` Thomas Bachem
2026-09-07  8:14     ` Patrick Steinhardt
2026-09-07 16:35       ` Thomas Bachem
2026-09-07 13:24     ` Phillip Wood
2026-09-07 16:37       ` Thomas Bachem
2026-09-08 10:28 ` [PATCH v3 0/3] sequencer: leave auto maintenance to the end of a sequence Thomas Bachem via GitGitGadget
2026-09-08 10:28   ` [PATCH v3 1/3] config: add git_config_append_parameter() Thomas Bachem via GitGitGadget
2026-09-08 10:28   ` [PATCH v3 2/3] rebase, cherry-pick, revert: run auto maintenance when done Thomas Bachem via GitGitGadget
2026-09-08 10:28   ` [PATCH v3 3/3] sequencer: disable auto maintenance in spawned commands Thomas Bachem via GitGitGadget
2026-09-08 15:53   ` [PATCH v3 0/3] sequencer: leave auto maintenance to the end of a sequence Junio C Hamano
2026-09-08 19:38     ` Kristoffer Haugsbakk
2026-09-09  5:57     ` Patrick Steinhardt
2026-09-10  8:25     ` Thomas Bachem
2026-09-09  8:25 ` [PATCH v4 " Thomas Bachem via GitGitGadget
2026-09-09  8:25   ` [PATCH v4 1/3] config: add git_config_append_parameter() Thomas Bachem via GitGitGadget
2026-09-11  7:34     ` Patrick Steinhardt
2026-09-09  8:25   ` [PATCH v4 2/3] rebase, cherry-pick, revert: run auto maintenance when done Thomas Bachem via GitGitGadget
2026-09-11  7:34     ` Patrick Steinhardt
2026-09-09  8:25   ` [PATCH v4 3/3] sequencer: disable auto maintenance in spawned commands Thomas Bachem via GitGitGadget
2026-09-09 15:40     ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=845f8476-3105-4174-af51-9dd74147ea33@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=mail@thomasbachem.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox