Share transient libgit2 config lookup helper

GVFS/GVFS.Common/Git/LibGit2Repo.cs

@@ -39,8 +39,13 @@ public LibGit2Repo(ITracer tracer, string repoPath)
         }
 
         protected LibGit2Repo()
+            : this(NullTracer.Instance)
         {
-            this.Tracer = NullTracer.Instance;
+        }
+
+        protected LibGit2Repo(ITracer tracer)
+        {
+            this.Tracer = tracer;
         }
 
         ~LibGit2Repo()
@@ -306,6 +311,55 @@ public virtual string GetConfigString(string name)
             }
         }
 
+        /// <summary>
+        /// Reads a boolean config value from this already-open repo, falling back to
+        /// <paramref name="defaultValue"/> if the key is unset or the read fails for any reason
+        /// (e.g. a corrupt/unreadable config).
+        /// </summary>
+        public bool GetConfigBoolOrDefault(string key, bool defaultValue)
+        {
+            try
+            {
+                return this.GetConfigBool(key) ?? defaultValue;
+            }
+            catch (Exception e)
+            {
+                this.Tracer.RelatedWarning($"Failed to read {key} config, using default: {e.Message}");
+                return defaultValue;
+            }
+        }
+
+        /// <summary>
+        /// Reads a single boolean config value from the repo at <paramref name="repoPath"/>,
+        /// opening and disposing a transient <see cref="LibGit2Repo"/> for the lookup. Prefer
+        /// this over <see cref="LibGit2RepoInvoker"/> for one-off config reads:
+        /// LibGit2RepoInvoker.InitializeSharedRepo forces the object store to load, which is
+        /// wasted work when all that's needed is a single config value. Falls back to
+        /// <paramref name="defaultValue"/> if the repo can't be opened or the read fails for any
+        /// reason.
+        /// </summary>
+        public static bool GetConfigBoolOrDefault(ITracer tracer, string repoPath, string key, bool defaultValue)
+        {
+            try
+            {
+                using (LibGit2Repo repo = new LibGit2Repo(tracer, repoPath))
+                {
+                    return repo.GetConfigBoolOrDefault(key, defaultValue);
+                }
+            }
+            catch (InvalidDataException)
+            {
+                // The LibGit2Repo constructor already logged a RelatedWarning with the native
+                // failure reason before throwing; avoid logging the same failure twice.
+                return defaultValue;
+            }
+            catch (Exception e)
+            {
+                tracer.RelatedWarning($"Failed to read {key} config, using default: {e.Message}");
+                return defaultValue;
+            }
+        }
+
         public void ForEachMultiVarConfig(string key, MultiVarConfigCallback callback)
         {
             if (Native.Config.GetConfig(out IntPtr configHandle, this.RepoHandle) != Native.ResultCode.Success)

GVFS/GVFS.Hooks/GVFS.Hooks.csproj

@@ -118,4 +118,3 @@
 
 
 </Project>
-

GVFS/GVFS.Hooks/Program.cs

@@ -171,10 +171,11 @@ private static bool HasShortFlag(string arg, string flag)
 
         private static bool ConfigurationAllowsHydrationStatus()
         {
-            using (LibGit2RepoInvoker repo = new LibGit2RepoInvoker(NullTracer.Instance, normalizedCurrentDirectory))
-            {
-                return repo.GetConfigBoolOrDefault(GVFSConstants.GitConfig.ShowHydrationStatus, GVFSConstants.GitConfig.ShowHydrationStatusDefault);
-            }
+            return LibGit2Repo.GetConfigBoolOrDefault(
+                NullTracer.Instance,
+                normalizedCurrentDirectory,
+                GVFSConstants.GitConfig.ShowHydrationStatus,
+                GVFSConstants.GitConfig.ShowHydrationStatusDefault);
         }
 
         /// <summary>

GVFS/GVFS.Mount/InProcessMount.cs

@@ -475,27 +475,11 @@ private GVFSContext CreateContext()
 
         private bool IsBackgroundCacheAuthEnabled()
         {
-            // Read the flag via libgit2 (in-process) rather than spawning git.exe.
-            // The GVFSContext (and its shared libgit2 repo) is not created until
-            // later in mount, so open a short-lived repo here just for the config
-            // read. Default to off on any failure.
-            try
-            {
-                using (LibGit2Repo repo = new LibGit2Repo(this.tracer, this.enlistment.WorkingDirectoryBackingRoot))
-                {
-                    return repo.GetConfigBool(GVFSConstants.GitConfig.BackgroundCacheAuth)
-                        ?? GVFSConstants.GitConfig.BackgroundCacheAuthDefault;
-                }
-            }
-            catch (Exception e)
-            {
-                this.tracer.RelatedWarning(
-                    "Failed to read {0} config, defaulting to {1}: {2}",
-                    GVFSConstants.GitConfig.BackgroundCacheAuth,
-                    GVFSConstants.GitConfig.BackgroundCacheAuthDefault,
-                    e.Message);
-                return GVFSConstants.GitConfig.BackgroundCacheAuthDefault;
-            }
+            return LibGit2Repo.GetConfigBoolOrDefault(
+                this.tracer,
+                this.enlistment.WorkingDirectoryBackingRoot,
+                GVFSConstants.GitConfig.BackgroundCacheAuth,
+                GVFSConstants.GitConfig.BackgroundCacheAuthDefault);
         }
 
         private void ValidateMountPoints()

GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs

@@ -0,0 +1,127 @@
+using GVFS.Common.Git;
+using GVFS.Tests.Should;
+using GVFS.UnitTests.Mock.Common;
+using NUnit.Framework;
+using System;
+using System.IO;
+
+namespace GVFS.UnitTests.Common
+{
+    [TestFixture]
+    public class LibGit2RepoConfigLookupTests
+    {
+        [TestCase]
+        public void GetConfigBoolOrDefaultOnRepoReturnsConfiguredValue()
+        {
+            MockTracer tracer = new MockTracer();
+
+            using (MockConfigRepo repo = new MockConfigRepo(tracer, true))
+            {
+                bool value = repo.GetConfigBoolOrDefault("gvfs.test", false);
+
+                value.ShouldEqual(true);
+                tracer.RelatedWarningEvents.Count.ShouldEqual(0);
+            }
+        }
+
+        [TestCase]
+        public void GetConfigBoolOrDefaultOnRepoReturnsDefaultWhenKeyIsUnset()
+        {
+            MockTracer tracer = new MockTracer();
+
+            using (MockConfigRepo repo = new MockConfigRepo(tracer, (bool?)null))
+            {
+                bool value = repo.GetConfigBoolOrDefault("gvfs.test", true);
+
+                value.ShouldEqual(true);
+                tracer.RelatedWarningEvents.Count.ShouldEqual(0);
+            }
+        }
+
+        [TestCase]
+        public void GetConfigBoolOrDefaultOnRepoReturnsDefaultOnLibGit2ExceptionAndLogsOnce()
+        {
+            MockTracer tracer = new MockTracer();
+
+            using (MockConfigRepo repo = new MockConfigRepo(tracer, new LibGit2Exception("boom")))
+            {
+                bool value = repo.GetConfigBoolOrDefault("gvfs.test", false);
+
+                value.ShouldEqual(false);
+                tracer.RelatedWarningEvents.Count.ShouldEqual(1);
+                tracer.RelatedWarningEvents[0].ShouldContain("Failed to read gvfs.test config, using default: boom");
+            }
+        }
+
+        [TestCase]
+        public void GetConfigBoolOrDefaultOnRepoReturnsDefaultOnInvalidDataExceptionAndLogsOnce()
+        {
+            MockTracer tracer = new MockTracer();
+
+            using (MockConfigRepo repo = new MockConfigRepo(tracer, new InvalidDataException("corrupt config")))
+            {
+                bool value = repo.GetConfigBoolOrDefault("gvfs.test", false);
+
+                value.ShouldEqual(false);
+                tracer.RelatedWarningEvents.Count.ShouldEqual(1);
+                tracer.RelatedWarningEvents[0].ShouldContain("Failed to read gvfs.test config, using default: corrupt config");
+            }
+        }
+
+        [TestCase]
+        public void GetConfigBoolOrDefaultOnPathReturnsDefaultForMissingRepoAndLogsExactlyOnce()
+        {
+            MockTracer tracer = new MockTracer();
+
+            // A GUID-suffixed path under the OS temp directory is guaranteed not to exist and
+            // does not depend on any particular drive letter being unmapped (unlike a
+            // hardcoded "Z:\..." path, which could resolve on a host with that drive mapped).
+            string missingRepoPath = Path.Combine(
+                Path.GetTempPath(),
+                "LibGit2RepoConfigLookupTests_" + Guid.NewGuid().ToString("N"));
+
+            bool value = LibGit2Repo.GetConfigBoolOrDefault(
+                tracer,
+                missingRepoPath,
+                "gvfs.test",
+                false);
+
+            value.ShouldEqual(false);
+
+            // The LibGit2Repo constructor logs a RelatedWarning with the native open-failure
+            // reason before throwing InvalidDataException; the static helper's catch does not
+            // log a second time for that case (see LibGit2Repo.GetConfigBoolOrDefault), so
+            // exactly one warning is expected here.
+            tracer.RelatedWarningEvents.Count.ShouldEqual(1);
+            tracer.RelatedWarningEvents[0].ShouldContain("Couldn't open repo at");
+        }
+
+        private class MockConfigRepo : LibGit2Repo
+        {
+            private readonly bool? value;
+            private readonly Exception exceptionToThrow;
+
+            public MockConfigRepo(MockTracer tracer, bool? value)
+                : base(tracer)
+            {
+                this.value = value;
+            }
+
+            public MockConfigRepo(MockTracer tracer, Exception exceptionToThrow)
+                : base(tracer)
+            {
+                this.exceptionToThrow = exceptionToThrow;
+            }
+
+            public override bool? GetConfigBool(string name)
+            {
+                if (this.exceptionToThrow != null)
+                {
+                    throw this.exceptionToThrow;
+                }
+
+                return this.value;
+            }
+        }
+    }
+}

GVFS/GVFS/CommandLine/CloneVerb.cs

@@ -152,7 +152,7 @@ public override void Execute()
 
                 CacheServerInfo cacheServer = null;
                 ServerGVFSConfig serverGVFSConfig = null;
-                bool trustPackIndexes;
+                bool trustPackIndexes = GVFSConstants.GitConfig.TrustPackIndexesDefault;
 
                 using (JsonTracer tracer = new JsonTracer(GVFSConstants.GVFSEtwProviderName, "GVFSClone"))
                 {
@@ -248,10 +248,13 @@ public override void Execute()
                     {
                         tracer.RelatedError(cloneResult.ErrorMessage);
                     }
-
-                    using (var repo = new LibGit2RepoInvoker(tracer, enlistment.WorkingDirectoryBackingRoot))
+                    else
                     {
-                        trustPackIndexes = repo.GetConfigBoolOrDefault(GVFSConstants.GitConfig.TrustPackIndexes, GVFSConstants.GitConfig.TrustPackIndexesDefault);
+                        trustPackIndexes = LibGit2Repo.GetConfigBoolOrDefault(
+                            tracer,
+                            enlistment.WorkingDirectoryBackingRoot,
+                            GVFSConstants.GitConfig.TrustPackIndexes,
+                            GVFSConstants.GitConfig.TrustPackIndexesDefault);
                     }
                 }
 

GVFS/GVFS/CommandLine/PrefetchVerb.cs

@@ -700,19 +700,11 @@ private string GetCacheServerDisplay(CacheServerInfo cacheServer, string repoUrl
 
         private bool IsPrefetchOffloadEnabled(ITracer tracer, GVFSEnlistment enlistment)
         {
-            try
-            {
-                using (LibGit2Repo repo = new LibGit2Repo(tracer, enlistment.WorkingDirectoryBackingRoot))
-                {
-                    bool? enabled = repo.GetConfigBool(GVFSConstants.GitConfig.PrefetchOffload);
-                    return enabled ?? GVFSConstants.GitConfig.PrefetchOffloadDefault;
-                }
-            }
-            catch (Exception ex)
-            {
-                tracer.RelatedWarning($"Failed to read '{GVFSConstants.GitConfig.PrefetchOffload}' config; defaulting to {GVFSConstants.GitConfig.PrefetchOffloadDefault}: {ex.GetType().Name}: {ex.Message}");
-                return GVFSConstants.GitConfig.PrefetchOffloadDefault;
-            }
+            return LibGit2Repo.GetConfigBoolOrDefault(
+                tracer,
+                enlistment.WorkingDirectoryBackingRoot,
+                GVFSConstants.GitConfig.PrefetchOffload,
+                GVFSConstants.GitConfig.PrefetchOffloadDefault);
         }
 
         /// <summary>