All of lore.kernel.org
 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>,
	Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>,
	Thomas Bachem <mail@thomasbachem.com>
Subject: Re: [PATCH v5 0/3] sequencer: leave auto maintenance to the end of a sequence
Date: Wed, 23 Sep 2026 16:07:23 +0100	[thread overview]
Message-ID: <c67ed25c-e54b-4289-bfb4-67f86beb6df6@gmail.com> (raw)
In-Reply-To: <pull.2217.v5.git.1789670534.gitgitgadget@gmail.com>

Hi Thomas

On 17/09/2026 19:42, Thomas Bachem via GitGitGadget wrote:
> Changes since v4:
> 
>   * 2/3 takes Patrick's wording, with two corrections: the sequencer itself
>     spawns the "git commit" for a resolved conflict, and the apply backend
>     belongs to "git rebase", not to the sequencer.
>   * 3/3 opens with Phillip's sentence.
>   * The header comment of git_config_append_parameter() is Patrick's, plus
>     one sentence on a NULL value.
>   * Both tests got a comment on which picks conflict and where the sequence
>     stops (Phillip).

Thanks for adding that, it is easier to understand what the tests are 
doing now. This looks ready for next to me.

Thanks for working on it

Phillip

> 
> The code is unchanged.
> 
> Patrick, my answer to your question on 2/3 went out under the 1/3 subject by
> mistake [1]. The single picks are another exception: a "git cherry-pick
> <commit>" or "git revert <commit>" never reaches pick_commits(), and neither
> does its "--continue" or "--skip". So inside the sequencer the call would go
> to the end of pick_commits() and to those three places, plus the aborts if
> you want them. I kept it in the two builtins, which Phillip preferred, and
> 2/3 now says why. Say if you'd rather have it in the sequencer and I'll move
> it.
> 
> Based on master. Independent of the rerere series in [2].
> 
> [1] <CAA0xjtoW3JfSbuBot0ANFiDEhEv1N-Di4mSd5tYfgkFEeh07Nw@mail.gmail.com>
> 
> [2] <pull.2214.v4.git.1789373061.gitgitgadget@gmail.com>
> 
> Thomas Bachem (3):
>    config: add git_config_append_parameter()
>    rebase, cherry-pick, revert: run auto maintenance when done
>    sequencer: disable auto maintenance in spawned commands
> 
>   builtin/rebase.c                | 13 ++++++++---
>   builtin/revert.c                | 19 +++++++++++------
>   config.c                        | 20 +++++++++++------
>   config.h                        | 12 +++++++++++
>   sequencer.c                     | 38 ++++++++++++++++++++++++++++++---
>   t/t3418-rebase-continue.sh      | 20 +++++++++++++++++
>   t/t3510-cherry-pick-sequence.sh | 33 ++++++++++++++++++++++++++++
>   7 files changed, 135 insertions(+), 20 deletions(-)
> 
> 
> base-commit: 3cb9185f65410273787f74333cc027d2ea5daada
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2217%2Fthomasbachem%2Frebase-auto-maintenance-v5
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2217/thomasbachem/rebase-auto-maintenance-v5
> Pull-Request: https://github.com/gitgitgadget/git/pull/2217
> 
> Range-diff vs v4:
> 
>   1:  0472fadbc5 ! 1:  724baf2789 config: add git_config_append_parameter()
>       @@ config.h: int git_config_from_blob_oid(config_fn_t fn, const char *name,
>         void git_config_push_env(const char *spec);
>        +
>        +/*
>       -+ * Append a "-c key=value" setting to a GIT_CONFIG_PARAMETERS value in
>       -+ * `env`. The variable carries such settings from a git process to the
>       -+ * git commands it spawns, as a space separated list of 'key'='value'
>       -+ * pairs with both sides single quoted, which git_config_from_parameters()
>       -+ * reads back. A NULL `value` appends 'key'= with nothing after the equals
>       -+ * sign, which stands for a boolean true, like "-c key" on the command
>       -+ * line.
>       ++ * Append a config option to the buffer that can be exported via the
>       ++ * GIT_CONFIG_PARAMETERS environment variable, which allows us to
>       ++ * propagate configuration across Git processes. The format of the
>       ++ * variable is a space-separated list of quoted "'<key>'='<value>'"
>       ++ * pairs. With a NULL `value`, only 'key'= is appended, which git reads
>       ++ * back as a boolean true, like "-c key" on the command line.
>        + */
>        +void git_config_append_parameter(struct strbuf *env, const char *key,
>        +				 const char *value);
>   2:  b7b97262f2 ! 2:  f0ec8f1f41 rebase, cherry-pick, revert: run auto maintenance when done
>       @@ Metadata
>         ## Commit message ##
>            rebase, cherry-pick, revert: run auto maintenance when done
>        
>       -    "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.
>       +    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. Only the commands they spawn on the
>       +    way run it, like git-commit(1) for a resolved conflict or an edited
>       +    message.
>        
>       -    Run it once when the sequence is done, like the apply backend does.
>       +    In contrast to that, the "apply" backend of git-rebase(1) _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.
>        
>       -    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".
>       +    Adapt users of the "merge" backend to do the same.
>       +
>       +    The sequencer has no single exit where a call to run_auto_maintenance()
>       +    could go. A sequence ends in pick_commits(), but a single pick never
>       +    gets there: it returns from sequencer_pick_revisions() via
>       +    single_pick(), its "--continue" from sequencer_continue() via
>       +    continue_single_pick(), and its "--skip" from sequencer_skip(). So
>       +    call it from the sequencer's two callers instead: run_specific_rebase()
>       +    for the merge backend, once its state directory is gone, and
>       +    run_sequencer() in builtin/revert.c after a successful pick,
>       +    "--continue" or "--skip".
>        
>            Assisted-by: Claude Fable 5.1
>            Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
>       @@ t/t3418-rebase-continue.sh: test_orig_head () {
>         test_orig_head --merge
>         
>        +test_expect_success 'rebase runs auto maintenance once it is done' '
>       ++	# topic and main both add F2, so the pick conflicts and the rebase
>       ++	# stops before the exec runs, and once more when the exec fails
>        +	git checkout -b auto-maintenance topic &&
>        +	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/stop.txt" \
>        +		git rebase -x false main &&
>       @@ t/t3510-cherry-pick-sequence.sh: test_expect_success 'commit descriptions in ins
>        +'
>        +
>        +test_expect_success 'cherry-pick runs auto maintenance once a stopped sequence is done' '
>       ++	# both picked and anotherpick conflict on foo, so "--continue" stops
>       ++	# once more before "--skip" ends the sequence
>        +	pristine_detach initial &&
>        +	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/stop.txt" \
>        +		git cherry-pick base..anotherpick &&
>   3:  031b3bd498 ! 3:  c51ea031ea sequencer: disable auto maintenance in spawned commands
>       @@ Metadata
>         ## Commit message ##
>            sequencer: disable auto maintenance in spawned commands
>        
>       -    Sequencer-spawned commands like 'commit' and 'merge' run
>       -    background auto maintenance, which interferes with ongoing
>       -    operations (e.g. 'rerere gc' holding MERGE_RR.lock or repacks
>       -    deleting active packs).
>       +    When the sequencer spawns "git commit" or "git merge", those commands
>       +    run "git maintenance run --auto" in the background, which can
>       +    interfere with the sequencer (e.g. 'rerere gc' holding MERGE_RR.lock
>       +    or repacks deleting active packs).
>        
>            Pass maintenance.auto=false via GIT_CONFIG_PARAMETERS to the
>            spawned commit, merge and exec commands. Appending it after the
>       @@ sequencer.c: static int continue_single_pick(struct repository *r, struct replay
>        
>         ## t/t3418-rebase-continue.sh ##
>        @@ t/t3418-rebase-continue.sh: test_orig_head --merge
>       +
>         test_expect_success 'rebase runs auto maintenance once it is done' '
>       + 	# topic and main both add F2, so the pick conflicts and the rebase
>       +-	# stops before the exec runs, and once more when the exec fails
>       ++	# stops before the exec runs. "--continue" commits the resolution
>       ++	# first, then runs the exec, which fails and stops it again.
>         	git checkout -b auto-maintenance topic &&
>         	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/stop.txt" \
>        -		git rebase -x false main &&
> 


  parent reply	other threads:[~2026-09-23 15:07 UTC|newest]

Thread overview: 52+ 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-14  7:12       ` Thomas Bachem
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-14  7:12       ` Thomas Bachem
2026-09-22  7:38       ` Thomas Bachem
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
2026-09-14  7:11       ` Thomas Bachem
2026-09-17 18:42 ` [PATCH v5 0/3] sequencer: leave auto maintenance to the end of a sequence Thomas Bachem via GitGitGadget
2026-09-17 18:42   ` [PATCH v5 1/3] config: add git_config_append_parameter() Thomas Bachem via GitGitGadget
2026-09-17 18:42   ` [PATCH v5 2/3] rebase, cherry-pick, revert: run auto maintenance when done Thomas Bachem via GitGitGadget
2026-09-17 18:42   ` [PATCH v5 3/3] sequencer: disable auto maintenance in spawned commands Thomas Bachem via GitGitGadget
2026-09-23 15:07   ` Phillip Wood [this message]
2026-09-28  7:49     ` [PATCH v5 0/3] sequencer: leave auto maintenance to the end of a sequence Patrick Steinhardt

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=c67ed25c-e54b-4289-bfb4-67f86beb6df6@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=kristofferhaugsbakk@fastmail.com \
    --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 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.