git.vger.kernel.org archive mirror
 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 v4 3/3] sequencer: disable auto maintenance in spawned commands
Date: Wed, 9 Sep 2026 16:40:07 +0100	[thread overview]
Message-ID: <cd3d98ef-ad80-405d-98b8-fc40e7c5ec9b@gmail.com> (raw)
In-Reply-To: <031b3bd498bd5efee7e0d9752a6dd36fe7a55360.1788942331.git.gitgitgadget@gmail.com>

Hi Thomas

On 09/09/2026 09:25, Thomas Bachem via GitGitGadget wrote:
> From: Thomas Bachem <mail@thomasbachem.com>
> 
> 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).

This is much more concise, but still sounds a bit strange to me. I'd suggest

When the sequencer spawns "git commit", or "git merge", those commands
run "git maintenance --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
> user's own settings ensures it wins, and the environment reaches
> whatever they spawn in turn.
> 
> Auto maintenance now runs exactly once when the sequence
> completes. Commands run manually by the user while stopped are
> unaffected and continue to run auto maintenance normally.

Good, and the implementation changes look correct.

> diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
> index 025787b5f2..16def261b0 100755
> --- a/t/t3418-rebase-continue.sh
> +++ b/t/t3418-rebase-continue.sh
> @@ -398,13 +398,18 @@ test_orig_head --merge
>   test_expect_success 'rebase runs auto maintenance once it is done' '
>   	git checkout -b auto-maintenance topic &&
>   	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/stop.txt" \
> -		git rebase -x false main &&
> +		git rebase -x "git commit --allow-empty -m exec && false" main &&
>   	test_subcommand_flex ! git maintenance run --auto <stop.txt &&
>   	echo resolved >F2 &&
>   	git add F2 &&
> -	test_must_fail git rebase --continue &&
> +	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/mid.txt" \
> +		git rebase --continue &&

What's this trying to check - there wasn't a conflict so 
commit_staged_changes() will error out without trying to commit 
anything. This series is looking pretty good now. Just to let you know 
I'll be off the list from tomorrow until the middle of next week so it 
will be a few days before I look at any new versions of this patch.

Thanks

Phillip

> +	test_subcommand_flex git commit <mid.txt &&
> +	test_subcommand_flex ! git maintenance run --auto <mid.txt &&
>   	GIT_TRACE2_EVENT="$(pwd)/end.txt" git rebase --continue &&
> -	test_subcommand_flex git maintenance run --auto <end.txt
> +	test_subcommand_flex git maintenance run --auto <end.txt &&
> +	grep "\"child_start\".*\"maintenance\"" end.txt >maintenance &&
> +	test_line_count = 1 maintenance
>   '
>   
>   test_done
> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
> index 2bea55c3b6..1e3fa1803c 100755
> --- a/t/t3510-cherry-pick-sequence.sh
> +++ b/t/t3510-cherry-pick-sequence.sh
> @@ -723,8 +723,11 @@ test_expect_success 'commit descriptions in insn sheet are optional' '
>   
>   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 &&
> +	GIT_TRACE2_EVENT="$(pwd)/single.txt" git cherry-pick --edit picked &&
> +	test_subcommand_flex git commit <single.txt &&
>   	test_subcommand_flex git maintenance run --auto <single.txt &&
> +	grep "\"child_start\".*\"maintenance\"" single.txt >maintenance &&
> +	test_line_count = 1 maintenance &&
>   	GIT_TRACE2_EVENT="$(pwd)/sequence.txt" \
>   		git cherry-pick anotherpick yetanotherpick &&
>   	test_subcommand_flex git maintenance run --auto <sequence.txt &&
> @@ -739,9 +742,14 @@ test_expect_success 'cherry-pick runs auto maintenance once a stopped sequence i
>   	test_subcommand_flex ! git maintenance run --auto <stop.txt &&
>   	echo resolved >foo &&
>   	git add foo &&
> -	test_must_fail git cherry-pick --continue &&
> +	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/mid.txt" \
> +		git cherry-pick --continue &&
> +	test_subcommand_flex git commit <mid.txt &&
> +	test_subcommand_flex ! git maintenance run --auto <mid.txt &&
>   	GIT_TRACE2_EVENT="$(pwd)/end.txt" git cherry-pick --skip &&
> -	test_subcommand_flex git maintenance run --auto <end.txt
> +	test_subcommand_flex git maintenance run --auto <end.txt &&
> +	grep "\"child_start\".*\"maintenance\"" end.txt >maintenance &&
> +	test_line_count = 1 maintenance
>   '
>   
>   test_done


  reply	other threads:[~2026-09-09 15:40 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 [this message]
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   ` [PATCH v5 0/3] sequencer: leave auto maintenance to the end of a sequence Phillip Wood
2026-09-28  7:49     ` 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=cd3d98ef-ad80-405d-98b8-fc40e7c5ec9b@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).