sequencer: release the ODB before spawning git commit - #2198
Open
dscho wants to merge 1 commit into
Open
Conversation
Member
Author
|
/submit |
|
Submitted as pull.2198.git.1786388689444.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
1 task
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "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 |
As of 4557f1a (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 #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 28d04e1 (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 69ed0e3 (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>
dscho
force-pushed
the
rebase-release-odb-before-commit
branch
from
August 12, 2026 09:52
904d65e to
039fd29
Compare
Member
Author
|
/submit |
|
Submitted as pull.2198.v2.git.1786528498689.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
This branch is now known as |
|
This patch series was integrated into seen via git@5db3f91. |
|
Junio C Hamano wrote on the Git mailing list (how to reply to this email): "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): 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
Thanks. Let me mark the topic for 'next'.
> 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 |
|
This patch series is no longer integrated into seen. |
|
This patch series was integrated into seen via git@b81eaff. |
|
There was a status update in the "New Topics" section about the branch The sequencer has been updated to release the object database before spawning 'git commit'. This prevents open file handles from blocking auto-maintenance tasks, such as repacking, on systems like Windows where open files cannot be easily unlinked. Will merge to 'next'? cf. <xmqqqzk3xqxs.fsf@gitster.g> source: <pull.2198.v2.git.1786528498689.gitgitgadget@gmail.com> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes git-for-windows#6315
Changes since v1: