Merge pull request #1900 from tyrielv/tyrielv/delete-folder-then-restore-functional-test

Fix for delete/restore issue

Author: tyrielv

GVFS/GVFS.FunctionalTests/Tests/GitCommands/CorruptionReproTests.cs

@@ -75,5 +75,24 @@ public void ReproCherryPickRestoreCorruption()
             this.ValidateGitCommand("restore -- .");
             this.FilesShouldMatchCheckoutOfSourceBranch();
         }
+
+        /// <summary>
+        /// Reproduction of a reported issue:
+        /// Restoring a file after its parent directory was deleted fails with
+        /// "fatal: could not unlink 'path\to\': Directory not empty"
+        ///
+        /// See https://github.com/microsoft/VFSForGit/issues/1901
+        /// </summary>
+        [TestCase]
+        public void RestoreAfterDeleteNesteredDirectory()
+        {
+            // Delete a directory with nested subdirectories and files.
+            this.ValidateNonGitCommand("cmd.exe", "/c \"rmdir /s /q GVFlt_DeleteFileTest\"");
+
+            // Restore the working directory.
+            this.ValidateGitCommand("restore .");
+
+            this.FilesShouldMatchCheckoutOfSourceBranch();
+        }
     }
 }

GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs

@@ -269,6 +269,23 @@ protected void ValidateGitCommand(string command, params object[] args)
                 args);
         }
 
+        protected void ValidateNonGitCommand(string command, string args = "", bool ignoreErrors = false, bool checkStatus = true)
+        {
+            string controlRepoRoot = this.ControlGitRepo.RootPath;
+            string gvfsRepoRoot = this.Enlistment.RepoRoot;
+
+            ProcessResult expectedResult = ProcessHelper.Run(command, args, controlRepoRoot);
+            ProcessResult actualResult = ProcessHelper.Run(command, args, gvfsRepoRoot);
+            if (!ignoreErrors)
+            {
+                GitHelpers.ErrorsShouldMatch(command, expectedResult, actualResult);
+            }
+            if (checkStatus)
+            {
+                this.ValidateGitCommand("status");
+            }
+        }
+
         protected void ChangeMode(string filePath, ushort mode)
         {
             string virtualFile = Path.Combine(this.Enlistment.RepoRoot, filePath);

GVFS/GVFS.FunctionalTests/Tools/ProcessHelper.cs

@@ -6,6 +6,11 @@ namespace GVFS.FunctionalTests.Tools
     public static class ProcessHelper
     {
         public static ProcessResult Run(string fileName, string arguments)
+        {
+            return Run(fileName, arguments, null);
+        }
+
+        public static ProcessResult Run(string fileName, string arguments, string workingDirectory)
         {
             ProcessStartInfo startInfo = new ProcessStartInfo();
             startInfo.UseShellExecute = false;
@@ -14,6 +19,10 @@ public static ProcessResult Run(string fileName, string arguments)
             startInfo.CreateNoWindow = true;
             startInfo.FileName = fileName;
             startInfo.Arguments = arguments;
+            if (!string.IsNullOrEmpty(workingDirectory))
+            {
+                startInfo.WorkingDirectory = workingDirectory;
+            }
 
             return Run(startInfo);
         }

GVFS/GVFS.Platform.Windows/WindowsFileSystemVirtualizer.cs

@@ -1014,7 +1014,25 @@ private void NotifyNewFileCreatedHandler(
                         GitCommandLineParser gitCommand = new GitCommandLineParser(this.Context.Repository.GVFSLock.GetLockedGitCommand());
                         if (gitCommand.IsValidGitCommand)
                         {
-                            this.MarkDirectoryAsPlaceholder(virtualPath, triggeringProcessId, triggeringProcessImageFileName);
+                            // When git recreates a directory that was previously deleted (and is
+                            // tracked in ModifiedPaths), skip marking it as a ProjFS placeholder.
+                            // Otherwise ProjFS would immediately project all children into it,
+                            // conflicting with git's own attempt to populate the directory.
+                            //
+                            // This check is safe from races with the background task that updates
+                            // ModifiedPaths: the deletion happens from a non-git process (e.g.,
+                            // rmdir), and IsReadyForExternalAcquireLockRequests() blocks git from
+                            // acquiring the GVFS lock until the background queue is drained. When
+                            // git itself deletes a folder, the code takes the IsValidGitCommand
+                            // path in OnWorkingDirectoryFileOrFolderDeleteNotification and calls
+                            // OnPossibleTombstoneFolderCreated instead of OnFolderDeleted, so
+                            // ModifiedPaths is not involved.
+                            //
+                            // See https://github.com/microsoft/VFSForGit/issues/1901
+                            if (!this.FileSystemCallbacks.IsPathOrParentInModifiedPaths(virtualPath, isFolder: true))
+                            {
+                                this.MarkDirectoryAsPlaceholder(virtualPath, triggeringProcessId, triggeringProcessImageFileName);
+                            }
                         }
                         else
                         {

GVFS/GVFS.Virtualization/FileSystemCallbacks.cs

@@ -367,6 +367,17 @@ public IEnumerable<string> GetAllModifiedPaths()
             return this.modifiedPaths.GetAllModifiedPaths();
         }
 
+        /// <summary>
+        /// Checks whether the given folder path, or any of its parent folders,
+        /// is in the ModifiedPaths database. Used to determine if git/user has
+        /// taken ownership of a directory tree.
+        /// </summary>
+        public bool IsPathOrParentInModifiedPaths(string path, bool isFolder)
+        {
+            return this.modifiedPaths.Contains(path, isFolder) ||
+                   this.modifiedPaths.ContainsParentFolder(path, out _);
+        }
+
         /// <summary>
         /// Finds index entries that are staged (differ from HEAD) matching the given
         /// pathspec, and adds them to ModifiedPaths. This prepares for an unstage operation