From 1af261720a9440f5a5ee0e100e9b7072a8a35e17 Mon Sep 17 00:00:00 2001 From: William Baker Date: Tue, 9 Oct 2018 15:18:07 -0700 Subject: [PATCH 1/4] Mac: Handle projection changes where git deletes folders that are still in the projection --- .../Tests/GitCommands/CheckoutTests.cs | 1 - .../Projection/GitIndexProjection.cs | 57 +++++++++++++++---- ProjFS.Mac/PrjFSLib/PrjFSLib.cpp | 25 ++++++-- 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs index 8457720bb..351e6a7ef 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs @@ -892,7 +892,6 @@ public void CheckoutBranchDirectoryWithOneFileRead() } [TestCase] - [Category(Categories.MacTODO.M3)] public void CheckoutBranchDirectoryWithOneFileWrite() { this.RunFileDirectoryWriteTest("checkout", commandBranch: GitRepoTests.DirectoryWithDifferentFileAfterBranch); diff --git a/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs b/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs index c99808b74..49384e3bc 100644 --- a/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs +++ b/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs @@ -1158,8 +1158,11 @@ private void UpdatePlaceholders() new HashSet(placeholderFoldersListCopy.Select(x => x.Path), StringComparer.OrdinalIgnoreCase) : null; - // Order the folders in decscending order so that we walk the tree from bottom up (ensuring child folders are deleted before - // their parents) + // Order the folders in decscending order so that we walk the tree from bottom up. + // Traversing the folders in this order: + // 1. Ensures child folders are deleted before their parents + // 2. Ensures that folders that have been deleted by git (but are still in the projection) are found before their + // parent folder is re-expanded (only applies on platforms where EnumerationExpandsDirectories is true) foreach (PlaceholderListDatabase.PlaceholderData folderPlaceholder in placeholderFoldersListCopy.OrderByDescending(x => x.Path)) { // Remove folder placeholders before re-expansion to ensure that projection changes that convert a folder to a file work @@ -1388,15 +1391,31 @@ private void ReExpandFolder( childRelativePath = relativeFolderPath + Path.DirectorySeparatorChar + childEntry.Name.GetString(); } - // TODO(Mac): Issue #245, handle failures of WritePlaceholderDirectory and WritePlaceholderFile if (childEntry.IsFolder) { if (!existingFolderPlaceholders.Contains(childRelativePath)) { - this.fileSystemVirtualizer.WritePlaceholderDirectory(childRelativePath); - updatedPlaceholderList.TryAdd( - childRelativePath, - new PlaceholderListDatabase.PlaceholderData(childRelativePath, PlaceholderListDatabase.PartialFolderValue)); + FileSystemResult result = this.fileSystemVirtualizer.WritePlaceholderDirectory(childRelativePath); + switch (result.Result) + { + case FSResult.Ok: + updatedPlaceholderList.TryAdd( + childRelativePath, + new PlaceholderListDatabase.PlaceholderData(childRelativePath, PlaceholderListDatabase.PartialFolderValue)); + break; + + case FSResult.FileOrPathNotFound: + // Git command must have removed the folder being re-expanded (relativeFolderPath) + // Remove the folder from existingFolderPlaceholders so that its parent will create + // it again (when it's re-expanded) + existingFolderPlaceholders.Remove(relativeFolderPath); + return; + + default: + // TODO(Mac): Issue #245, handle failures of WritePlaceholderDirectory and WritePlaceholderFile + break; + } + } } else @@ -1406,10 +1425,26 @@ private void ReExpandFolder( FileData childFileData = childEntry as FileData; string sha = childFileData.Sha.ToString(); - this.fileSystemVirtualizer.WritePlaceholderFile(childRelativePath, childFileData.Size, sha); - updatedPlaceholderList.TryAdd( - childRelativePath, - new PlaceholderListDatabase.PlaceholderData(childRelativePath, sha)); + FileSystemResult result = this.fileSystemVirtualizer.WritePlaceholderFile(childRelativePath, childFileData.Size, sha); + switch (result.Result) + { + case FSResult.Ok: + updatedPlaceholderList.TryAdd( + childRelativePath, + new PlaceholderListDatabase.PlaceholderData(childRelativePath, sha)); + break; + + case FSResult.FileOrPathNotFound: + // Git command must have removed the folder being re-expanded (relativeFolderPath) + // Remove the folder from existingFolderPlaceholders so that its parent will create + // it again (when it's re-expanded) + existingFolderPlaceholders.Remove(relativeFolderPath); + return; + + default: + // TODO(Mac): Issue #245, handle failures of WritePlaceholderDirectory and WritePlaceholderFile + break; + } } } } diff --git a/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp b/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp index 132ccd383..2bd6636d0 100644 --- a/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp +++ b/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp @@ -323,23 +323,36 @@ PrjFS_Result PrjFS_WritePlaceholderFile( return PrjFS_Result_EInvalidArgs; } + PrjFS_Result result = PrjFS_Result_Invalid; PrjFSFileXAttrData fileXattrData = {}; char fullPath[PrjFSMaxPath]; CombinePaths(s_virtualizationRootFullPath.c_str(), relativePath, fullPath); - // Mode "wbx" means - // - Create an empty file if none exists - // - Fail if a file already exists at this path - FILE* file = fopen(fullPath, "wbx"); + // Mode "wx" means: + // - "w": Open for writing. The stream is positioned at the beginning of the file. Create the file if it does not exist. + // - "x": If the file already exists, fopen() fails, and sets errno to EEXIST. + FILE* file = fopen(fullPath, "wx"); if (nullptr == file) { + switch(errno) + { + case ENOENT: // A directory component in fullPath does not exist or is a dangling symbolic link. + result = PrjFS_Result_EPathNotFound; + break; + case EEXIST: // The file already exists + default: + result = PrjFS_Result_EIOError; + break; + } + goto CleanupAndFail; } // Expand the file to the desired size if (ftruncate(fileno(file), fileSize)) { + result = PrjFS_Result_EIOError; goto CleanupAndFail; } @@ -354,12 +367,14 @@ PrjFS_Result PrjFS_WritePlaceholderFile( &fileXattrData, PrjFSFileXAttrName)) { + result = PrjFS_Result_EIOError; goto CleanupAndFail; } // TODO(Mac): Only call chmod if fileMode is different than the default file mode if (chmod(fullPath, fileMode)) { + result = PrjFS_Result_EIOError; goto CleanupAndFail; } @@ -375,7 +390,7 @@ PrjFS_Result PrjFS_WritePlaceholderFile( file = nullptr; } - return PrjFS_Result_EIOError; + return result; } PrjFS_Result PrjFS_WriteSymLink( From 5d62ef31e56f985029515b48de9676337f136eab Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 10 Oct 2018 10:16:59 -0700 Subject: [PATCH 2/4] Fix StyleCop issue --- GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs b/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs index 49384e3bc..a1a690986 100644 --- a/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs +++ b/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs @@ -1415,7 +1415,6 @@ private void ReExpandFolder( // TODO(Mac): Issue #245, handle failures of WritePlaceholderDirectory and WritePlaceholderFile break; } - } } else From d57e79157b62025d0315f95848f3c65ca00c3b4e Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 10 Oct 2018 15:07:58 -0700 Subject: [PATCH 3/4] Add additional functional test and update PrjFS_WritePlaceholderDirectory to return a more specific error message --- .../Tests/GitCommands/CheckoutTests.cs | 47 +++++++++++++++++++ .../Tests/GitCommands/GitRepoTests.cs | 2 + ProjFS.Mac/PrjFSLib/PrjFSLib.cpp | 16 ++++++- 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs index 351e6a7ef..ac861a982 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs @@ -897,6 +897,53 @@ public void CheckoutBranchDirectoryWithOneFileWrite() this.RunFileDirectoryWriteTest("checkout", commandBranch: GitRepoTests.DirectoryWithDifferentFileAfterBranch); } + [TestCase] + public void CheckoutBranchDirectoryWithOneDeepFileWrite() + { + this.ControlGitRepo.Fetch(DeepDirectoryWithOneFile); + this.ControlGitRepo.Fetch(DeepDirectoryWithOneDifferentFile); + this.ValidateGitCommand($"checkout {DeepDirectoryWithOneFile}"); + this.FileShouldHaveContents( + "TestFile1\n", + "GitCommandsTests", + "CheckoutBranchDirectoryWithOneDeepFile", + "FolderDepth1", + "FolderDepth2", + "FolderDepth3", + "File1.txt"); + + // Edit the file and commit the change so that git will + // delete the file (and its parent directories) when + // changing branches + this.EditFile( + "Change file", + "GitCommandsTests", + "CheckoutBranchDirectoryWithOneDeepFile", + "FolderDepth1", + "FolderDepth2", + "FolderDepth3", + "File1.txt"); + this.ValidateGitCommand("add --all"); + this.RunGitCommand("commit -m \"Some change\""); + + this.ValidateGitCommand($"checkout {DeepDirectoryWithOneDifferentFile}"); + this.FileShouldHaveContents( + "TestFile2\n", + "GitCommandsTests", + "CheckoutBranchDirectoryWithOneDeepFile", + "FolderDepth1", + "FolderDepth2", + "FolderDepth3", + "File2.txt"); + this.ShouldNotExistOnDisk( + "GitCommandsTests", + "CheckoutBranchDirectoryWithOneDeepFile", + "FolderDepth1", + "FolderDepth2", + "FolderDepth3", + "File1.txt"); + } + private static void CopyIndexAndRename(string indexPath) { string tempIndexPath = indexPath + ".lock"; diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs index 9200be7c5..d68e38095 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs @@ -19,6 +19,8 @@ public abstract class GitRepoTests protected const string DirectoryWithFileBeforeBranch = "FunctionalTests/20171025_DirectoryWithFileBefore"; protected const string DirectoryWithFileAfterBranch = "FunctionalTests/20171025_DirectoryWithFileAfter"; protected const string DirectoryWithDifferentFileAfterBranch = "FunctionalTests/20171025_DirectoryWithDifferentFile"; + protected const string DeepDirectoryWithOneFile = "FunctionalTests/20181010_DeepFolderOneFile"; + protected const string DeepDirectoryWithOneDifferentFile = "FunctionalTests/20181010_DeepFolderOneDifferentFile"; private bool enlistmentPerTest; diff --git a/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp b/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp index 2bd6636d0..cddef78bb 100644 --- a/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp +++ b/ProjFS.Mac/PrjFSLib/PrjFSLib.cpp @@ -281,16 +281,29 @@ PrjFS_Result PrjFS_WritePlaceholderDirectory( return PrjFS_Result_EInvalidArgs; } + PrjFS_Result result = PrjFS_Result_Invalid; char fullPath[PrjFSMaxPath]; CombinePaths(s_virtualizationRootFullPath.c_str(), relativePath, fullPath); if (mkdir(fullPath, 0777)) { + switch(errno) + { + // TODO(Mac): Return more specific error codes for other failure scenarios + case ENOENT: // A component of the path prefix does not exist or path is an empty string + result = PrjFS_Result_EPathNotFound; + break; + default: + result = PrjFS_Result_EIOError; + break; + } + goto CleanupAndFail; } if (!InitializeEmptyPlaceholder(fullPath)) { + result = PrjFS_Result_EIOError; goto CleanupAndFail; } @@ -298,7 +311,7 @@ PrjFS_Result PrjFS_WritePlaceholderDirectory( CleanupAndFail: // TODO: cleanup the directory on disk if needed - return PrjFS_Result_EIOError; + return result; } PrjFS_Result PrjFS_WritePlaceholderFile( @@ -337,6 +350,7 @@ PrjFS_Result PrjFS_WritePlaceholderFile( { switch(errno) { + // TODO(Mac): Return more specific error codes for other failure scenarios case ENOENT: // A directory component in fullPath does not exist or is a dangling symbolic link. result = PrjFS_Result_EPathNotFound; break; From 74743c0198d6bee78df8a1a2f8467072449af524 Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 10 Oct 2018 15:35:54 -0700 Subject: [PATCH 4/4] Code cleanup --- .../Tests/GitCommands/CheckoutTests.cs | 8 +- .../Projection/GitIndexProjection.cs | 75 +++++++------------ 2 files changed, 33 insertions(+), 50 deletions(-) diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs index ac861a982..eb0491296 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/CheckoutTests.cs @@ -900,9 +900,9 @@ public void CheckoutBranchDirectoryWithOneFileWrite() [TestCase] public void CheckoutBranchDirectoryWithOneDeepFileWrite() { - this.ControlGitRepo.Fetch(DeepDirectoryWithOneFile); - this.ControlGitRepo.Fetch(DeepDirectoryWithOneDifferentFile); - this.ValidateGitCommand($"checkout {DeepDirectoryWithOneFile}"); + this.ControlGitRepo.Fetch(GitRepoTests.DeepDirectoryWithOneFile); + this.ControlGitRepo.Fetch(GitRepoTests.DeepDirectoryWithOneDifferentFile); + this.ValidateGitCommand($"checkout {GitRepoTests.DeepDirectoryWithOneFile}"); this.FileShouldHaveContents( "TestFile1\n", "GitCommandsTests", @@ -926,7 +926,7 @@ public void CheckoutBranchDirectoryWithOneDeepFileWrite() this.ValidateGitCommand("add --all"); this.RunGitCommand("commit -m \"Some change\""); - this.ValidateGitCommand($"checkout {DeepDirectoryWithOneDifferentFile}"); + this.ValidateGitCommand($"checkout {GitRepoTests.DeepDirectoryWithOneDifferentFile}"); this.FileShouldHaveContents( "TestFile2\n", "GitCommandsTests", diff --git a/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs b/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs index a1a690986..fb8edba06 100644 --- a/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs +++ b/GVFS/GVFS.Virtualization/Projection/GitIndexProjection.cs @@ -1391,59 +1391,42 @@ private void ReExpandFolder( childRelativePath = relativeFolderPath + Path.DirectorySeparatorChar + childEntry.Name.GetString(); } - if (childEntry.IsFolder) + bool newChild = childEntry.IsFolder ? !existingFolderPlaceholders.Contains(childRelativePath) : !updatedPlaceholderList.ContainsKey(childRelativePath); + + if (newChild) { - if (!existingFolderPlaceholders.Contains(childRelativePath)) + FileSystemResult result; + string fileShaOrFolderValue; + if (childEntry.IsFolder) { - FileSystemResult result = this.fileSystemVirtualizer.WritePlaceholderDirectory(childRelativePath); - switch (result.Result) - { - case FSResult.Ok: - updatedPlaceholderList.TryAdd( - childRelativePath, - new PlaceholderListDatabase.PlaceholderData(childRelativePath, PlaceholderListDatabase.PartialFolderValue)); - break; - - case FSResult.FileOrPathNotFound: - // Git command must have removed the folder being re-expanded (relativeFolderPath) - // Remove the folder from existingFolderPlaceholders so that its parent will create - // it again (when it's re-expanded) - existingFolderPlaceholders.Remove(relativeFolderPath); - return; - - default: - // TODO(Mac): Issue #245, handle failures of WritePlaceholderDirectory and WritePlaceholderFile - break; - } + fileShaOrFolderValue = PlaceholderListDatabase.PartialFolderValue; + result = this.fileSystemVirtualizer.WritePlaceholderDirectory(childRelativePath); } - } - else - { - if (!updatedPlaceholderList.ContainsKey(childRelativePath)) + else { FileData childFileData = childEntry as FileData; - string sha = childFileData.Sha.ToString(); + fileShaOrFolderValue = childFileData.Sha.ToString(); + result = this.fileSystemVirtualizer.WritePlaceholderFile(childRelativePath, childFileData.Size, fileShaOrFolderValue); + } - FileSystemResult result = this.fileSystemVirtualizer.WritePlaceholderFile(childRelativePath, childFileData.Size, sha); - switch (result.Result) - { - case FSResult.Ok: - updatedPlaceholderList.TryAdd( - childRelativePath, - new PlaceholderListDatabase.PlaceholderData(childRelativePath, sha)); - break; - - case FSResult.FileOrPathNotFound: - // Git command must have removed the folder being re-expanded (relativeFolderPath) - // Remove the folder from existingFolderPlaceholders so that its parent will create - // it again (when it's re-expanded) - existingFolderPlaceholders.Remove(relativeFolderPath); - return; + switch (result.Result) + { + case FSResult.Ok: + updatedPlaceholderList.TryAdd( + childRelativePath, + new PlaceholderListDatabase.PlaceholderData(childRelativePath, fileShaOrFolderValue)); + break; - default: - // TODO(Mac): Issue #245, handle failures of WritePlaceholderDirectory and WritePlaceholderFile - break; - } + case FSResult.FileOrPathNotFound: + // Git command must have removed the folder being re-expanded (relativeFolderPath) + // Remove the folder from existingFolderPlaceholders so that its parent will create + // it again (when it's re-expanded) + existingFolderPlaceholders.Remove(relativeFolderPath); + return; + + default: + // TODO(Mac): Issue #245, handle failures of WritePlaceholderDirectory and WritePlaceholderFile + break; } } }