Merge pull request #1730: Fix an issue with FastFetch when deleting f…

…iles This fixes #1729 and also takes care of a performance problem in the same method.
microsoft · Mar 31, 2021 · c42d132 · c42d132
2 parents c14c0f5 + 024f251
commit c42d132
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 25 deletions.
diff --git a/GVFS/GVFS.Common/Prefetch/Git/DiffHelper.cs b/GVFS/GVFS.Common/Prefetch/Git/DiffHelper.cs
@@ -166,37 +166,45 @@ public void ParseDiffFile(string filename)
 
         private void FlushStagedQueues()
         {
-            List<string> deletedPaths = new List<string>();
-            foreach (DiffTreeResult result in this.stagedDirectoryOperations)
+            using (ITracer activity = this.tracer.StartActivity("FlushStagedQueues", EventLevel.Informational))
             {
-                // Don't enqueue deletes that will be handled by recursively deleting their parent.
-                // Git traverses diffs in pre-order, so we are guaranteed to ignore child deletes here.
-                if (result.Operation == DiffTreeResult.Operations.Delete)
+                HashSet<string> deletedDirectories =
+                    new HashSet<string>(
+                        this.stagedDirectoryOperations
+                        .Where(d => d.Operation == DiffTreeResult.Operations.Delete)
+                        .Select(d => d.TargetPath.TrimEnd(Path.DirectorySeparatorChar)),
+                        GVFSPlatform.Instance.Constants.PathComparer);
+
+                foreach (DiffTreeResult result in this.stagedDirectoryOperations)
                 {
-                    if (deletedPaths.Any(path => result.TargetPath.StartsWith(path, GVFSPlatform.Instance.Constants.PathComparison)))
+                    string parentPath = Path.GetDirectoryName(result.TargetPath.TrimEnd(Path.DirectorySeparatorChar));
+                    if (deletedDirectories.Contains(parentPath))
                     {
-                        continue;
+                        if (result.Operation != DiffTreeResult.Operations.Delete)
+                        {
+                            EventMetadata metadata = new EventMetadata();
+                            metadata.Add(nameof(result.TargetPath), result.TargetPath);
+                            metadata.Add(TracingConstants.MessageKey.WarningMessage, "An operation is intended to go inside of a deleted folder");
+                            activity.RelatedError("InvalidOperation", metadata);
+                        }
+                    }
+                    else
+                    {
+                        this.DirectoryOperations.Enqueue(result);
                     }
-
-                    deletedPaths.Add(result.TargetPath);
                 }
 
-                this.DirectoryOperations.Enqueue(result);
-            }
-
-            foreach (string filePath in this.stagedFileDeletes)
-            {
-                if (deletedPaths.Any(path => filePath.StartsWith(path, GVFSPlatform.Instance.Constants.PathComparison)))
+                foreach (string filePath in this.stagedFileDeletes)
                 {
-                    continue;
+                    string parentPath = Path.GetDirectoryName(filePath);
+                    if (!deletedDirectories.Contains(parentPath))
+                    {
+                        this.FileDeleteOperations.Enqueue(filePath);
+                    }
                 }
 
-                deletedPaths.Add(filePath);
-
-                this.FileDeleteOperations.Enqueue(filePath);
+                this.RequiredBlobs.CompleteAdding();
             }
-
-            this.RequiredBlobs.CompleteAdding();
         }
 
         private void EnqueueOperationsFromLsTreeLine(ITracer activity, string line)

diff --git a/GVFS/GVFS.UnitTests/Data/backward.txt b/GVFS/GVFS.UnitTests/Data/backward.txt
@@ -19,6 +19,8 @@
 :000000 100644 0000000000000000000000000000000000000000 8724404da33605d27cfa5490d412f0dfea16a277 A	folderToDelete/fileToEdit2.txt
 :040000 040000 403c21d5d12f6e6a6659460b5cec7e4b66b6c0f2 9a894ca28a2a8d813f2bae305cd94faf366ad5e0 M	folderToEdit
 :100644 100644 45f57f3292952208ad72232bc2a64038a3d0987f 59b502537b91543cb8aa7e6a26ee235197fa154b M	folderToEdit/fileToEdit2.txt
+:000000 100644 bbbb404da33605d27cfa5490d412f0dfea16eeee 0000000000000000000000000000000000000000 D	folderToEdit/fileToDelete.txt
+:000000 100644 cccc404da33605d27cfa5490d412f0dfea16ffff 0000000000000000000000000000000000000000 D	folderToEdit/fileToDelete.txt.bak
 :000000 040000 0000000000000000000000000000000000000000 e30358a677e3a55aa838d0bbe0207f61ce40f401 A	folderToRename
 :000000 100644 0000000000000000000000000000000000000000 35151eac3bb089589836bfece4dfbb84fae502de A	folderToRename/existence.txt
 :040000 000000 e30358a677e3a55aa838d0bbe0207f61ce40f401 0000000000000000000000000000000000000000 D	folderWasRenamed

diff --git a/GVFS/GVFS.UnitTests/Prefetch/DiffHelperTests.cs b/GVFS/GVFS.UnitTests/Prefetch/DiffHelperTests.cs
@@ -90,10 +90,10 @@ public void CanParseBackwardsDiff()
             diffBackwards.RequiredBlobs.Count.ShouldEqual(10);
 
             // File added, folder > file, moved folder, added folder
-            diffBackwards.FileDeleteOperations.Count.ShouldEqual(4);
+            diffBackwards.FileDeleteOperations.Count.ShouldEqual(6);
 
             // Also includes, the children of: Folder added, folder renamed, file => folder
-            diffBackwards.TotalFileDeletes.ShouldEqual(7);
+            diffBackwards.TotalFileDeletes.ShouldEqual(9);
 
             // Folder created, folder edited, folder deleted, folder renamed (add + delete),
             // folder => file, file => folder, recursive delete (include subfolder)

diff --git a/Scripts/BuildGVFSForWindows.bat b/Scripts/BuildGVFSForWindows.bat
@@ -1,4 +1,4 @@
-@ECHO OFF
+@if "%_echo%"=="" (echo off) else (echo on)
 SETLOCAL
 setlocal enabledelayedexpansion
 CALL %~dp0\InitializeEnvironment.bat || EXIT /b 10

diff --git a/Scripts/InitializeEnvironment.bat b/Scripts/InitializeEnvironment.bat
@@ -1,4 +1,4 @@
-@ECHO OFF
+@if "%_echo%"=="" (echo off) else (echo on)
 
 REM Set environment variables for interesting paths that scripts might need access to.
 PUSHD %~dp0