From fcf6e52d14bc96a5afd6f040b241af1e54be64c3 Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Wed, 15 Aug 2018 12:42:40 -0400 Subject: [PATCH 1/7] GitStatusCache: fix race condition in test setup This is to fix an issue that happened (intermittently) in the functional tests. There is a race condition where the test is attempting to delete the status cache file, but the status cache might not have been generated for the test repository. To fix this, the test setup will wait for the initial status cache to be generated, so it can proceed from a known state. (cherry picked from commit 97975e4396893a9b44544bfe8a098196008e73b8) --- .../Tests/GitCommands/StatusTests.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/StatusTests.cs b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/StatusTests.cs index a66d691c9..9865431c3 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/GitCommands/StatusTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/GitCommands/StatusTests.cs @@ -98,6 +98,8 @@ public void ModifyingHeadRefInvalidatesCache() private void RepositoryIgnoreTestSetup() { + this.WaitForUpToDateStatusCache(); + string statusCachePath = Path.Combine(this.Enlistment.DotGVFSRoot, "GitStatusCache", "GitStatusCache.dat"); File.Delete(statusCachePath); @@ -110,6 +112,18 @@ private void RepositoryIgnoreTestSetup() this.ValidateGitCommand("status"); } + /// + /// Wait for an up-to-date status cache file to exist on disk. + /// + private void WaitForUpToDateStatusCache() + { + // Run "git status" for the side effect that it will delete any stale status cache file. + this.ValidateGitCommand("status"); + + // Wait for a new status cache to be generated. + this.WaitForStatusCacheToBeGenerated(waitForNewFile: false); + } + private void WaitForStatusCacheToBeGenerated(bool waitForNewFile = true) { string statusCachePath = Path.Combine(this.Enlistment.DotGVFSRoot, "GitStatusCache", "GitStatusCache.dat"); From 65240c42027b9b743450ac84c3cbfb093b64fe14 Mon Sep 17 00:00:00 2001 From: William Baker Date: Thu, 16 Aug 2018 09:11:59 -0700 Subject: [PATCH 2/7] Fix test reliability issues with checks in GetObjectRoot --- .../Tools/GVFSFunctionalTestEnlistment.cs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs index 1326cfd3b..2e369b8b9 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs @@ -115,7 +115,18 @@ public static string GetUniqueEnlistmentRootWithSpaces() public string GetObjectRoot(FileSystemRunner fileSystem) { IEnumerable localCacheRootItems = this.LocalCacheRoot.ShouldBeADirectory(fileSystem).WithItems(); - localCacheRootItems.Count().ShouldEqual(2, "Expected local cache root to contain 2 items. Actual items: " + string.Join(",", localCacheRootItems)); + + FileInfo[] files = localCacheRootItems.Where(info => info is FileInfo).Cast().ToArray(); + files.Where(f => string.Equals(f.Name, "mapping.dat", StringComparison.OrdinalIgnoreCase)) + .Count() + .ShouldEqual(1, "Local cache root should contain a single 'mapping.dat' file, actual files: " + string.Join(",", files)); + + IEnumerable unexpectedFiles = files.Where( + f => !string.Equals(f.Name, "mapping.dat", StringComparison.OrdinalIgnoreCase) && + !string.Equals(f.Name, "mapping.dat.lock", StringComparison.OrdinalIgnoreCase)); + + unexpectedFiles.Any().ShouldBeFalse("Local cache root contains unexpected files: " + string.Join(",", unexpectedFiles)); + DirectoryInfo[] directories = localCacheRootItems.Where(info => info is DirectoryInfo).Cast().ToArray(); directories.Length.ShouldEqual(1, this.LocalCacheRoot + " is expected to have only one folder. Actual: " + directories.Count()); return Path.Combine(directories[0].FullName, "gitObjects"); From 6c679094db744d64893b8bcc7a381ab84494728d Mon Sep 17 00:00:00 2001 From: William Baker Date: Thu, 16 Aug 2018 10:42:27 -0700 Subject: [PATCH 3/7] Simplify checks in GetObjectRoot --- .../Should/FileSystemShouldExtensions.cs | 16 +++++++++++ .../Tools/GVFSFunctionalTestEnlistment.cs | 27 ++++++++++--------- .../Should/EnumerableShouldExtensions.cs | 2 +- 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/GVFS/GVFS.FunctionalTests/Should/FileSystemShouldExtensions.cs b/GVFS/GVFS.FunctionalTests/Should/FileSystemShouldExtensions.cs index 543581e74..449c898a1 100644 --- a/GVFS/GVFS.FunctionalTests/Should/FileSystemShouldExtensions.cs +++ b/GVFS/GVFS.FunctionalTests/Should/FileSystemShouldExtensions.cs @@ -173,6 +173,22 @@ public IEnumerable WithItems() return this.WithItems("*"); } + public IEnumerable WithFiles() + { + IEnumerable items = this.WithItems(); + IEnumerable files = items.Where(info => info is FileInfo).Cast(); + files.Any().ShouldEqual(true, this.Path + " does not have any files. Contents: " + string.Join(",", items)); + return files; + } + + public IEnumerable WithDirectories() + { + IEnumerable items = this.WithItems(); + IEnumerable directories = items.Where(info => info is DirectoryInfo).Cast(); + directories.Any().ShouldEqual(true, this.Path + " does not have any directories. Contents: " + string.Join(",", items)); + return directories; + } + public IEnumerable WithItems(string searchPattern) { DirectoryInfo directory = new DirectoryInfo(this.Path); diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs index 2e369b8b9..37a287d57 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs @@ -114,21 +114,22 @@ public static string GetUniqueEnlistmentRootWithSpaces() public string GetObjectRoot(FileSystemRunner fileSystem) { - IEnumerable localCacheRootItems = this.LocalCacheRoot.ShouldBeADirectory(fileSystem).WithItems(); + Path.Combine(this.LocalCacheRoot, "mapping.dat").ShouldBeAFile(fileSystem); - FileInfo[] files = localCacheRootItems.Where(info => info is FileInfo).Cast().ToArray(); - files.Where(f => string.Equals(f.Name, "mapping.dat", StringComparison.OrdinalIgnoreCase)) - .Count() - .ShouldEqual(1, "Local cache root should contain a single 'mapping.dat' file, actual files: " + string.Join(",", files)); - - IEnumerable unexpectedFiles = files.Where( - f => !string.Equals(f.Name, "mapping.dat", StringComparison.OrdinalIgnoreCase) && - !string.Equals(f.Name, "mapping.dat.lock", StringComparison.OrdinalIgnoreCase)); - - unexpectedFiles.Any().ShouldBeFalse("Local cache root contains unexpected files: " + string.Join(",", unexpectedFiles)); + HashSet allowedFileNames = new HashSet(StringComparer.OrdinalIgnoreCase) + { + "mapping.dat", + "mapping.dat.lock" // mapping.dat.lock can be present, but doesn't have to be present + }; - DirectoryInfo[] directories = localCacheRootItems.Where(info => info is DirectoryInfo).Cast().ToArray(); - directories.Length.ShouldEqual(1, this.LocalCacheRoot + " is expected to have only one folder. Actual: " + directories.Count()); + this.LocalCacheRoot.ShouldBeADirectory(fileSystem).WithFiles().ShouldNotContain(f => !allowedFileNames.Contains(f.Name)); + + + DirectoryInfo[] directories = this.LocalCacheRoot.ShouldBeADirectory(fileSystem).WithDirectories().ToArray(); + directories.Length.ShouldEqual( + 1, + this.LocalCacheRoot + " is expected to have only one folder. Actual folders: " + string.Join(",", directories)); + return Path.Combine(directories[0].FullName, "gitObjects"); } diff --git a/GVFS/GVFS.Tests/Should/EnumerableShouldExtensions.cs b/GVFS/GVFS.Tests/Should/EnumerableShouldExtensions.cs index 4db59a36e..320bc2a81 100644 --- a/GVFS/GVFS.Tests/Should/EnumerableShouldExtensions.cs +++ b/GVFS/GVFS.Tests/Should/EnumerableShouldExtensions.cs @@ -48,7 +48,7 @@ public static T ShouldContainSingle(this IEnumerable group, Func public static void ShouldNotContain(this IEnumerable group, Func predicate) { T item = group.SingleOrDefault(predicate); - item.ShouldEqual(default(T)); + item.ShouldEqual(default(T), "Unexpected matching entry found in {" + string.Join(",", group) + "}"); } public static IEnumerable ShouldNotContain(this IEnumerable group, IEnumerable unexpectedValues, Func predicate) From 2204b377fd9d552f1d51331b8121725703a24eee Mon Sep 17 00:00:00 2001 From: William Baker Date: Thu, 16 Aug 2018 10:55:35 -0700 Subject: [PATCH 4/7] Fix StyleCop error --- GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs index 37a287d57..2d4e39948 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSFunctionalTestEnlistment.cs @@ -124,7 +124,6 @@ public string GetObjectRoot(FileSystemRunner fileSystem) this.LocalCacheRoot.ShouldBeADirectory(fileSystem).WithFiles().ShouldNotContain(f => !allowedFileNames.Contains(f.Name)); - DirectoryInfo[] directories = this.LocalCacheRoot.ShouldBeADirectory(fileSystem).WithDirectories().ToArray(); directories.Length.ShouldEqual( 1, From d95453d2235bd9f825213de51665d7abf5fa1730 Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Mon, 13 Aug 2018 14:30:38 -0400 Subject: [PATCH 5/7] Shutdown GitStatusCache before components it depends on Tweak the order that components are shutdown, to shutdown the GitStatusCache before other components that it depends on. This is to prevent problems that might arise from a component being disabled that GitStatusCache depends on. (cherry picked from commit 0c09ed4dc00709870c5738cbe9a04dfc639413b6) --- GVFS/GVFS.Virtualization/FileSystemCallbacks.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs b/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs index 5ef406b92..2e4419fd3 100644 --- a/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs +++ b/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs @@ -195,8 +195,11 @@ public void Stop() this.postFetchJobThread?.Abort(); } - this.fileSystemVirtualizer.PrepareToStop(); + // Shutdown the GitStatusCache before other + // components that it depends on. this.gitStatusCache.Shutdown(); + + this.fileSystemVirtualizer.PrepareToStop(); this.backgroundFileSystemTaskRunner.Shutdown(); this.GitIndexProjection.Shutdown(); this.BlobSizes.Shutdown(); From 86b66346858faec97c0c9c4b81f61815ca45b43e Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Tue, 14 Aug 2018 10:43:06 -0400 Subject: [PATCH 6/7] GVFSLock.Shared: correctly report whether lock was acquired This fixes a bug in TryAcquireGVFSLockForProcess where it does not correctly report whether it was able to acquire the GVFS lock. In cases where this method retries to acquire the lock, it would report success, even if it was not able to acquire the lock. This could lead to problems where commands that depended on the GVFS could run, even if it did not actually have the lock. (cherry picked from commit dfbe6579623acb650a633663c3212a19b3d15878) --- GVFS/GVFS.Common/GVFSLock.Shared.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/GVFS/GVFS.Common/GVFSLock.Shared.cs b/GVFS/GVFS.Common/GVFSLock.Shared.cs index 4f3fc5d7b..10785b445 100644 --- a/GVFS/GVFS.Common/GVFSLock.Shared.cs +++ b/GVFS/GVFS.Common/GVFSLock.Shared.cs @@ -81,13 +81,14 @@ public static bool TryAcquireGVFSLockForProcess( } }; + bool isSuccessfulLockResult; if (unattended) { - waitForLock(); + isSuccessfulLockResult = waitForLock(); } else { - ConsoleHelper.ShowStatusWhileRunning( + isSuccessfulLockResult = ConsoleHelper.ShowStatusWhileRunning( waitForLock, message, output: Console.Out, @@ -96,7 +97,7 @@ public static bool TryAcquireGVFSLockForProcess( } result = null; - return true; + return isSuccessfulLockResult; } public static void ReleaseGVFSLock( From 53f00adedd011df7a56f2d2142c91c716ae0807b Mon Sep 17 00:00:00 2001 From: Jameson Miller Date: Mon, 20 Aug 2018 16:05:47 -0400 Subject: [PATCH 7/7] Update name of file created by installer to indicate installation version Update name of file that GVFS installer creates when an on disk version 16 capable installation is first installed. (cherry picked from commit 120eeba149a78de3ed616dc5db3e11efc974b48a) --- GVFS/GVFS.Common/GVFSConstants.cs | 5 +++ GVFS/GVFS.Installer/Setup.iss | 14 ++++---- GVFS/GVFS.Service/GvfsService.cs | 53 ++++++++++++++----------------- 3 files changed, 35 insertions(+), 37 deletions(-) diff --git a/GVFS/GVFS.Common/GVFSConstants.cs b/GVFS/GVFS.Common/GVFSConstants.cs index f498773f1..ece894ef8 100644 --- a/GVFS/GVFS.Common/GVFSConstants.cs +++ b/GVFS/GVFS.Common/GVFSConstants.cs @@ -192,6 +192,11 @@ public static class Heads } } + public static class InstallationCapabilityFiles + { + public const string OnDiskVersion16CapableInstallation = "OnDiskVersion16CapableInstallation.dat"; + } + public static class VerbParameters { public static class Mount diff --git a/GVFS/GVFS.Installer/Setup.iss b/GVFS/GVFS.Installer/Setup.iss index 36bff4d3c..f2875b699 100644 --- a/GVFS/GVFS.Installer/Setup.iss +++ b/GVFS/GVFS.Installer/Setup.iss @@ -283,15 +283,15 @@ begin end; end; -procedure WriteGitStatusCacheAvailableFile(); +procedure WriteOnDiskVersion16CapableFile(); var - TokenFilePath: string; + FilePath: string; begin - TokenFilePath := ExpandConstant('{app}\GitStatusCacheAvailable'); - if not FileExists(TokenFilePath) then + FilePath := ExpandConstant('{app}\OnDiskVersion16CapableInstallation.dat'); + if not FileExists(FilePath) then begin - Log('WritingGitStatusCacheAvailableFile: Writing file ' + TokenFilePath); - SaveStringToFile(TokenFilePath, '', False); + Log('WriteOnDiskVersion16CapableFile: Writing file ' + FilePath); + SaveStringToFile(FilePath, '', False); end end; @@ -319,7 +319,7 @@ begin end; end; - WriteGitStatusCacheAvailableFile(); + WriteOnDiskVersion16CapableFile(); finally WizardForm.StatusLabel.Caption := StatusText; WizardForm.ProgressGauge.Style := npbstNormal; diff --git a/GVFS/GVFS.Service/GvfsService.cs b/GVFS/GVFS.Service/GvfsService.cs index bacab0457..0134cd244 100644 --- a/GVFS/GVFS.Service/GvfsService.cs +++ b/GVFS/GVFS.Service/GvfsService.cs @@ -287,28 +287,37 @@ private void CheckEnableGitStatusCacheTokenFile() try { string statusCacheVersionTokenPath = Path.Combine(Paths.GetServiceDataRoot(GVFSConstants.Service.ServiceName), GVFSConstants.GitStatusCache.EnableGitStatusCacheTokenFile); - - if (!File.Exists(statusCacheVersionTokenPath)) + if (File.Exists(statusCacheVersionTokenPath)) { - DateTime lastRebootTime = NativeMethods.GetLastRebootTime(); + this.tracer.RelatedInfo($"CheckEnableGitStatusCache: EnableGitStatusCacheToken file already exists at {statusCacheVersionTokenPath}."); + return; + } + + DateTime lastRebootTime = NativeMethods.GetLastRebootTime(); + + // GitStatusCache was included with GVFS on disk version 16. The 1st time GVFS that is at or above on disk version + // is installed, it will write out a file indicating that the installation is "OnDiskVersion16Capable". + // We can query the properties of this file to get the installation time, and compare this with the last reboot time for + // this machine. + string fileToCheck = Path.Combine(Configuration.AssemblyPath, GVFSConstants.InstallationCapabilityFiles.OnDiskVersion16CapableInstallation); - // When a version of GVFS that supports the GitStatusCache is installed, it will create - // the following file. By checking the time the file was created, we know when that - // version of GVFS was installed. - string fileToCheck = Path.Combine(Configuration.AssemblyPath, "GitStatusCacheAvailable"); - if (File.Exists(fileToCheck)) + if (File.Exists(fileToCheck)) + { + DateTime installTime = File.GetCreationTime(fileToCheck); + if (lastRebootTime > installTime) { - DateTime installTime = File.GetCreationTime(fileToCheck); - if (lastRebootTime > installTime) - { - File.WriteAllText(statusCacheVersionTokenPath, string.Empty); - } + this.tracer.RelatedInfo($"CheckEnableGitStatusCache: Writing out EnableGitStatusCacheToken file. GVFS installation time: {installTime}, last Reboot time: {lastRebootTime}."); + File.WriteAllText(statusCacheVersionTokenPath, string.Empty); } else { - this.tracer.RelatedError($"Unable to determine GVFS installation time: {fileToCheck} does not exist."); + this.tracer.RelatedInfo($"CheckEnableGitStatusCache: Not writing EnableGitStatusCacheToken file - machine has not been rebooted since OnDiskVersion16Capable installation. GVFS installation time: {installTime}, last reboot time: {lastRebootTime}"); } } + else + { + this.tracer.RelatedError($"Unable to determine GVFS installation time: {fileToCheck} does not exist."); + } } catch (Exception ex) { @@ -318,22 +327,6 @@ private void CheckEnableGitStatusCacheTokenFile() } } - private bool TryGetGVFSInstallTime(out DateTime installTime) - { - installTime = DateTime.Now; - - // Get the time of a file that was created by the GVFS installer (for a version of GVFS that supports the - // GitStatusCache). The expected path is written by the installer. - string fileToCheck = Path.Combine(Configuration.AssemblyPath, "GitStatusCacheAvailable"); - if (File.Exists(fileToCheck)) - { - installTime = File.GetCreationTime(fileToCheck); - return true; - } - - return false; - } - private void LogExceptionAndExit(Exception e, string method) { EventMetadata metadata = new EventMetadata();