Git development
 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>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>,
	Thomas Bachem <mail@thomasbachem.com>
Subject: Re: [PATCH v4 2/3] rebase, cherry-pick, revert: run auto maintenance when done
Date: Fri, 11 Sep 2026 09:34:26 +0200	[thread overview]
Message-ID: <aqOvAttPrhUIP_7U@pks.im> (raw)
In-Reply-To: <b7b97262f27782f3271369115496c67f9774b8b2.1788942331.git.gitgitgadget@gmail.com>

On Wed, Sep 09, 2026 at 08:25:30AM +0000, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
> 
> "git cherry-pick", "git revert" and the merge backend of "git rebase"
> create their commits in process, so auto maintenance runs only when
> they spawn a command that runs it, like the "git commit" for a
> resolved conflict. A sequence thus runs it in the middle, after each
> resolution, or never.

This paragraph still doesn't make a lot of sense. How about:

  Commands that use the sequencer with the "merge" backend, like
  git-cherry-pick(1) or git-rebase(1) with "--merge", create their
  commits in-process. Consequently, these commands typically don't
  execute auto-maintenance at all. The only exception is when a conflict
  happens, as the user would have to manually commit the result via
  git-commit(1), and that command triggers auto-maintenance for us.

  In contrast to that, the "apply" backend of the sequencer _does_ run
  auto-maintenance after it has processed the sequence of commits. And
  this is a sensible thing to do: after all, we may just have written
  lots of objects, so chances are high that we have something to clean
  up now.

  Adapt users of the "merge" backend to do the same.

  Unfortunately, there is no single exit point for this backend where we
  could add a call to `run_auto_maintenance()`. While one might expect
  that we could simply trigger auto-maintenance in `pick_commits()` and
  call it a day, a single pick as it is performed by e.g. git-revert(1)
  never executes that function. So instead, manually trigger
  auto-maintenance at several sites.

This last paragraph though...

> Run it once when the sequence is done, like the apply backend does.
> 
> The sequencer has no single place where every sequence ends: a
> sequence of several commits ends in pick_commits(), a single pick
> returns as soon as its commit is made, and "--continue" and "--skip"
> have entry points of their own. Run it from the two builtins that
> start or continue a sequence instead: run_specific_rebase() once the
> sequencer has returned and removed its state directory, and
> run_sequencer() after a successful pick, "--continue" or "--skip".

... is still kind of dubious. As far as I can see, almost everything
does end up in `pick_commits()` eventually:

  - git-revert(1) does via `run_sequencer()`, which calls
    `sequencer_pick_revisions()`, and that calls `pick_commits()`.

  - git-cherry-pick(1) does via the same call chain.

  - git-rebase(1) does so via `do_interactive_rebase()`, which calls
    `complete_action()`, and that function calls `pick_commits()`. Or
    alternatively via `sequencer_continue()`, which again calls it.

  - Skipping commits via "--skip" eventually ends up in
    `sequencer_continue()`, and that calls `pick_commits()`.

The only exception that I could spot is when we abort the sequencer. But
I'd rather have us call auto-maintenance when `pick_commits()` is done
and when we abort rather than having every user of the sequencer do it
manually.

Or am I missing something here?

Patrick

  reply	other threads:[~2026-09-11  7:34 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
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 [this message]
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=aqOvAttPrhUIP_7U@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=kristofferhaugsbakk@fastmail.com \
    --cc=mail@thomasbachem.com \
    --cc=phillip.wood123@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox