Rename worktrees to trash before deleting#59240
Draft
MartinYe1234 wants to merge 3 commits into
Draft
Conversation
Removing a worktree previously deleted its directory in place with a recursive unlink+rmdir, then ran `git worktree remove`. When another process (e.g. a `cargo check` spawned by rust-analyzer, or a user terminal) was still writing into the worktree, the final rmdir raced with those writes and failed with ENOTEMPTY, aborting the archive and leaving a partially deleted tree. Instead, atomically rename the worktree into a `.trash` sibling of the managed worktrees directory, then delete it in a detached background task with retry/backoff. The rename is a single atomic operation that succeeds even with live writers, so the archive completes immediately and stray writes land harmlessly in the trash entry until the writers wind down. A `.trash/.gitignore` keeps the entry out of git when the worktrees directory is configured inside a working tree, and leftover entries are swept (age-gated) when a repository is registered. If `git worktree remove` fails after the rename, the directory is renamed back so an error from remove_worktree still means the worktree is intact at its original path.
The sweep scheduled on repository registration issued background filesystem operations that, on FakeFs, consumed the deterministic test scheduler's RNG and perturbed the completion ordering of unrelated async work, breaking editor hover-link tests. Skip it on the fake filesystem (matching how fs watchers are handled) and cover the sweep logic by calling it directly in tests.
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
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.
Removing a worktree deleted its directory in place with a recursive
unlink +
rmdir, then rangit worktree remove. When another processwas still writing into the worktree (a
cargo checkfrom rust-analyzer,a user terminal running a build, another editor, ...), a file created
between the unlink and the final
rmdirmade it fail withENOTEMPTY,aborting the archive and leaving a partially deleted tree on disk.
This atomically renames the worktree into a
.trashsibling of themanaged worktrees directory and deletes it in a detached background task
with retry/backoff. The rename always succeeds even with live writers,
so the archive completes immediately and stray writes land harmlessly in
the trash entry until the writers wind down. A
.trash/.gitignorekeepsthe entry out of git, leftover entries are swept (age-gated) on
repository registration, and if
git worktree removefails after therename the directory is renamed back so the worktree is left intact.
Closes AI-400
Release Notes: