All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Thomas Bachem via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Phillip Wood <phillip.wood@dunelm.org.uk>,
	Junio C Hamano <gitster@pobox.com>,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Thomas Bachem <mail@thomasbachem.com>
Subject: Re: [PATCH v2 2/3] sequencer: run auto maintenance once a sequence is done
Date: Mon, 7 Sep 2026 10:14:12 +0200	[thread overview]
Message-ID: <ap5yVFNEFm2vdP1B@pks.im> (raw)
In-Reply-To: <baab8d4876441ea883044c34bb5584631e30e1ec.1788537086.git.gitgitgadget@gmail.com>

On Fri, Sep 04, 2026 at 03:51:25PM +0000, 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
> 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?

> 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

  reply	other threads:[~2026-09-07  8:14 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
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 [this message]
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=ap5yVFNEFm2vdP1B@pks.im \
    --to=ps@pks.im \
    --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 \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.