[PATCH] sequencer: release the ODB before spawning git commit

[PATCH] sequencer: release the ODB before spawning git commit

[Johannes Schindelin via GitGitGadget]

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 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.

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

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2198%2Fgit-for-windows%2Frebase-release-odb-before-commit-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2198/git-for-windows/rebase-release-odb-before-commit-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2198

 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
-- 
gitgitgadget

Re: [PATCH] sequencer: release the ODB before spawning git commit

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> 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).

The sequencer is holding some resources, presumably because it needs
them to continue, and yet auto-maintenance wants to remove them?
Whether or not we face a Windows-specific limitation when removing
them, it is concerning to imagine what would happen if
auto-maintenance were allowed to do so and the sequencer then
resumed its work, only to find that the resources needed for its
operation were gone.

I think what the proposed commit log message lacks after 'must be
released' is 'and after auto-maintenance finishes and we regain
control, we will automatically reacquire these resources in a
refreshed state to proceed'.  Such an explanation would allay the
unease I expressed in the previous paragraph.

Perhaps issues/6315 talks about what exactly are held, but not
spelling it out in the log message is not helping readers.

> Release the sequencer's ODB before spawning `git commit`.

Makes sense.

> 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.

> 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

[PATCH v2] sequencer: release the ODB before spawning git commit

[Johannes Schindelin via GitGitGadget]

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.

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
-- 
gitgitgadget