Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH v2] sequencer: release the ODB before spawning git commit
Date: Mon, 24 Aug 2026 11:03:16 +0100	[thread overview]
Message-ID: <a786e6c0-1c17-4121-8623-b4541478a88f@gmail.com> (raw)
In-Reply-To: <pull.2198.v2.git.1786528498689.gitgitgadget@gmail.com>

Hi Johannes

On 12/08/2026 10:54, Johannes Schindelin via GitGitGadget wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> As of 4557f1add261 (rebase--helper: add a builtin helper for interactive
> rebases, 2017-02-09), continuing an interactive rebase uses the builtin
> sequencer, which spawns `git commit`.
> 
> The child may trigger auto-maintenance, which may need to replace files
> for which the sequencer still holds resources. See
> https://github.com/git-for-windows/git/issues/6315: on Windows, this
> produces unlink retry prompts that cannot succeed while the sequencer
> waits for the child.
> 
> Resources such as file handles or memory mappings must be released
> before spawning a command that may run auto-maintenance, as established
> by 28d04e1ec197 (run-command: offer to close the object store before
> running, 2021-09-09): release the ODB file handles and memory mappings,
> so that auto-gc can repack (potentially deleting existing packfiles in
> the process); If the sequencer needs to access the ODB afterwards, it
> will gracefully (re-)open the ODB.
> 
> Release the sequencer's ODB before spawning `git commit`. The regression
> test uses the legacy-delete trick introduced by 69ed0e35a754 (mingw:
> optionally use legacy (non-POSIX) delete semantics, 2026-05-07) to
> trigger the failure on modern Windows.

This looks fine as an immediate fix for the bug but I wonder if we 
should pass "-c gc.auto=false" when we fork "git commit" from the 
sequencer. We call run_auto_maintenance() at the end of the rebase and 
its not clear to me that repacking during the rebase is helpful. Another 
thought I had was whether we should automatically close the object 
database when forking another git command. I'm not sure how easy that is 
to implement but it would prevent future regressions and I assuming 
re-opening the object store is cheap compared to forking another git 
command.

Thanks

Phillip


> Assisted-by: GPT-5.6 Sol
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>      sequencer: release the ODB before spawning git commit
>      
>      This fixes https://github.com/git-for-windows/git/issues/6315
>      
>      Changes since v1:
>      
>       * Clarify in the commit message what the strategy introduced in
>         28d04e1ec197 (run-command: offer to close the object store before
>         running, 2021-09-09) is all about.
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2198%2Fgit-for-windows%2Frebase-release-odb-before-commit-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2198/git-for-windows/rebase-release-odb-before-commit-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/2198
> 
> Range-diff vs v1:
> 
>   1:  904d65e8cb ! 1:  039fd29039 sequencer: release the ODB before spawning git commit
>       @@ Commit message
>            Resources such as file handles or memory mappings must be released
>            before spawning a command that may run auto-maintenance, as established
>            by 28d04e1ec197 (run-command: offer to close the object store before
>       -    running, 2021-09-09).
>       +    running, 2021-09-09): release the ODB file handles and memory mappings,
>       +    so that auto-gc can repack (potentially deleting existing packfiles in
>       +    the process); If the sequencer needs to access the ODB afterwards, it
>       +    will gracefully (re-)open the ODB.
>        
>            Release the sequencer's ODB before spawning `git commit`. The regression
>            test uses the legacy-delete trick introduced by 69ed0e35a754 (mingw:
> 
> 
>   sequencer.c                   |  1 +
>   t/t3404-rebase-interactive.sh | 18 ++++++++++++++++++
>   2 files changed, 19 insertions(+)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 57855b0066..83952d96e3 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1127,6 +1127,7 @@ static int run_git_commit(const char *defmsg,
>   	struct child_process cmd = CHILD_PROCESS_INIT;
>   
>   	cmd.git_cmd = 1;
> +	cmd.odb_to_close = the_repository->objects;
>   
>   	if (is_rebase_i(opts) &&
>   	    ((opts->committer_date_is_author_date && !opts->ignore_date) ||
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 58b3bb0c27..8f81c80fd4 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -65,6 +65,24 @@ test_expect_success 'setup' '
>   	test_commit P fileP
>   '
>   
> +test_expect_success MINGW 'rebase releases object database before committing' '
> +	test_when_finished "rm -f .git/hooks/post-commit repacked packs" &&
> +	git switch -C repack-rewrite primary &&
> +	git repack -ad &&
> +	write_script .git/hooks/post-commit <<-\EOF &&
> +	git repack -ad &&
> +	>repacked
> +	EOF
> +	(
> +		set_fake_editor &&
> +		FAKE_LINES="reword 1" GIT_TEST_LEGACY_DELETE=1 \
> +			git -c core.commitGraph=false rebase -i HEAD^
> +	) &&
> +	test_path_is_file repacked &&
> +	ls .git/objects/pack/*.pack >packs &&
> +	test_line_count = 1 packs
> +'
> +
>   # "exec" commands are run with the user shell by default, but this may
>   # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
>   # to create a file. Unsetting SHELL avoids such non-portable behavior
> 
> base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc


  parent reply	other threads:[~2026-08-24 10:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-10 19:04 [PATCH] sequencer: release the ODB before spawning git commit Johannes Schindelin via GitGitGadget
2026-08-11 18:22 ` Junio C Hamano
2026-08-12  9:54 ` [PATCH v2] " Johannes Schindelin via GitGitGadget
2026-08-12 16:07   ` Junio C Hamano
2026-08-24 10:03   ` Phillip Wood [this message]
2026-08-24 14:36     ` Junio C Hamano
2026-08-25 15:54       ` 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=a786e6c0-1c17-4121-8623-b4541478a88f@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    --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