From f6fbfc415538c007bd9b6a266c356d72f9c1996f Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Thu, 24 Sep 2026 12:03:54 -0700 Subject: [PATCH 1/4] Improve missing disk layout metadata error When the disk layout version is missing from .gvfs metadata, report that the metadata is incomplete and tell the user to reclone the enlistment. Return a distinct mount exit code for that case so automation can identify it. Keep generic upgrade failures on the existing error path. Cover both a missing layout key and a missing repo metadata file in mount functional tests. Assisted-by: GitHub Copilot Signed-off-by: Tyrie Vella --- .../DiskLayoutUpgrades/DiskLayoutUpgrade.cs | 64 +++++++++++++++++-- GVFS/GVFS.Common/RepoMetadata.cs | 15 ++++- GVFS/GVFS.Common/ReturnCode.cs | 1 + .../Tests/EnlistmentPerFixture/MountTests.cs | 34 +++++++++- .../GVFS.FunctionalTests/Tools/GVFSHelpers.cs | 39 ++++++++++- GVFS/GVFS/CommandLine/MountVerb.cs | 16 +++-- 6 files changed, 155 insertions(+), 14 deletions(-) diff --git a/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs b/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs index d268191ab..dcb0668fe 100644 --- a/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs +++ b/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs @@ -17,9 +17,18 @@ public abstract class DiskLayoutUpgrade protected abstract bool IsMajorUpgrade { get; } public static bool TryRunAllUpgrades(string enlistmentRoot) + { + string error; + ReturnCode returnCode; + return TryRunAllUpgrades(enlistmentRoot, out error, out returnCode); + } + + public static bool TryRunAllUpgrades(string enlistmentRoot, out string error, out ReturnCode returnCode) { majorVersionUpgrades = new Dictionary(); minorVersionUpgrades = new Dictionary>(); + error = null; + returnCode = ReturnCode.Success; foreach (DiskLayoutUpgrade upgrade in GVFSPlatform.Instance.DiskLayoutUpgrade.Upgrades) { @@ -31,7 +40,7 @@ public static bool TryRunAllUpgrades(string enlistmentRoot) try { DiskLayoutUpgrade upgrade = null; - while (TryFindUpgrade(tracer, enlistmentRoot, out upgrade)) + while (TryFindUpgrade(tracer, enlistmentRoot, out upgrade, out error, out returnCode)) { if (upgrade == null) { @@ -40,11 +49,13 @@ public static bool TryRunAllUpgrades(string enlistmentRoot) if (!upgrade.TryUpgrade(tracer, enlistmentRoot)) { + returnCode = ReturnCode.GenericError; return false; } if (!CheckLayoutVersionWasIncremented(tracer, enlistmentRoot, upgrade)) { + returnCode = ReturnCode.GenericError; return false; } } @@ -55,6 +66,8 @@ public static bool TryRunAllUpgrades(string enlistmentRoot) { StartLogFile(enlistmentRoot, tracer); tracer.RelatedError(e.ToString()); + error = e.ToString(); + returnCode = ReturnCode.GenericError; return false; } finally @@ -65,8 +78,15 @@ public static bool TryRunAllUpgrades(string enlistmentRoot) } public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRoot, out string error) + { + ReturnCode returnCode; + return TryCheckDiskLayoutVersion(tracer, enlistmentRoot, out error, out returnCode); + } + + public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRoot, out string error, out ReturnCode returnCode) { error = string.Empty; + returnCode = ReturnCode.Success; int majorVersion; int minorVersion; try @@ -75,6 +95,7 @@ public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRo { if (majorVersion < GVFSPlatform.Instance.DiskLayoutUpgrade.Version.MinimumSupportedMajorVersion) { + returnCode = ReturnCode.GenericError; error = string.Format( "Breaking change to GVFS disk layout has been made since cloning. \r\nEnlistment disk layout version: {0} \r\nGVFS disk layout version: {1} \r\nMinimum supported version: {2}", majorVersion, @@ -85,6 +106,7 @@ public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRo } else if (majorVersion > GVFSPlatform.Instance.DiskLayoutUpgrade.Version.CurrentMajorVersion) { + returnCode = ReturnCode.GenericError; error = string.Format( "Changes to GVFS disk layout do not allow mounting after downgrade. Try mounting again using a more recent version of GVFS. \r\nEnlistment disk layout version: {0} \r\nGVFS disk layout version: {1}", majorVersion, @@ -94,6 +116,7 @@ public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRo } else if (majorVersion != GVFSPlatform.Instance.DiskLayoutUpgrade.Version.CurrentMajorVersion) { + returnCode = ReturnCode.GenericError; error = string.Format( "GVFS disk layout version doesn't match current version. Try running 'gvfs mount' to upgrade. \r\nEnlistment disk layout version: {0}.{1} \r\nGVFS disk layout version: {2}.{3}", majorVersion, @@ -112,7 +135,16 @@ public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRo RepoMetadata.Shutdown(); } - error = "Failed to read disk layout version. " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot); + if (error == RepoMetadata.MissingDiskLayoutVersionMessage) + { + returnCode = ReturnCode.MissingDiskLayoutVersion; + } + else + { + returnCode = ReturnCode.GenericError; + error = "Failed to read disk layout version. " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot); + } + return false; } @@ -257,12 +289,18 @@ private static bool CheckLayoutVersionWasIncremented(JsonTracer tracer, string e } private static bool TryFindUpgrade(JsonTracer tracer, string enlistmentRoot, out DiskLayoutUpgrade upgrade) + { + string error; + ReturnCode returnCode; + return TryFindUpgrade(tracer, enlistmentRoot, out upgrade, out error, out returnCode); + } + + private static bool TryFindUpgrade(JsonTracer tracer, string enlistmentRoot, out DiskLayoutUpgrade upgrade, out string error, out ReturnCode returnCode) { int majorVersion; int minorVersion; - string error; - if (!TryGetDiskLayoutVersion(tracer, enlistmentRoot, out majorVersion, out minorVersion, out error)) + if (!TryGetDiskLayoutVersion(tracer, enlistmentRoot, out majorVersion, out minorVersion, out error, out returnCode)) { StartLogFile(enlistmentRoot, tracer); tracer.RelatedError(error); @@ -300,6 +338,8 @@ private static bool TryFindUpgrade(JsonTracer tracer, string enlistmentRoot, out // return true to indicate that we succeeded, and no upgrader was found upgrade = null; + error = null; + returnCode = ReturnCode.Success; return true; } @@ -309,9 +349,22 @@ private static bool TryGetDiskLayoutVersion( out int majorVersion, out int minorVersion, out string error) + { + ReturnCode returnCode; + return TryGetDiskLayoutVersion(tracer, enlistmentRoot, out majorVersion, out minorVersion, out error, out returnCode); + } + + private static bool TryGetDiskLayoutVersion( + ITracer tracer, + string enlistmentRoot, + out int majorVersion, + out int minorVersion, + out string error, + out ReturnCode returnCode) { majorVersion = 0; minorVersion = 0; + returnCode = ReturnCode.GenericError; string dotGVFSPath = Path.Combine(enlistmentRoot, GVFSPlatform.Instance.Constants.DotGVFSRoot); @@ -323,13 +376,14 @@ private static bool TryGetDiskLayoutVersion( return false; } - if (!RepoMetadata.Instance.TryGetOnDiskLayoutVersion(out majorVersion, out minorVersion, out error)) + if (!RepoMetadata.Instance.TryGetOnDiskLayoutVersion(out majorVersion, out minorVersion, out error, out returnCode)) { return false; } } error = null; + returnCode = ReturnCode.Success; return true; } diff --git a/GVFS/GVFS.Common/RepoMetadata.cs b/GVFS/GVFS.Common/RepoMetadata.cs index ea3d3ebf1..d7475a628 100644 --- a/GVFS/GVFS.Common/RepoMetadata.cs +++ b/GVFS/GVFS.Common/RepoMetadata.cs @@ -1,4 +1,4 @@ -using GVFS.Common.FileSystem; +using GVFS.Common.FileSystem; using GVFS.Common.Tracing; using System; using System.Collections.Generic; @@ -8,6 +8,8 @@ namespace GVFS.Common { public class RepoMetadata { + public const string MissingDiskLayoutVersionMessage = "Enlistment disk layout version not found. The .gvfs metadata is incomplete. Recloning this enlistment is the recommended fix."; + private FileBasedDictionary repoMetadata; private ITracer tracer; @@ -90,16 +92,24 @@ public static void Shutdown() } public bool TryGetOnDiskLayoutVersion(out int majorVersion, out int minorVersion, out string error) + { + ReturnCode returnCode; + return this.TryGetOnDiskLayoutVersion(out majorVersion, out minorVersion, out error, out returnCode); + } + + public bool TryGetOnDiskLayoutVersion(out int majorVersion, out int minorVersion, out string error, out ReturnCode returnCode) { majorVersion = 0; minorVersion = 0; + returnCode = ReturnCode.GenericError; try { string value; if (!this.repoMetadata.TryGetValue(Keys.DiskLayoutMajorVersion, out value)) { - error = "Enlistment disk layout version not found, check if a breaking change has been made to GVFS since cloning this enlistment."; + error = MissingDiskLayoutVersionMessage; + returnCode = ReturnCode.MissingDiskLayoutVersion; return false; } @@ -125,6 +135,7 @@ public bool TryGetOnDiskLayoutVersion(out int majorVersion, out int minorVersion } error = null; + returnCode = ReturnCode.Success; return true; } diff --git a/GVFS/GVFS.Common/ReturnCode.cs b/GVFS/GVFS.Common/ReturnCode.cs index 31842f396..4c5347cc6 100644 --- a/GVFS/GVFS.Common/ReturnCode.cs +++ b/GVFS/GVFS.Common/ReturnCode.cs @@ -14,5 +14,6 @@ public enum ReturnCode AuthenticationError = 9, CredentialTimeout = 10, RemoteGvfsConfigError = 11, + MissingDiskLayoutVersion = 12, } } diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs index 576534d1c..429d123c9 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs @@ -1,4 +1,4 @@ -using GVFS.FunctionalTests.FileSystemRunners; +using GVFS.FunctionalTests.FileSystemRunners; using GVFS.FunctionalTests.Properties; using GVFS.FunctionalTests.Should; using GVFS.FunctionalTests.Tools; @@ -18,6 +18,7 @@ namespace GVFS.FunctionalTests.Tests.EnlistmentPerFixture public class MountTests : TestsWithEnlistmentPerFixture { private const int GVFSGenericError = 3; + private const int GVFSMissingDiskLayoutVersion = 12; private const uint GenericRead = 2147483648; private const uint FileFlagBackupSemantics = 3355443; private readonly int fileDeletedBackgroundOperationCode; @@ -99,6 +100,33 @@ public void MountChangesMountId() .ShouldNotEqual(mountId, "gvfs.mount-id should change on every mount"); } + [TestCase] + public void MountFailsWithUniqueExitCodeWhenDiskLayoutVersionIsMissing() + { + this.Enlistment.UnmountGVFS(); + + string majorVersion; + string minorVersion; + GVFSHelpers.GetPersistedDiskLayoutVersion(this.Enlistment.DotGVFSRoot, out majorVersion, out minorVersion); + majorVersion.ShouldNotBeNull(); + minorVersion.ShouldNotBeNull(); + + try + { + GVFSHelpers.DeletePersistedDiskLayoutMajorVersion(this.Enlistment.DotGVFSRoot); + + this.MountShouldFail( + GVFSMissingDiskLayoutVersion, + "The .gvfs metadata is incomplete. Recloning this enlistment is the recommended fix."); + } + finally + { + GVFSHelpers.SaveDiskLayoutVersion(this.Enlistment.DotGVFSRoot, majorVersion, minorVersion); + } + + this.Enlistment.MountGVFS(); + } + [TestCase] public void MountFailsWhenNoOnDiskVersion() { @@ -126,7 +154,9 @@ public void MountFailsWhenNoOnDiskVersion() this.fileSystem.MoveFile(versionDatabasePath, tempDatabasePath); versionDatabasePath.ShouldNotExistOnDisk(this.fileSystem); - this.MountShouldFail("Failed to upgrade repo disk layout"); + this.MountShouldFail( + GVFSMissingDiskLayoutVersion, + "The .gvfs metadata is incomplete. Recloning this enlistment is the recommended fix."); } finally { diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs index d1b356f3d..ea0bdae6d 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs @@ -1,4 +1,4 @@ -using GVFS.Common; +using GVFS.Common; using GVFS.FunctionalTests.FileSystemRunners; using GVFS.FunctionalTests.Should; using GVFS.Tests.Should; @@ -50,6 +50,11 @@ public static void SaveDiskLayoutVersion(string dotGVFSRoot, string majorVersion SavePersistedValue(dotGVFSRoot, DiskLayoutMinorVersionKey, minorVersion); } + public static void DeletePersistedDiskLayoutMajorVersion(string dotGVFSRoot) + { + DeletePersistedValue(dotGVFSRoot, DiskLayoutMajorVersionKey); + } + public static void GetPersistedDiskLayoutVersion(string dotGVFSRoot, out string majorVersion, out string minorVersion) { majorVersion = GetPersistedValue(dotGVFSRoot, DiskLayoutMajorVersionKey); @@ -366,6 +371,38 @@ private static void SavePersistedValue(string dotGVFSRoot, string key, string va File.WriteAllText(metadataPath, newRepoMetadataContents); } + private static void DeletePersistedValue(string dotGVFSRoot, string key) + { + string metadataPath = Path.Combine(dotGVFSRoot, RepoMetadataName); + + Dictionary repoMetadata = new Dictionary(); + string json; + using (FileStream fs = new FileStream(metadataPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete)) + using (StreamReader reader = new StreamReader(fs)) + { + while (!reader.EndOfStream) + { + json = reader.ReadLine(); + json.Substring(0, 2).ShouldEqual("A "); + + KeyValuePair kvp = GVFSJsonOptions.Deserialize>(json.Substring(2)); + if (kvp.Key != key) + { + repoMetadata[kvp.Key] = kvp.Value; + } + } + } + + string newRepoMetadataContents = string.Empty; + + foreach (KeyValuePair kvp in repoMetadata) + { + newRepoMetadataContents += "A " + GVFSJsonOptions.Serialize(kvp).Trim() + "\r\n"; + } + + File.WriteAllText(metadataPath, newRepoMetadataContents); + } + [DllImport(PrjFSLibPath, EntryPoint = "PrjFS_RegisterForOfflineIO")] private static extern uint PrjFSRegisterForOfflineIO(); diff --git a/GVFS/GVFS/CommandLine/MountVerb.cs b/GVFS/GVFS/CommandLine/MountVerb.cs index 5ea8c5781..4a38b37dc 100644 --- a/GVFS/GVFS/CommandLine/MountVerb.cs +++ b/GVFS/GVFS/CommandLine/MountVerb.cs @@ -1,4 +1,4 @@ -using GVFS.Common; +using GVFS.Common; using GVFS.Common.Http; using GVFS.Common.NamedPipes; using GVFS.Common.Tracing; @@ -130,15 +130,23 @@ protected override void PreCreateEnlistment() } } - if (!DiskLayoutUpgrade.TryRunAllUpgrades(enlistmentRoot)) + string diskLayoutError; + ReturnCode diskLayoutReturnCode; + if (!DiskLayoutUpgrade.TryRunAllUpgrades(enlistmentRoot, out diskLayoutError, out diskLayoutReturnCode)) { + if (diskLayoutReturnCode == ReturnCode.MissingDiskLayoutVersion) + { + this.ReportErrorAndExit(tracer: null, exitCode: diskLayoutReturnCode, error: "Error: " + diskLayoutError); + } + this.ReportErrorAndExit("Failed to upgrade repo disk layout. " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot)); } string error; - if (!DiskLayoutUpgrade.TryCheckDiskLayoutVersion(tracer: null, enlistmentRoot: enlistmentRoot, error: out error)) + ReturnCode returnCode; + if (!DiskLayoutUpgrade.TryCheckDiskLayoutVersion(tracer: null, enlistmentRoot: enlistmentRoot, error: out error, returnCode: out returnCode)) { - this.ReportErrorAndExit("Error: " + error); + this.ReportErrorAndExit(tracer: null, exitCode: returnCode, error: "Error: " + error); } } From 881644ee58c2b474405f50cb3231cfd0f6a974ba Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Fri, 25 Sep 2026 16:52:58 -0700 Subject: [PATCH 2/4] Address missing layout review feedback Use structured return codes throughout disk layout version checks instead of deriving the missing-layout case from message text. Propagate the new return code through repair and dehydrate as well as mount. Keep functional tests tied to the production return code and message constants, and share repo metadata read/write helper logic in test utilities. Assisted-by: GitHub Copilot Signed-off-by: Tyrie Vella --- .../DiskLayoutUpgrades/DiskLayoutUpgrade.cs | 15 +--- GVFS/GVFS.Common/RepoMetadata.cs | 2 +- .../Tests/EnlistmentPerFixture/MountTests.cs | 12 ++-- .../GVFS.FunctionalTests/Tools/GVFSHelpers.cs | 72 ++++++------------- GVFS/GVFS/CommandLine/DehydrateVerb.cs | 5 +- GVFS/GVFS/CommandLine/MountVerb.cs | 2 +- GVFS/GVFS/CommandLine/RepairVerb.cs | 5 +- 7 files changed, 38 insertions(+), 75 deletions(-) diff --git a/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs b/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs index dcb0668fe..a9a6e82c1 100644 --- a/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs +++ b/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs @@ -16,13 +16,6 @@ public abstract class DiskLayoutUpgrade protected abstract int SourceMinorVersion { get; } protected abstract bool IsMajorUpgrade { get; } - public static bool TryRunAllUpgrades(string enlistmentRoot) - { - string error; - ReturnCode returnCode; - return TryRunAllUpgrades(enlistmentRoot, out error, out returnCode); - } - public static bool TryRunAllUpgrades(string enlistmentRoot, out string error, out ReturnCode returnCode) { majorVersionUpgrades = new Dictionary(); @@ -91,7 +84,7 @@ public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRo int minorVersion; try { - if (TryGetDiskLayoutVersion(tracer, enlistmentRoot, out majorVersion, out minorVersion, out error)) + if (TryGetDiskLayoutVersion(tracer, enlistmentRoot, out majorVersion, out minorVersion, out error, out returnCode)) { if (majorVersion < GVFSPlatform.Instance.DiskLayoutUpgrade.Version.MinimumSupportedMajorVersion) { @@ -135,11 +128,7 @@ public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRo RepoMetadata.Shutdown(); } - if (error == RepoMetadata.MissingDiskLayoutVersionMessage) - { - returnCode = ReturnCode.MissingDiskLayoutVersion; - } - else + if (returnCode != ReturnCode.MissingDiskLayoutVersion) { returnCode = ReturnCode.GenericError; error = "Failed to read disk layout version. " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot); diff --git a/GVFS/GVFS.Common/RepoMetadata.cs b/GVFS/GVFS.Common/RepoMetadata.cs index d7475a628..f3213903b 100644 --- a/GVFS/GVFS.Common/RepoMetadata.cs +++ b/GVFS/GVFS.Common/RepoMetadata.cs @@ -1,4 +1,4 @@ -using GVFS.Common.FileSystem; +using GVFS.Common.FileSystem; using GVFS.Common.Tracing; using System; using System.Collections.Generic; diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs index 429d123c9..ff2ad39b8 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs @@ -1,4 +1,4 @@ -using GVFS.FunctionalTests.FileSystemRunners; +using GVFS.FunctionalTests.FileSystemRunners; using GVFS.FunctionalTests.Properties; using GVFS.FunctionalTests.Should; using GVFS.FunctionalTests.Tools; @@ -11,14 +11,16 @@ using System.IO; using System.Linq; using System.Runtime.InteropServices; +using RepoMetadata = GVFS.Common.RepoMetadata; +using ReturnCode = GVFS.Common.ReturnCode; namespace GVFS.FunctionalTests.Tests.EnlistmentPerFixture { [TestFixture] public class MountTests : TestsWithEnlistmentPerFixture { - private const int GVFSGenericError = 3; - private const int GVFSMissingDiskLayoutVersion = 12; + private const int GVFSGenericError = (int)ReturnCode.GenericError; + private const int GVFSMissingDiskLayoutVersion = (int)ReturnCode.MissingDiskLayoutVersion; private const uint GenericRead = 2147483648; private const uint FileFlagBackupSemantics = 3355443; private readonly int fileDeletedBackgroundOperationCode; @@ -117,7 +119,7 @@ public void MountFailsWithUniqueExitCodeWhenDiskLayoutVersionIsMissing() this.MountShouldFail( GVFSMissingDiskLayoutVersion, - "The .gvfs metadata is incomplete. Recloning this enlistment is the recommended fix."); + RepoMetadata.MissingDiskLayoutVersionMessage); } finally { @@ -156,7 +158,7 @@ public void MountFailsWhenNoOnDiskVersion() this.MountShouldFail( GVFSMissingDiskLayoutVersion, - "The .gvfs metadata is incomplete. Recloning this enlistment is the recommended fix."); + RepoMetadata.MissingDiskLayoutVersionMessage); } finally { diff --git a/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs b/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs index ea0bdae6d..632a5869f 100644 --- a/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs +++ b/GVFS/GVFS.FunctionalTests/Tools/GVFSHelpers.cs @@ -1,4 +1,4 @@ -using GVFS.Common; +using GVFS.Common; using GVFS.FunctionalTests.FileSystemRunners; using GVFS.FunctionalTests.Should; using GVFS.Tests.Should; @@ -319,62 +319,29 @@ private static byte CharToByte(char c) private static string GetPersistedValue(string dotGVFSRoot, string key) { - string metadataPath = Path.Combine(dotGVFSRoot, RepoMetadataName); - string json; - using (FileStream fs = new FileStream(metadataPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete)) - using (StreamReader reader = new StreamReader(fs)) - { - while (!reader.EndOfStream) - { - json = reader.ReadLine(); - json.Substring(0, 2).ShouldEqual("A "); - - KeyValuePair kvp = GVFSJsonOptions.Deserialize>(json.Substring(2)); - if (kvp.Key == key) - { - return kvp.Value; - } - } - } - - return null; + Dictionary repoMetadata = ReadPersistedValues(dotGVFSRoot); + string value; + repoMetadata.TryGetValue(key, out value); + return value; } private static void SavePersistedValue(string dotGVFSRoot, string key, string value) { - string metadataPath = Path.Combine(dotGVFSRoot, RepoMetadataName); - - Dictionary repoMetadata = new Dictionary(); - string json; - using (FileStream fs = new FileStream(metadataPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete)) - using (StreamReader reader = new StreamReader(fs)) - { - while (!reader.EndOfStream) - { - json = reader.ReadLine(); - json.Substring(0, 2).ShouldEqual("A "); - - KeyValuePair kvp = GVFSJsonOptions.Deserialize>(json.Substring(2)); - repoMetadata.Add(kvp.Key, kvp.Value); - } - } - + Dictionary repoMetadata = ReadPersistedValues(dotGVFSRoot); repoMetadata[key] = value; - - string newRepoMetadataContents = string.Empty; - - foreach (KeyValuePair kvp in repoMetadata) - { - newRepoMetadataContents += "A " + GVFSJsonOptions.Serialize(kvp).Trim() + "\r\n"; - } - - File.WriteAllText(metadataPath, newRepoMetadataContents); + WritePersistedValues(dotGVFSRoot, repoMetadata); } private static void DeletePersistedValue(string dotGVFSRoot, string key) { - string metadataPath = Path.Combine(dotGVFSRoot, RepoMetadataName); + Dictionary repoMetadata = ReadPersistedValues(dotGVFSRoot); + repoMetadata.Remove(key); + WritePersistedValues(dotGVFSRoot, repoMetadata); + } + private static Dictionary ReadPersistedValues(string dotGVFSRoot) + { + string metadataPath = Path.Combine(dotGVFSRoot, RepoMetadataName); Dictionary repoMetadata = new Dictionary(); string json; using (FileStream fs = new FileStream(metadataPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete)) @@ -386,13 +353,16 @@ private static void DeletePersistedValue(string dotGVFSRoot, string key) json.Substring(0, 2).ShouldEqual("A "); KeyValuePair kvp = GVFSJsonOptions.Deserialize>(json.Substring(2)); - if (kvp.Key != key) - { - repoMetadata[kvp.Key] = kvp.Value; - } + repoMetadata[kvp.Key] = kvp.Value; } } + return repoMetadata; + } + + private static void WritePersistedValues(string dotGVFSRoot, Dictionary repoMetadata) + { + string metadataPath = Path.Combine(dotGVFSRoot, RepoMetadataName); string newRepoMetadataContents = string.Empty; foreach (KeyValuePair kvp in repoMetadata) diff --git a/GVFS/GVFS/CommandLine/DehydrateVerb.cs b/GVFS/GVFS/CommandLine/DehydrateVerb.cs index 28ce30357..9c3922ede 100644 --- a/GVFS/GVFS/CommandLine/DehydrateVerb.cs +++ b/GVFS/GVFS/CommandLine/DehydrateVerb.cs @@ -223,9 +223,10 @@ from a parent of the folders list. this.Unmount(tracer); string error; - if (!DiskLayoutUpgrade.TryCheckDiskLayoutVersion(tracer, enlistment.PrimaryEnlistmentRoot, out error)) + ReturnCode returnCode; + if (!DiskLayoutUpgrade.TryCheckDiskLayoutVersion(tracer, enlistment.PrimaryEnlistmentRoot, out error, out returnCode)) { - this.ReportErrorAndExit(tracer, error); + this.ReportErrorAndExit(tracer, returnCode, error); } RetryConfig retryConfig; diff --git a/GVFS/GVFS/CommandLine/MountVerb.cs b/GVFS/GVFS/CommandLine/MountVerb.cs index 4a38b37dc..7a3091aba 100644 --- a/GVFS/GVFS/CommandLine/MountVerb.cs +++ b/GVFS/GVFS/CommandLine/MountVerb.cs @@ -1,4 +1,4 @@ -using GVFS.Common; +using GVFS.Common; using GVFS.Common.Http; using GVFS.Common.NamedPipes; using GVFS.Common.Tracing; diff --git a/GVFS/GVFS/CommandLine/RepairVerb.cs b/GVFS/GVFS/CommandLine/RepairVerb.cs index dc0034adc..839765629 100644 --- a/GVFS/GVFS/CommandLine/RepairVerb.cs +++ b/GVFS/GVFS/CommandLine/RepairVerb.cs @@ -92,9 +92,10 @@ To actually execute any necessary repair(s), run 'gvfs repair --confirm' } string error; - if (!DiskLayoutUpgrade.TryCheckDiskLayoutVersion(tracer: null, enlistmentRoot: enlistment.PrimaryEnlistmentRoot, error: out error)) + ReturnCode returnCode; + if (!DiskLayoutUpgrade.TryCheckDiskLayoutVersion(tracer: null, enlistmentRoot: enlistment.PrimaryEnlistmentRoot, error: out error, returnCode: out returnCode)) { - this.ReportErrorAndExit(error); + this.ReportErrorAndExit(tracer: null, exitCode: returnCode, error: error); } if (!ConsoleHelper.ShowStatusWhileRunning( From 0ccec54f6713ae067321bbddeacff982fd53a03b Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Tue, 13 Oct 2026 13:02:36 -0700 Subject: [PATCH 3/4] Address disk layout review follow-up Qualify the missing disk layout recovery guidance and keep the log pointer on that path. Remove unused disk layout helper overloads, make the mount upgrade failure branch explicit, and pin the functional test to the shipped exit-code contract. Add RepoMetadata unit coverage for missing and invalid disk layout values. Assisted-by: GitHub Copilot Signed-off-by: Tyrie Vella --- .../DiskLayoutUpgrades/DiskLayoutUpgrade.cs | 18 ++-- GVFS/GVFS.Common/RepoMetadata.cs | 2 +- .../Tests/EnlistmentPerFixture/MountTests.cs | 12 +-- .../Common/RepoMetadataTests.cs | 82 +++++++++++++++++++ GVFS/GVFS/CommandLine/MountVerb.cs | 6 +- 5 files changed, 98 insertions(+), 22 deletions(-) create mode 100644 GVFS/GVFS.UnitTests/Common/RepoMetadataTests.cs diff --git a/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs b/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs index a9a6e82c1..2dccefa75 100644 --- a/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs +++ b/GVFS/GVFS.Common/DiskLayoutUpgrades/DiskLayoutUpgrade.cs @@ -70,12 +70,6 @@ public static bool TryRunAllUpgrades(string enlistmentRoot, out string error, ou } } - public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRoot, out string error) - { - ReturnCode returnCode; - return TryCheckDiskLayoutVersion(tracer, enlistmentRoot, out error, out returnCode); - } - public static bool TryCheckDiskLayoutVersion(ITracer tracer, string enlistmentRoot, out string error, out ReturnCode returnCode) { error = string.Empty; @@ -277,13 +271,6 @@ private static bool CheckLayoutVersionWasIncremented(JsonTracer tracer, string e return true; } - private static bool TryFindUpgrade(JsonTracer tracer, string enlistmentRoot, out DiskLayoutUpgrade upgrade) - { - string error; - ReturnCode returnCode; - return TryFindUpgrade(tracer, enlistmentRoot, out upgrade, out error, out returnCode); - } - private static bool TryFindUpgrade(JsonTracer tracer, string enlistmentRoot, out DiskLayoutUpgrade upgrade, out string error, out ReturnCode returnCode) { int majorVersion; @@ -367,6 +354,11 @@ private static bool TryGetDiskLayoutVersion( if (!RepoMetadata.Instance.TryGetOnDiskLayoutVersion(out majorVersion, out minorVersion, out error, out returnCode)) { + if (returnCode == ReturnCode.MissingDiskLayoutVersion) + { + error += " " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot); + } + return false; } } diff --git a/GVFS/GVFS.Common/RepoMetadata.cs b/GVFS/GVFS.Common/RepoMetadata.cs index f3213903b..00854fc69 100644 --- a/GVFS/GVFS.Common/RepoMetadata.cs +++ b/GVFS/GVFS.Common/RepoMetadata.cs @@ -8,7 +8,7 @@ namespace GVFS.Common { public class RepoMetadata { - public const string MissingDiskLayoutVersionMessage = "Enlistment disk layout version not found. The .gvfs metadata is incomplete. Recloning this enlistment is the recommended fix."; + public const string MissingDiskLayoutVersionMessage = "Enlistment disk layout version not found. The .gvfs metadata is incomplete. Make sure your work is pushed or backed up, then reclone this enlistment."; private FileBasedDictionary repoMetadata; private ITracer tracer; diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs index ff2ad39b8..b47a00b77 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs @@ -11,16 +11,16 @@ using System.IO; using System.Linq; using System.Runtime.InteropServices; -using RepoMetadata = GVFS.Common.RepoMetadata; -using ReturnCode = GVFS.Common.ReturnCode; namespace GVFS.FunctionalTests.Tests.EnlistmentPerFixture { [TestFixture] public class MountTests : TestsWithEnlistmentPerFixture { - private const int GVFSGenericError = (int)ReturnCode.GenericError; - private const int GVFSMissingDiskLayoutVersion = (int)ReturnCode.MissingDiskLayoutVersion; + private const int GVFSGenericError = 3; + // These values are a shipped command-line contract. Do not bind them to product constants. + private const int GVFSMissingDiskLayoutVersion = 12; + private const string MissingDiskLayoutVersionMessage = "The .gvfs metadata is incomplete"; private const uint GenericRead = 2147483648; private const uint FileFlagBackupSemantics = 3355443; private readonly int fileDeletedBackgroundOperationCode; @@ -119,7 +119,7 @@ public void MountFailsWithUniqueExitCodeWhenDiskLayoutVersionIsMissing() this.MountShouldFail( GVFSMissingDiskLayoutVersion, - RepoMetadata.MissingDiskLayoutVersionMessage); + MissingDiskLayoutVersionMessage); } finally { @@ -158,7 +158,7 @@ public void MountFailsWhenNoOnDiskVersion() this.MountShouldFail( GVFSMissingDiskLayoutVersion, - RepoMetadata.MissingDiskLayoutVersionMessage); + MissingDiskLayoutVersionMessage); } finally { diff --git a/GVFS/GVFS.UnitTests/Common/RepoMetadataTests.cs b/GVFS/GVFS.UnitTests/Common/RepoMetadataTests.cs new file mode 100644 index 000000000..f6ba95437 --- /dev/null +++ b/GVFS/GVFS.UnitTests/Common/RepoMetadataTests.cs @@ -0,0 +1,82 @@ +using GVFS.Common; +using GVFS.Tests.Should; +using GVFS.UnitTests.Mock.Common; +using GVFS.UnitTests.Mock.FileSystem; +using NUnit.Framework; +using System.Collections.Generic; +using System.IO; + +namespace GVFS.UnitTests.Common +{ + [TestFixture] + public class RepoMetadataTests + { + private const string DiskLayoutMajorVersionKey = "DiskLayoutVersion"; + private const string DotGVFSPath = "mock:\\repo\\.gvfs"; + + private MockFileSystem fileSystem; + + [SetUp] + public void SetUp() + { + if (GVFSPlatform.Instance == null) + { + GVFSPlatform.Register(new MockPlatform()); + } + + MockDirectory root = new MockDirectory("mock:", null, null); + root.CreateDirectory(Path.Combine(DotGVFSPath, "databases")); + this.fileSystem = new MockFileSystem(root); + + string error; + RepoMetadata.TryInitialize(new MockTracer(), this.fileSystem, DotGVFSPath, out error) + .ShouldEqual(true); + } + + [TearDown] + public void TearDown() + { + RepoMetadata.Shutdown(); + } + + [TestCase] + public void MissingDiskLayoutVersionReturnsSpecificReturnCode() + { + int majorVersion; + int minorVersion; + string error; + ReturnCode returnCode; + + RepoMetadata.Instance.TryGetOnDiskLayoutVersion(out majorVersion, out minorVersion, out error, out returnCode) + .ShouldEqual(false); + + returnCode.ShouldEqual(ReturnCode.MissingDiskLayoutVersion); + error.ShouldEqual(RepoMetadata.MissingDiskLayoutVersionMessage); + } + + [TestCase] + public void InvalidDiskLayoutVersionReturnsGenericError() + { + string metadataPath = RepoMetadata.Instance.DataFilePath; + KeyValuePair diskLayoutVersion = new KeyValuePair(DiskLayoutMajorVersionKey, "not-an-int"); + this.fileSystem.WriteAllText(metadataPath, "A " + GVFSJsonOptions.Serialize(diskLayoutVersion).Trim() + "\r\n"); + + RepoMetadata.Shutdown(); + + string initializeError; + RepoMetadata.TryInitialize(new MockTracer(), this.fileSystem, DotGVFSPath, out initializeError) + .ShouldEqual(true); + + int majorVersion; + int minorVersion; + string error; + ReturnCode returnCode; + + RepoMetadata.Instance.TryGetOnDiskLayoutVersion(out majorVersion, out minorVersion, out error, out returnCode) + .ShouldEqual(false); + + returnCode.ShouldEqual(ReturnCode.GenericError); + error.ShouldContain("Failed to parse persisted disk layout version number"); + } + } +} diff --git a/GVFS/GVFS/CommandLine/MountVerb.cs b/GVFS/GVFS/CommandLine/MountVerb.cs index 7a3091aba..700cb8528 100644 --- a/GVFS/GVFS/CommandLine/MountVerb.cs +++ b/GVFS/GVFS/CommandLine/MountVerb.cs @@ -138,8 +138,10 @@ protected override void PreCreateEnlistment() { this.ReportErrorAndExit(tracer: null, exitCode: diskLayoutReturnCode, error: "Error: " + diskLayoutError); } - - this.ReportErrorAndExit("Failed to upgrade repo disk layout. " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot)); + else + { + this.ReportErrorAndExit("Failed to upgrade repo disk layout. " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot)); + } } string error; From 439c98adc170562d51237f73ff4be4a9aa85445f Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Thu, 15 Oct 2026 14:40:56 -0700 Subject: [PATCH 4/4] Address disk layout follow-up review Surface disk layout upgrade error details on non-missing-layout failures instead of discarding them. Extend the functional tests for missing disk layout metadata to assert that the mount output includes the gvfs log pointer. Assisted-by: GitHub Copilot Signed-off-by: Tyrie Vella --- .../Tests/EnlistmentPerFixture/MountTests.cs | 14 +++++++++++--- GVFS/GVFS/CommandLine/MountVerb.cs | 8 +++++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs index b47a00b77..259a4dc98 100644 --- a/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs +++ b/GVFS/GVFS.FunctionalTests/Tests/EnlistmentPerFixture/MountTests.cs @@ -21,6 +21,7 @@ public class MountTests : TestsWithEnlistmentPerFixture // These values are a shipped command-line contract. Do not bind them to product constants. private const int GVFSMissingDiskLayoutVersion = 12; private const string MissingDiskLayoutVersionMessage = "The .gvfs metadata is incomplete"; + private const string GVFSLogMessage = "Run 'gvfs log"; private const uint GenericRead = 2147483648; private const uint FileFlagBackupSemantics = 3355443; private readonly int fileDeletedBackgroundOperationCode; @@ -119,7 +120,8 @@ public void MountFailsWithUniqueExitCodeWhenDiskLayoutVersionIsMissing() this.MountShouldFail( GVFSMissingDiskLayoutVersion, - MissingDiskLayoutVersionMessage); + MissingDiskLayoutVersionMessage, + additionalExpectedErrorMessage: GVFSLogMessage); } finally { @@ -158,7 +160,8 @@ public void MountFailsWhenNoOnDiskVersion() this.MountShouldFail( GVFSMissingDiskLayoutVersion, - MissingDiskLayoutVersionMessage); + MissingDiskLayoutVersionMessage, + additionalExpectedErrorMessage: GVFSLogMessage); } finally { @@ -391,7 +394,7 @@ public void ProjFS_CMDHangNoneActiveInstance() this.Enlistment.MountGVFS(); } - private void MountShouldFail(int expectedExitCode, string expectedErrorMessage, string mountWorkingDirectory = null) + private void MountShouldFail(int expectedExitCode, string expectedErrorMessage, string mountWorkingDirectory = null, string additionalExpectedErrorMessage = null) { string enlistmentRoot = this.Enlistment.EnlistmentRoot; @@ -411,6 +414,11 @@ private void MountShouldFail(int expectedExitCode, string expectedErrorMessage, { string combinedOutput = result.Output + "\n" + result.Errors; combinedOutput.ShouldContain(expectedErrorMessage); + + if (additionalExpectedErrorMessage != null) + { + combinedOutput.ShouldContain(additionalExpectedErrorMessage); + } } } diff --git a/GVFS/GVFS/CommandLine/MountVerb.cs b/GVFS/GVFS/CommandLine/MountVerb.cs index 700cb8528..2c4ab26cb 100644 --- a/GVFS/GVFS/CommandLine/MountVerb.cs +++ b/GVFS/GVFS/CommandLine/MountVerb.cs @@ -140,7 +140,13 @@ protected override void PreCreateEnlistment() } else { - this.ReportErrorAndExit("Failed to upgrade repo disk layout. " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot)); + string upgradeErrorMessage = "Failed to upgrade repo disk layout."; + if (!string.IsNullOrWhiteSpace(diskLayoutError)) + { + upgradeErrorMessage += " " + diskLayoutError; + } + + this.ReportErrorAndExit(upgradeErrorMessage + " " + ConsoleHelper.GetGVFSLogMessage(enlistmentRoot)); } }