diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs index f6287134a..3776c7e99 100644 --- a/GVFS/GVFS.Common/Git/GitProcess.cs +++ b/GVFS/GVFS.Common/Git/GitProcess.cs @@ -577,22 +577,20 @@ public bool TryGetAllConfig(bool localOnly, out DictionaryThe value found for the setting. public virtual ConfigResult GetFromConfig(string settingName, bool forceOutsideEnlistment = false, PhysicalFileSystem fileSystem = null) { - string command = string.Format("config {0}", settingName); - fileSystem = fileSystem ?? new PhysicalFileSystem(); - - // This method is called at clone time, so the physical repo may not exist yet. - // Pre-command hook never applies to `git config` reads (no working tree - // mutation happens), and skipping the hook makes us robust to a broken - // hook config in the enlistment - which is exactly what we'd be trying - // to repair via TryUpdateHooks at mount time. - Result result = - fileSystem.DirectoryExists(this.workingDirectoryRoot) && !forceOutsideEnlistment - ? this.InvokeConfigRead(settingName, () => this.InvokeGitAgainstDotGitFolder(command, usePreCommandHook: false)) - : this.InvokeConfigRead(settingName, () => this.InvokeGitOutsideEnlistment(command)); - + Result result = this.ReadConfig(settingName, string.Format("config {0}", settingName), forceOutsideEnlistment, fileSystem); return new ConfigResult(result, settingName); } + /// + /// Reads a boolean config value. is null when the key is unset. + /// Matches . + /// + public bool TryGetBoolFromConfig(string settingName, out bool? value, out string error) + { + Result result = this.ReadConfig(settingName, LibGit2Repo.GetConfigBoolArguments(settingName), forceOutsideEnlistment: false, fileSystem: null); + return LibGit2Repo.TryParseConfigBoolOutput(settingName, result.ExitCode, result.Output, result.Errors, out value, out error); + } + public ConfigResult GetFromLocalConfig(string settingName) { // See GetFromConfig above for why pre-command hook is disabled here. @@ -1293,6 +1291,21 @@ private static bool IsCredentialConfigReadFailure(Result result) return configReadFailed && transientAccessFailure; } + private Result ReadConfig(string settingName, string command, bool forceOutsideEnlistment, PhysicalFileSystem fileSystem) + { + fileSystem = fileSystem ?? new PhysicalFileSystem(); + + // This method is called at clone time, so the physical repo may not exist yet. + // Pre-command hook never applies to `git config` reads (no working tree + // mutation happens), and skipping the hook makes us robust to a broken + // hook config in the enlistment - which is exactly what we'd be trying + // to repair via TryUpdateHooks at mount time. + return + fileSystem.DirectoryExists(this.workingDirectoryRoot) && !forceOutsideEnlistment + ? this.InvokeConfigRead(settingName, () => this.InvokeGitAgainstDotGitFolder(command, usePreCommandHook: false)) + : this.InvokeConfigRead(settingName, () => this.InvokeGitOutsideEnlistment(command)); + } + private Result InvokeConfigRead(string configName, Func readConfig) { return GitConfigReadRetry.InvokeResult( diff --git a/GVFS/GVFS.Common/Git/GitRepo.cs b/GVFS/GVFS.Common/Git/GitRepo.cs index 055f85f61..1d74c911a 100644 --- a/GVFS/GVFS.Common/Git/GitRepo.cs +++ b/GVFS/GVFS.Common/Git/GitRepo.cs @@ -33,7 +33,8 @@ public GitRepo(ITracer tracer, Enlistment enlistment, PhysicalFileSystem fileSys this.libgit2RepoInvoker = new LibGit2RepoInvoker( tracer, - repoFactory ?? (() => new LibGit2Repo(this.tracer, this.enlistment.WorkingDirectoryBackingRoot))); + repoFactory ?? (() => new LibGit2Repo(this.tracer, this.enlistment.WorkingDirectoryBackingRoot)), + this.TryReadConfigBoolWithGit); } // For Unit Testing @@ -224,6 +225,11 @@ public void Dispose() } } + private bool TryReadConfigBoolWithGit(string key, out bool? value, out string error) + { + return this.enlistment.CreateGitProcess(this.tracer).TryGetBoolFromConfig(key, out value, out error); + } + private static bool ReadLooseObjectHeader(Stream input, out long size) { size = 0; diff --git a/GVFS/GVFS.Common/Git/LibGit2Repo.cs b/GVFS/GVFS.Common/Git/LibGit2Repo.cs index c3540e55e..02b0c459c 100644 --- a/GVFS/GVFS.Common/Git/LibGit2Repo.cs +++ b/GVFS/GVFS.Common/Git/LibGit2Repo.cs @@ -63,6 +63,12 @@ public class LibGit2Repo : IDisposable public delegate void MultiVarConfigCallback(string value); + /// + /// Reads a boolean config value with git.exe. is null when the key + /// is unset. Returns false and sets when the read fails. + /// + public delegate bool TryReadConfigBoolWithGit(string key, out bool? value, out string error); + public LibGit2Repo(ITracer tracer, string repoPath) : this(tracer, repoPath, GitConfigReadRetry.Delay, GitConfigReadRetry.MaxAttempts) { @@ -634,8 +640,35 @@ public bool GetConfigBoolOrDefault(string key, bool defaultValue) /// reason. bounds the transient-failure retries /// for latency-sensitive callers. /// - public static bool GetConfigBoolOrDefault(ITracer tracer, string repoPath, string key, bool defaultValue, int configReadMaxAttempts = GitConfigReadRetry.MaxAttempts) + /// + /// Reads the key with git.exe. This method uses it instead of libgit2 when + /// is true. + /// + public static bool GetConfigBoolOrDefault( + ITracer tracer, + string repoPath, + string key, + bool defaultValue, + TryReadConfigBoolWithGit readBoolWithGit, + int configReadMaxAttempts = GitConfigReadRetry.MaxAttempts) { + return GetConfigBoolOrDefault(tracer, repoPath, key, defaultValue, readBoolWithGit, Environment.GetEnvironmentVariables(), configReadMaxAttempts); + } + + internal static bool GetConfigBoolOrDefault( + ITracer tracer, + string repoPath, + string key, + bool defaultValue, + TryReadConfigBoolWithGit readBoolWithGit, + IDictionary environment, + int configReadMaxAttempts = GitConfigReadRetry.MaxAttempts) + { + if (EnvironmentChangesGitConfig(environment)) + { + return GetConfigBoolWithGitOrDefault(tracer, key, defaultValue, readBoolWithGit); + } + try { using (LibGit2Repo repo = new LibGit2Repo(tracer, repoPath, GitConfigReadRetry.Delay, configReadMaxAttempts)) @@ -656,6 +689,64 @@ public static bool GetConfigBoolOrDefault(ITracer tracer, string repoPath, strin } } + /// + /// Reads a boolean config value with git.exe through . + /// Falls back to if the key is unset or the read fails. + /// + internal static bool GetConfigBoolWithGitOrDefault(ITracer tracer, string key, bool defaultValue, TryReadConfigBoolWithGit readBoolWithGit) + { + tracer = tracer ?? NullTracer.Instance; + + try + { + if (readBoolWithGit(key, out bool? value, out string error)) + { + return value ?? defaultValue; + } + + tracer.RelatedWarning($"Failed to read {key} config, using default: {error}"); + } + catch (Exception e) + { + tracer.RelatedWarning($"Failed to read {key} config, using default: {e.Message}"); + } + + return defaultValue; + } + + /// + /// The git.exe arguments that read for . + /// + internal static string GetConfigBoolArguments(string key) + { + return "config --type=bool " + key; + } + + /// + /// Parses the result of git config --type=bool. Git normalizes a boolean to "true" or + /// "false", exits with 1 when the key is unset, and fails when the value is not a boolean. + /// + internal static bool TryParseConfigBoolOutput(string key, int exitCode, string output, string errors, out bool? value, out string error) + { + value = null; + error = null; + + if (exitCode == 1 && string.IsNullOrWhiteSpace(errors)) + { + return true; + } + + string trimmedOutput = output?.Trim(); + if (exitCode != 0 || !bool.TryParse(trimmedOutput, out bool parsed)) + { + error = $"Error while reading '{key}' from config: exit code {exitCode}, output '{trimmedOutput}', errors '{errors?.Trim()}'"; + return false; + } + + value = parsed; + return true; + } + /// /// Reads a string config value as git.exe would, without loading the object store. Reads /// the repo at , or the global, XDG, system, and ProgramData diff --git a/GVFS/GVFS.Common/Git/LibGit2RepoInvoker.cs b/GVFS/GVFS.Common/Git/LibGit2RepoInvoker.cs index f3fdf83b3..e00121b02 100644 --- a/GVFS/GVFS.Common/Git/LibGit2RepoInvoker.cs +++ b/GVFS/GVFS.Common/Git/LibGit2RepoInvoker.cs @@ -1,5 +1,6 @@ using GVFS.Common.Tracing; using System; +using System.Collections; using System.Threading; namespace GVFS.Common.Git @@ -7,6 +8,8 @@ namespace GVFS.Common.Git public class LibGit2RepoInvoker : IDisposable { private readonly Func createRepo; + private readonly LibGit2Repo.TryReadConfigBoolWithGit readBoolWithGit; + private readonly IDictionary environment; private readonly ITracer tracer; private readonly Lock sharedRepoLock = new Lock(); private volatile bool disposing; @@ -19,9 +22,21 @@ public LibGit2RepoInvoker(ITracer tracer, string repoPath) } public LibGit2RepoInvoker(ITracer tracer, Func createRepo) + : this(tracer, createRepo, readBoolWithGit: null) + { + } + + /// + /// Reads a key with git.exe. uses it instead of libgit2 + /// when is true. + /// + /// The environment to check. Null selects the process environment. + public LibGit2RepoInvoker(ITracer tracer, Func createRepo, LibGit2Repo.TryReadConfigBoolWithGit readBoolWithGit, IDictionary environment = null) { this.tracer = tracer; this.createRepo = createRepo; + this.readBoolWithGit = readBoolWithGit; + this.environment = environment; this.InitializeSharedRepo(); } @@ -89,6 +104,12 @@ public void InitializeSharedRepo() public bool GetConfigBoolOrDefault(string key, bool defaultValue) { + if (this.readBoolWithGit != null && + LibGit2Repo.EnvironmentChangesGitConfig(this.environment ?? Environment.GetEnvironmentVariables())) + { + return LibGit2Repo.GetConfigBoolWithGitOrDefault(this.tracer, key, defaultValue, this.readBoolWithGit); + } + bool? value = defaultValue; if (this.TryInvoke(repo => repo.GetConfigBool(key), out value)) { diff --git a/GVFS/GVFS.Hooks/Program.cs b/GVFS/GVFS.Hooks/Program.cs index 3093ab15d..6780631fc 100644 --- a/GVFS/GVFS.Hooks/Program.cs +++ b/GVFS/GVFS.Hooks/Program.cs @@ -177,9 +177,26 @@ private static bool ConfigurationAllowsHydrationStatus() normalizedCurrentDirectory, GVFSConstants.GitConfig.ShowHydrationStatus, GVFSConstants.GitConfig.ShowHydrationStatusDefault, + TryReadBoolConfigWithGit, ConfigReadMaxAttempts); } + private static bool TryReadBoolConfigWithGit(string key, out bool? value, out string error) + { + // Git runs this hook, so the child git.exe reads the config of the same repository. + ProcessResult result = GitConfigReadRetry.InvokeResult( + key, + () => ProcessHelper.Run("git", LibGit2Repo.GetConfigBoolArguments(key)), + (ProcessResult processResult) => + processResult.ExitCode != 0 && + GitConfigReadRetry.IsTransientFileAccessError(processResult.Errors, lineMustContain: "unable to access"), + NullTracer.Instance, + GitConfigReadRetry.Delay, + ConfigReadMaxAttempts); + + return LibGit2Repo.TryParseConfigBoolOutput(key, result.ExitCode, result.Output, result.Errors, out value, out error); + } + /// /// Query the mount process for the cached hydration summary via named pipe. /// The entire operation (connect + send + receive + parse) is bounded to diff --git a/GVFS/GVFS.Mount/InProcessMount.cs b/GVFS/GVFS.Mount/InProcessMount.cs index f5eae0859..60200cc20 100644 --- a/GVFS/GVFS.Mount/InProcessMount.cs +++ b/GVFS/GVFS.Mount/InProcessMount.cs @@ -500,7 +500,8 @@ private bool IsBackgroundCacheAuthEnabled() this.tracer, this.enlistment.WorkingDirectoryBackingRoot, GVFSConstants.GitConfig.BackgroundCacheAuth, - GVFSConstants.GitConfig.BackgroundCacheAuthDefault); + GVFSConstants.GitConfig.BackgroundCacheAuthDefault, + this.enlistment.CreateGitProcess(this.tracer).TryGetBoolFromConfig); } private void ValidateMountPoints() @@ -629,18 +630,12 @@ private bool ShouldReportMountProgress() // pipe starts), and its constructor force-loads the object store, which we must // not do on the pre-pipe critical path. A raw LibGit2Repo opens only the repo // handle + config (no object store) and gives native git-bool parsing. - try - { - using (LibGit2Repo repo = new LibGit2Repo(this.tracer, this.enlistment.WorkingDirectoryBackingRoot)) - { - return repo.GetConfigBool(GVFSConstants.GitConfig.MountProgress) ?? GVFSConstants.GitConfig.MountProgressDefault; - } - } - catch (Exception e) - { - this.tracer.RelatedWarning($"Failed to read {GVFSConstants.GitConfig.MountProgress} config, using default: {e.Message}"); - return GVFSConstants.GitConfig.MountProgressDefault; - } + return LibGit2Repo.GetConfigBoolOrDefault( + this.tracer, + this.enlistment.WorkingDirectoryBackingRoot, + GVFSConstants.GitConfig.MountProgress, + GVFSConstants.GitConfig.MountProgressDefault, + this.enlistment.CreateGitProcess(this.tracer).TryGetBoolFromConfig); } private void FailMountAndExit(string error, params object[] args) diff --git a/GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs b/GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs index 7fe8ba564..a67e92874 100644 --- a/GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs +++ b/GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs @@ -269,7 +269,9 @@ public void GetConfigBoolOrDefaultOnPathReturnsDefaultForMissingRepoAndLogsExact tracer, missingRepoPath, "gvfs.test", - false); + false, + ReadBoolWithGitMustNotRun, + new Hashtable { { "PATH", "C:\\bin" } }); value.ShouldEqual(false); @@ -281,6 +283,115 @@ public void GetConfigBoolOrDefaultOnPathReturnsDefaultForMissingRepoAndLogsExact tracer.RelatedWarningEvents[0].ShouldContain("Couldn't open repo at"); } + [TestCase(false)] + [TestCase(true)] + public void GetConfigBoolOrDefaultOnPathReadsWithGitWhenTheEnvironmentChangesGitConfig(bool gitValue) + { + MockTracer tracer = new MockTracer(); + List gitKeys = new List(); + + // libgit2 cannot open this path, so a libgit2 read logs a warning and returns the default. + bool value = LibGit2Repo.GetConfigBoolOrDefault( + tracer, + this.GetMissingRepoPath(), + "gvfs.test", + !gitValue, + (string key, out bool? result, out string error) => + { + gitKeys.Add(key); + result = gitValue; + error = null; + return true; + }, + new Hashtable { { "GIT_CONFIG_GLOBAL", "C:\\test.gitconfig" } }); + + value.ShouldEqual(gitValue); + gitKeys.ShouldMatchInOrder("gvfs.test"); + tracer.RelatedWarningEvents.Count.ShouldEqual(0); + } + + [TestCase] + public void GetConfigBoolOrDefaultOnPathReturnsDefaultWhenGitReportsAnUnsetKey() + { + MockTracer tracer = new MockTracer(); + + bool value = LibGit2Repo.GetConfigBoolOrDefault( + tracer, + this.GetMissingRepoPath(), + "gvfs.test", + true, + (string key, out bool? result, out string error) => + { + result = null; + error = null; + return true; + }, + new Hashtable { { "GIT_CONFIG_COUNT", "1" } }); + + value.ShouldEqual(true); + tracer.RelatedWarningEvents.Count.ShouldEqual(0); + } + + [TestCase] + public void GetConfigBoolOrDefaultOnPathReturnsDefaultAndLogsWhenGitFails() + { + MockTracer tracer = new MockTracer(); + + bool value = LibGit2Repo.GetConfigBoolOrDefault( + tracer, + this.GetMissingRepoPath(), + "gvfs.test", + true, + (string key, out bool? result, out string error) => + { + result = null; + error = "bad boolean"; + return false; + }, + new Hashtable { { "GIT_CONFIG_PARAMETERS", "'gvfs.test=maybe'" } }); + + value.ShouldEqual(true); + tracer.RelatedWarningEvents.Count.ShouldEqual(1); + tracer.RelatedWarningEvents[0].ShouldContain("Failed to read gvfs.test config, using default: bad boolean"); + } + + [TestCase(0, "true\n", "", true)] + [TestCase(0, "false\n", "", false)] + [TestCase(1, "", "", null)] + public void TryParseConfigBoolOutputParsesGitOutput(int exitCode, string output, string errors, bool? expected) + { + LibGit2Repo.TryParseConfigBoolOutput("gvfs.test", exitCode, output, errors, out bool? value, out string error).ShouldBeTrue(); + + value.ShouldEqual(expected); + error.ShouldBeNull(); + } + + [TestCase(128, "", "fatal: bad boolean config value 'maybe' for 'gvfs.test'")] + [TestCase(1, "", "error: could not lock config file")] + [TestCase(0, "maybe\n", "")] + public void TryParseConfigBoolOutputReportsFailures(int exitCode, string output, string errors) + { + LibGit2Repo.TryParseConfigBoolOutput("gvfs.test", exitCode, output, errors, out bool? value, out string error).ShouldBeFalse(); + + value.HasValue.ShouldBeFalse(); + error.ShouldContain("Error while reading 'gvfs.test' from config"); + } + + private static bool ReadBoolWithGitMustNotRun(string key, out bool? value, out string error) + { + Assert.Fail("Read " + key + " with git.exe"); + value = null; + error = null; + return false; + } + + private string GetMissingRepoPath() + { + return Path.Combine( + Path.GetTempPath(), + "LibGit2RepoConfigLookupTests_" + Guid.NewGuid().ToString("N")); + } + private class MockConfigRepo : LibGit2Repo { private readonly bool? value; diff --git a/GVFS/GVFS.UnitTests/Common/LibGit2RepoInvokerTests.cs b/GVFS/GVFS.UnitTests/Common/LibGit2RepoInvokerTests.cs index fbef35f22..44d6a2e19 100644 --- a/GVFS/GVFS.UnitTests/Common/LibGit2RepoInvokerTests.cs +++ b/GVFS/GVFS.UnitTests/Common/LibGit2RepoInvokerTests.cs @@ -2,7 +2,9 @@ using GVFS.Tests.Should; using GVFS.UnitTests.Mock.Common; using NUnit.Framework; +using System.Collections; using System.Collections.Concurrent; +using System.Collections.Generic; using System.Threading; namespace GVFS.UnitTests.Common @@ -171,6 +173,48 @@ public void UsesOnlyOneRepoMultipleThreads() this.numConstructors.ShouldEqual(1); } + [TestCase] + public void GetConfigBoolOrDefaultReadsWithGitWhenTheEnvironmentChangesGitConfig() + { + List gitKeys = new List(); + using (LibGit2RepoInvoker invoker = new LibGit2RepoInvoker( + this.tracer, + this.CreateRepo, + (string key, out bool? value, out string error) => + { + gitKeys.Add(key); + value = false; + error = null; + return true; + }, + new Hashtable { { "GIT_CONFIG_GLOBAL", "C:\\test.gitconfig" } })) + { + // The mock libgit2 repo returns true. + invoker.GetConfigBoolOrDefault("gvfs.test", true).ShouldEqual(false); + } + + gitKeys.ShouldMatchInOrder("gvfs.test"); + } + + [TestCase] + public void GetConfigBoolOrDefaultReadsWithLibGit2WhenTheEnvironmentDoesNotChangeGitConfig() + { + using (LibGit2RepoInvoker invoker = new LibGit2RepoInvoker( + this.tracer, + this.CreateRepo, + (string key, out bool? value, out string error) => + { + Assert.Fail("Read " + key + " with git.exe"); + value = null; + error = null; + return false; + }, + new Hashtable { { "PATH", "C:\\bin" } })) + { + invoker.GetConfigBoolOrDefault("gvfs.test", false).ShouldEqual(true); + } + } + private LibGit2Repo CreateRepo() { Interlocked.Increment(ref this.numConstructors); @@ -191,6 +235,11 @@ public override bool ObjectExists(string sha) return false; } + public override bool? GetConfigBool(string name) + { + return true; + } + protected override void Dispose(bool disposing) { if (disposing) diff --git a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs index 4dd4b0fec..f85caf13a 100644 --- a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs @@ -252,6 +252,20 @@ public void GetFromConfigRetriesTransientFailure() getDelayCount().ShouldEqual(1); } + [TestCase] + public void TryGetBoolFromConfigRunsGitConfigWithTypeBool() + { + MockGitProcess process = this.CreateProcessWithTransientThenSuccess( + "config --type=bool gvfs.test", + "false\n", + out Func getDelayCount); + + process.TryGetBoolFromConfig("gvfs.test", out bool? value, out string error).ShouldBeTrue(error); + + value.ShouldEqual(false); + getDelayCount().ShouldEqual(1); + } + [TestCase] public void GetOriginUrlRetriesTransientFailure() { diff --git a/GVFS/GVFS/CommandLine/CloneVerb.cs b/GVFS/GVFS/CommandLine/CloneVerb.cs index fda015182..f00192366 100644 --- a/GVFS/GVFS/CommandLine/CloneVerb.cs +++ b/GVFS/GVFS/CommandLine/CloneVerb.cs @@ -308,7 +308,8 @@ public override void Execute() tracer, enlistment.WorkingDirectoryBackingRoot, GVFSConstants.GitConfig.TrustPackIndexes, - GVFSConstants.GitConfig.TrustPackIndexesDefault); + GVFSConstants.GitConfig.TrustPackIndexesDefault, + enlistment.CreateGitProcess(tracer).TryGetBoolFromConfig); } } diff --git a/GVFS/GVFS/CommandLine/PrefetchVerb.cs b/GVFS/GVFS/CommandLine/PrefetchVerb.cs index ee0ae862f..5c212ea60 100644 --- a/GVFS/GVFS/CommandLine/PrefetchVerb.cs +++ b/GVFS/GVFS/CommandLine/PrefetchVerb.cs @@ -706,7 +706,8 @@ private bool IsPrefetchOffloadEnabled(ITracer tracer, GVFSEnlistment enlistment) tracer, enlistment.WorkingDirectoryBackingRoot, GVFSConstants.GitConfig.PrefetchOffload, - GVFSConstants.GitConfig.PrefetchOffloadDefault); + GVFSConstants.GitConfig.PrefetchOffloadDefault, + enlistment.CreateGitProcess(tracer).TryGetBoolFromConfig); } ///