Skip to content

Fix for delete/restore issue#1900

Merged
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/delete-folder-then-restore-functional-test
Mar 27, 2026
Merged

Fix for delete/restore issue#1900
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/delete-folder-then-restore-functional-test

Conversation

@tyrielv
Copy link
Copy Markdown
Contributor

@tyrielv tyrielv commented Feb 5, 2026

Fix git restore after deleting directory with nested subdirs

Fixes #1901

Problem

Running git restore . after deleting a directory with rmdir /s /q fails with:

warning: unable to unlink 'GVFlt_DeleteFileTest/...': Directory not empty
fatal: cannot create directory at 'GVFlt_DeleteFileTest/...': Directory not empty

This affects git restore (both . and individual file paths), git checkout -- <dir>, and any directory with nested subdirectories — even flat directories with only files.

git reset --hard and git checkout <commit> are not affected.

Root Cause

When git recreates a deleted directory during git restore, ProjFS fires NotifyNewFileCreated. GVFS's handler unconditionally calls MarkDirectoryAsPlaceholder(), which tells ProjFS to virtualize the directory. ProjFS then immediately calls back into GVFS's enumeration callbacks to project all children — recreating subdirectories and files as placeholders before git has a chance to populate them itself.

Git then tries to create those same subdirectories and fails because they already exist (and are non-empty, since ProjFS recursively projected their contents too).

Trace of the failing sequence

1. git.exe creates GVFlt_DeleteFileTest/          → NotifyNewFileCreated
2. GVFS calls MarkDirectoryAsPlaceholder()        → ProjFS now virtualizes the dir
3. ProjFS calls GetPlaceholderInfo for child dirs  → GVFS projects them
4. ProjFS calls StartDirEnum for each child dir    → GVFS returns their contents
5. git.exe tries to create child subdirectory      → FAILS: Directory not empty

Fix

Skip MarkDirectoryAsPlaceholder() when the directory (or a parent) is already in the ModifiedPaths database. A folder in ModifiedPaths means git/user has taken ownership of that path (e.g., by deleting it). Leaving the directory as a regular (non-virtualized) directory allows git to populate it directly without ProjFS interference.

Changes

  • WindowsFileSystemVirtualizer.cs — In NotifyNewFileCreatedHandler, guard the MarkDirectoryAsPlaceholder() call with an IsPathOrParentInModifiedPaths check.
  • FileSystemCallbacks.cs — Add IsPathOrParentInModifiedPaths() helper that checks the ModifiedPathsDatabase for the path and its ancestors.
  • CorruptionReproTests.cs — Add RestoreAfterDeleteNesteredDirectory functional test reproducing the issue.
  • GitRepoTests.cs — Add ValidateNonGitCommand helper for running non-git commands (e.g., rmdir) against both control and GVFS repos.
  • ProcessHelper.cs — Add Run overload accepting a working directory.

Testing

Functional test RestoreAfterDeleteNesteredDirectory:

  1. Deletes GVFlt_DeleteFileTest/ (which has 14 subdirectories, some deeply nested) via rmdir /s /q
  2. Runs git restore .
  3. Validates that the working tree matches the control repo

Also manually verified that existing scenarios continue to work:

  • git reset --hard after directory deletion
  • git checkout <commit> after committing a deletion
  • git checkout -- . after creating a new file in a directory

Synchronization Safety

The IsPathOrParentInModifiedPaths check relies on ModifiedPaths being updated before NotifyNewFileCreatedHandler fires. ModifiedPaths is updated asynchronously via a background task queue, which raises a potential race condition concern. However, this is safe because:

  1. Cross-process case (rmdir then git restore): When git starts, its pre-command hook requests the GVFS lock. IsReadyForExternalAcquireLockRequests() checks backgroundFileSystemTaskRunner.IsEmpty and denies the lock until the queue is drained. So ModifiedPaths is always up-to-date before git's NotifyNewFileCreated can fire.

  2. Within a single git command: When git itself deletes and recreates a directory, OnWorkingDirectoryFileOrFolderDeleteNotification takes the gitCommand.IsValidGitCommand branch and calls OnPossibleTombstoneFolderCreated instead of OnFolderDeleted — ModifiedPaths is not involved, and the fix is not triggered.

The fix only activates when a directory is in ModifiedPaths because a non-git process deleted it, and a subsequent git command recreates it — with a lock boundary guaranteeing ModifiedPaths is current.

@tyrielv tyrielv force-pushed the tyrielv/delete-folder-then-restore-functional-test branch from dffce20 to 175686d Compare March 25, 2026 22:00
@tyrielv tyrielv changed the title Add functional test to repro delete/restore issue Fix for delete/restore issue Mar 25, 2026
@tyrielv tyrielv force-pushed the tyrielv/delete-folder-then-restore-functional-test branch from 175686d to 90fdd5b Compare March 25, 2026 23:07
Add functional test to reproduce issue microsoft#1901: running 'git restore .'
after deleting a directory with nested subdirectories fails with
'fatal: cannot create directory: Directory not empty'.

Root cause: when git recreates a deleted directory, GVFS's
NotifyNewFileCreated handler calls MarkDirectoryAsPlaceholder(), which
causes ProjFS to immediately project all children back into the
directory. Git then fails when it tries to create subdirectories that
ProjFS has already auto-projected.

Fix: skip MarkDirectoryAsPlaceholder() for directories whose path (or a
parent path) is already in ModifiedPaths, indicating git/user has taken
ownership. The directory stays non-virtualized so git can populate it
directly without ProjFS interference.

Fixes microsoft#1901

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tyrielv tyrielv force-pushed the tyrielv/delete-folder-then-restore-functional-test branch from 90fdd5b to 4fb56b4 Compare March 26, 2026 17:19
@tyrielv tyrielv marked this pull request as ready for review March 26, 2026 18:09
@tyrielv tyrielv requested a review from KeithIsSleeping March 26, 2026 18:09
Copy link
Copy Markdown

@KeithIsSleeping KeithIsSleeping left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multi-model review (Opus 4.6, GPT-5.1-Codex-Max, GPT-5.3-Codex, GPT-5.4): production fix is solid — ModifiedPaths check is correct, thread-safe, performant at scale, and compatible with git reset/checkout paths. Minor note: the repro test could be strengthened by asserting exit codes in ValidateNonGitCommand and verifying the deleted subtree (GVFlt_DeleteFileTest) is actually removed/restored, not just the Test_ConflictTests paths. LGTM.

@tyrielv tyrielv merged commit ff944c1 into microsoft:master Mar 27, 2026
49 checks passed
@mjcheetham mjcheetham mentioned this pull request Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

git restore after deleting a directory with nested subdirectories fails

2 participants