diff --git a/GVFS/GVFS.Common/Enlistment.cs b/GVFS/GVFS.Common/Enlistment.cs index 1faf2f59d..65d6e4643 100644 --- a/GVFS/GVFS.Common/Enlistment.cs +++ b/GVFS/GVFS.Common/Enlistment.cs @@ -1,5 +1,6 @@ using GVFS.Common.FileSystem; using GVFS.Common.Git; +using GVFS.Common.Tracing; using System; using System.IO; @@ -123,5 +124,10 @@ public virtual GitProcess CreateGitProcess() { return new GitProcess(this); } + + public virtual GitProcess CreateGitProcess(ITracer tracer) + { + return new GitProcess(this, tracer); + } } } diff --git a/GVFS/GVFS.Common/Git/GitConfigHelper.cs b/GVFS/GVFS.Common/Git/GitConfigHelper.cs index eb0b08c87..bd156e0b8 100644 --- a/GVFS/GVFS.Common/Git/GitConfigHelper.cs +++ b/GVFS/GVFS.Common/Git/GitConfigHelper.cs @@ -4,7 +4,7 @@ namespace GVFS.Common.Git { /// - /// Helper methods for git config-style file reading and parsing. + /// Helper methods for reading Git config files and parsing config values. /// public static class GitConfigHelper { @@ -30,6 +30,34 @@ public static bool TrySanitizeConfigFileLine(string fileLine, out string sanitiz return !string.IsNullOrWhiteSpace(sanitizedLine); } + /// + /// Parses an integer setting. A null or blank value selects . + /// + public static bool TryParseIntSetting(string configName, string valueString, int defaultValue, int minValue, out int value, out string error) + { + value = defaultValue; + error = string.Empty; + + if (string.IsNullOrWhiteSpace(valueString)) + { + return true; + } + + if (!int.TryParse(valueString, out value)) + { + error = string.Format("Misconfigured config setting {0}, could not parse value `{1}` as an int", configName, valueString); + return false; + } + + if (value < minValue) + { + error = string.Format("Invalid value {0} for setting {1}, value must be greater than or equal to {2}", value, configName, minValue); + return false; + } + + return true; + } + /// /// Get the settings for a section in a given config file. /// diff --git a/GVFS/GVFS.Common/Git/GitConfigReadRetry.cs b/GVFS/GVFS.Common/Git/GitConfigReadRetry.cs new file mode 100644 index 000000000..f1b362ede --- /dev/null +++ b/GVFS/GVFS.Common/Git/GitConfigReadRetry.cs @@ -0,0 +1,180 @@ +using GVFS.Common.Tracing; +using System; +using System.Threading; + +namespace GVFS.Common.Git +{ + internal static class GitConfigReadRetry + { + internal const int MaxAttempts = 5; + internal const int InitialDelayMilliseconds = 20; + + private static readonly string[] TransientReasons = + { + "Permission denied", + "Device or resource busy", + "being used by another process", + "sharing violation", + }; + + /// + /// Returns false when the caller's time budget cannot cover the next delay. The read then + /// stops as if it used all attempts. + /// + /// Receives each failed result that will be read again. + internal static T InvokeResult( + string configName, + Func readConfig, + Func isTransientFailure, + ITracer tracer, + Action delay, + int maxAttempts = MaxAttempts, + Func canWait = null, + Action beforeRetry = null) + { + for (int attempt = 1; ; ++attempt) + { + T result = readConfig(); + if (!isTransientFailure(result)) + { + return result; + } + + TimeSpan retryDelay = GetRetryDelay(attempt); + if (attempt >= maxAttempts || (canWait != null && !canWait(retryDelay))) + { + TraceExhausted(tracer, configName, attempt); + return result; + } + + TraceRetry(tracer, configName, attempt, maxAttempts, retryDelay); + beforeRetry?.Invoke(result); + delay(retryDelay); + } + } + + internal static T InvokeCatching( + string configName, + Func readConfig, + Func isTransientFailure, + ITracer tracer, + Action delay, + int maxAttempts = MaxAttempts) + { + for (int attempt = 1; ; ++attempt) + { + try + { + return readConfig(); + } + catch (Exception e) when (isTransientFailure(e)) + { + if (attempt >= maxAttempts) + { + TraceExhausted(tracer, configName, attempt); + throw; + } + + TimeSpan retryDelay = GetRetryDelay(attempt); + TraceRetry(tracer, configName, attempt, maxAttempts, retryDelay); + delay(retryDelay); + } + } + } + + internal static bool IsTransientLibGit2Error(Exception exception) + { + return + exception is LibGit2Exception && + IsTransientFileAccessError(exception.Message); + } + + /// + /// Both git.exe and libgit2 report a file failure as ... '<path>': <reason>. + /// Only the reason after the final ': is tested, so a path or key that contains + /// a transient phrase does not match. + /// + internal static bool IsTransientFileAccessError(string error, string lineMustContain = null) + { + if (string.IsNullOrEmpty(error)) + { + return false; + } + + foreach (string line in error.Split('\n')) + { + if (lineMustContain != null && !ContainsOrdinalIgnoreCase(line, lineMustContain)) + { + continue; + } + + int reasonSeparator = line.LastIndexOf("': ", StringComparison.Ordinal); + if (reasonSeparator < 0) + { + continue; + } + + string reason = line.Substring(reasonSeparator + 3); + foreach (string transientReason in TransientReasons) + { + if (ContainsOrdinalIgnoreCase(reason, transientReason)) + { + return true; + } + } + } + + return false; + } + + internal static bool ContainsOrdinalIgnoreCase(string value, string expected) + { + return + value != null && + value.IndexOf(expected, StringComparison.OrdinalIgnoreCase) >= 0; + } + + internal static void Delay(TimeSpan delay) + { + Thread.Sleep(delay); + } + + private static TimeSpan GetRetryDelay(int failedAttempt) + { + return TimeSpan.FromMilliseconds(InitialDelayMilliseconds * (1 << (failedAttempt - 1))); + } + + // A repository URL with embedded credentials can become part of a deprecated cache + // server config key name. + private static string GetTraceableConfigName(string configName) + { + return configName != null && configName.IndexOf('@') >= 0 + ? "" + : configName; + } + + private static void TraceRetry(ITracer tracer, string configName, int failedAttempt, int maxAttempts, TimeSpan retryDelay) + { + EventMetadata metadata = new EventMetadata(); + metadata.Add("ConfigName", GetTraceableConfigName(configName)); + metadata.Add("AttemptNumber", failedAttempt); + metadata.Add("MaxAttempts", maxAttempts); + metadata.Add("RetryDelayMilliseconds", retryDelay.TotalMilliseconds); + tracer.RelatedWarning( + metadata, + "Transient failure while reading git config. Retrying.", + Keywords.Telemetry); + } + + private static void TraceExhausted(ITracer tracer, string configName, int attempts) + { + EventMetadata metadata = new EventMetadata(); + metadata.Add("ConfigName", GetTraceableConfigName(configName)); + metadata.Add("Attempts", attempts); + tracer.RelatedWarning( + metadata, + "Transient failure while reading git config persisted after all attempts.", + Keywords.Telemetry); + } + } +} diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs index ac8a1eb3d..f6287134a 100644 --- a/GVFS/GVFS.Common/Git/GitProcess.cs +++ b/GVFS/GVFS.Common/Git/GitProcess.cs @@ -40,8 +40,6 @@ public class GitProcess : ICredentialStore private const string MissingPasswordErrorMessage = "Git credential fill returned no password"; private const int MaxCredentialOutputKeys = 32; private const int MaxCredentialOutputKeyLength = 32; - private const int MaxCredentialConfigReadAttempts = 5; - private const int CredentialConfigReadRetryDelayMs = 20; private static readonly Encoding UTF8NoBOM = new UTF8Encoding(false); @@ -85,6 +83,8 @@ public class GitProcess : ICredentialStore private string gitBinPath; private string workingDirectoryRoot; private string gitDirPath; + private readonly ITracer tracer; + private readonly Action configReadDelay; private Process executingProcess; private bool stopping; @@ -114,11 +114,26 @@ static GitProcess() } public GitProcess(Enlistment enlistment) - : this(enlistment.GitBinPath, enlistment.WorkingDirectoryBackingRoot) + : this(enlistment.GitBinPath, enlistment.WorkingDirectoryBackingRoot, NullTracer.Instance, GitConfigReadRetry.Delay) + { + } + + public GitProcess(Enlistment enlistment, ITracer tracer) + : this(enlistment.GitBinPath, enlistment.WorkingDirectoryBackingRoot, tracer, GitConfigReadRetry.Delay) + { + } + + internal GitProcess(Enlistment enlistment, ITracer tracer, Action configReadDelay) + : this(enlistment.GitBinPath, enlistment.WorkingDirectoryBackingRoot, tracer, configReadDelay) { } public GitProcess(string gitBinPath, string workingDirectoryRoot) + : this(gitBinPath, workingDirectoryRoot, NullTracer.Instance, GitConfigReadRetry.Delay) + { + } + + internal GitProcess(string gitBinPath, string workingDirectoryRoot, ITracer tracer, Action configReadDelay) { if (string.IsNullOrWhiteSpace(gitBinPath)) { @@ -127,6 +142,8 @@ public GitProcess(string gitBinPath, string workingDirectoryRoot) this.gitBinPath = gitBinPath; this.workingDirectoryRoot = workingDirectoryRoot; + this.tracer = tracer ?? NullTracer.Instance; + this.configReadDelay = configReadDelay ?? GitConfigReadRetry.Delay; if (this.workingDirectoryRoot != null) { @@ -167,22 +184,28 @@ public static Result Init(Enlistment enlistment) public static ConfigResult GetFromGlobalConfig(string gitBinPath, string settingName) { - return new ConfigResult( - new GitProcess(gitBinPath, workingDirectoryRoot: null).InvokeGitOutsideEnlistment("config --global " + settingName), + GitProcess git = new GitProcess(gitBinPath, workingDirectoryRoot: null); + return new ConfigResult(git.InvokeConfigRead( + settingName, + () => git.InvokeGitOutsideEnlistment("config --global " + settingName)), settingName); } public static ConfigResult GetFromSystemConfig(string gitBinPath, string settingName) { - return new ConfigResult( - new GitProcess(gitBinPath, workingDirectoryRoot: null).InvokeGitOutsideEnlistment("config --system " + settingName), + GitProcess git = new GitProcess(gitBinPath, workingDirectoryRoot: null); + return new ConfigResult(git.InvokeConfigRead( + settingName, + () => git.InvokeGitOutsideEnlistment("config --system " + settingName)), settingName); } public static ConfigResult GetFromFileConfig(string gitBinPath, string configFile, string settingName) { - return new ConfigResult( - new GitProcess(gitBinPath, workingDirectoryRoot: null).InvokeGitOutsideEnlistment("config --file " + configFile + " " + settingName), + GitProcess git = new GitProcess(gitBinPath, workingDirectoryRoot: null); + return new ConfigResult(git.InvokeConfigRead( + settingName, + () => git.InvokeGitOutsideEnlistment("config --file " + configFile + " " + settingName)), settingName); } @@ -510,7 +533,9 @@ public Result SetInFileConfig(string configFile, string settingName, string valu public bool TryGetConfigUrlMatch(string section, string repositoryUrl, out Dictionary configSettings) { // See GetFromConfig for why pre-command hook is disabled. - Result result = this.InvokeGitAgainstDotGitFolder($"config --get-urlmatch {section} {repositoryUrl}", usePreCommandHook: false); + Result result = this.InvokeConfigRead( + section, + () => this.InvokeGitAgainstDotGitFolder($"config --get-urlmatch {section} {repositoryUrl}", usePreCommandHook: false)); if (result.ExitCodeIsFailure) { configSettings = null; @@ -526,7 +551,11 @@ public bool TryGetAllConfig(bool localOnly, out Dictionary this.InvokeGitAgainstDotGitFolder("config --list " + localParameter, usePreCommandHook: false)), + "--list"); if (result.TryParseAsString(out string output, out string _, string.Empty)) { @@ -556,16 +585,22 @@ public virtual ConfigResult GetFromConfig(string settingName, bool forceOutsideE // 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 + Result result = fileSystem.DirectoryExists(this.workingDirectoryRoot) && !forceOutsideEnlistment - ? new ConfigResult(this.InvokeGitAgainstDotGitFolder(command, usePreCommandHook: false), settingName) - : new ConfigResult(this.InvokeGitOutsideEnlistment(command), settingName); + ? this.InvokeConfigRead(settingName, () => this.InvokeGitAgainstDotGitFolder(command, usePreCommandHook: false)) + : this.InvokeConfigRead(settingName, () => this.InvokeGitOutsideEnlistment(command)); + + return new ConfigResult(result, settingName); } public ConfigResult GetFromLocalConfig(string settingName) { // See GetFromConfig above for why pre-command hook is disabled here. - return new ConfigResult(this.InvokeGitAgainstDotGitFolder("config --local " + settingName, usePreCommandHook: false), settingName); + return new ConfigResult( + this.InvokeConfigRead( + settingName, + () => this.InvokeGitAgainstDotGitFolder("config --local " + settingName, usePreCommandHook: false)), + settingName); } /// @@ -597,7 +632,12 @@ public ConfigResult GetOriginUrl() { /* Disable precommand hook because this config call is used during mounting process * which needs to be able to fix a bad precommand hook configuration. */ - return new ConfigResult(this.InvokeGitAgainstDotGitFolder("config --local remote.origin.url", usePreCommandHook: false), "remote.origin.url"); + const string ConfigName = "remote.origin.url"; + return new ConfigResult( + this.InvokeConfigRead( + ConfigName, + () => this.InvokeGitAgainstDotGitFolder("config --local " + ConfigName, usePreCommandHook: false)), + ConfigName); } public Result DiffTree(string sourceTreeish, string targetTreeish, Action onResult) @@ -1189,51 +1229,31 @@ private Result InvokeCredentialFill( bool hasVisibleConsoleWindow, int timeoutMs = Timeout.Infinite) { + // Each attempt gets only the time that remains of the caller's timeout. Stopwatch timer = Stopwatch.StartNew(); - int remainingTimeoutMs = timeoutMs; - for (int attempt = 1; ; ++attempt) - { - Result result = this.InvokeGitAgainstDotGitFolder( - command, - stdin => stdin.Write(input), - parseStdOutLine: null, - usePreCommandHook: false, - timeoutMs: remainingTimeoutMs, - mayRequireAuth: true, - hasVisibleConsoleWindow: hasVisibleConsoleWindow); - - if (!IsCredentialConfigReadFailure(result)) - { - return result; - } - - int delayMs = CredentialConfigReadRetryDelayMs << (attempt - 1); - bool willRetry = attempt < MaxCredentialConfigReadAttempts && - (timeoutMs == Timeout.Infinite || timeoutMs - timer.ElapsedMilliseconds > delayMs); - tracer.RelatedEvent( - EventLevel.Warning, - "CredentialConfigReadFailure", - new EventMetadata - { - { "AttemptNumber", attempt }, - { "MaxAttempts", MaxCredentialConfigReadAttempts }, - { "WillRetry", willRetry }, - { "RetryDelayMilliseconds", willRetry ? delayMs : 0 }, - }, - Keywords.Network | Keywords.Telemetry); - - if (!willRetry) + bool firstAttempt = true; + return GitConfigReadRetry.InvokeResult( + "credential fill", + () => { - return result; - } - - TraceCredentialFillErrorOutput(tracer, result.Errors); - Thread.Sleep(delayMs); - if (timeoutMs != Timeout.Infinite) - { - remainingTimeoutMs = (int)Math.Max(0, timeoutMs - timer.ElapsedMilliseconds); - } - } + int attemptTimeoutMs = firstAttempt || timeoutMs == Timeout.Infinite + ? timeoutMs + : (int)Math.Max(0, timeoutMs - timer.ElapsedMilliseconds); + firstAttempt = false; + return this.InvokeGitAgainstDotGitFolder( + command, + stdin => stdin.Write(input), + parseStdOutLine: null, + usePreCommandHook: false, + timeoutMs: attemptTimeoutMs, + mayRequireAuth: true, + hasVisibleConsoleWindow: hasVisibleConsoleWindow); + }, + IsCredentialConfigReadFailure, + tracer, + this.configReadDelay, + canWait: delay => timeoutMs == Timeout.Infinite || timeoutMs - timer.ElapsedMilliseconds > delay.TotalMilliseconds, + beforeRetry: result => TraceCredentialFillErrorOutput(tracer, result.Errors)); } private static bool IsCredentialConfigReadFailure(Result result) @@ -1259,20 +1279,9 @@ private static bool IsCredentialConfigReadFailure(Result result) { configReadFailed = true; } - else if (line.StartsWith("warning: unable to access '", StringComparison.Ordinal)) + else if (line.StartsWith("warning: unable to access '", StringComparison.Ordinal) && + GitConfigReadRetry.IsTransientFileAccessError(line)) { - int separator = line.LastIndexOf("': ", StringComparison.Ordinal); - if (separator < 0) - { - return false; - } - - string reason = line.Substring(separator + 3); - if (reason != "Permission denied" && reason != "Device or resource busy") - { - return false; - } - transientAccessFailure = true; } else @@ -1284,6 +1293,23 @@ private static bool IsCredentialConfigReadFailure(Result result) return configReadFailed && transientAccessFailure; } + private Result InvokeConfigRead(string configName, Func readConfig) + { + return GitConfigReadRetry.InvokeResult( + configName, + readConfig, + IsTransientConfigReadFailure, + this.tracer, + this.configReadDelay); + } + + private static bool IsTransientConfigReadFailure(Result result) + { + return + result.ExitCodeIsFailure && + GitConfigReadRetry.IsTransientFileAccessError(result.Errors, lineMustContain: "unable to access"); + } + private static void AddCredentialFillResult(EventMetadata data, Result result) { // Describes the failure without the text of stderr, which can contain a secret. @@ -1660,32 +1686,13 @@ public bool TryParseAsString(out string value, out string error, string defaultV public bool TryParseAsInt(int defaultValue, int minValue, out int value, out string error) { value = defaultValue; - error = string.Empty; if (!this.TryParseAsString(out string valueString, out error)) { return false; } - if (string.IsNullOrWhiteSpace(valueString)) - { - // Use default value - return true; - } - - if (!int.TryParse(valueString, out value)) - { - error = string.Format("Misconfigured config setting {0}, could not parse value `{1}` as an int", this.configName, valueString); - return false; - } - - if (value < minValue) - { - error = string.Format("Invalid value {0} for setting {1}, value must be greater than or equal to {2}", value, this.configName, minValue); - return false; - } - - return true; + return GitConfigHelper.TryParseIntSetting(this.configName, valueString, defaultValue, minValue, out value, out error); } } } diff --git a/GVFS/GVFS.Common/Git/LibGit2Repo.cs b/GVFS/GVFS.Common/Git/LibGit2Repo.cs index 6667dd2c8..c3540e55e 100644 --- a/GVFS/GVFS.Common/Git/LibGit2Repo.cs +++ b/GVFS/GVFS.Common/Git/LibGit2Repo.cs @@ -1,5 +1,6 @@ using GVFS.Common.Tracing; using System; +using System.Collections; using System.Collections.Generic; using System.IO; using System.Runtime.InteropServices; @@ -27,21 +28,73 @@ namespace GVFS.Common.Git /// public class LibGit2Repo : IDisposable { + // git.exe reads config from these variables, or uses them to select config files. + // libgit2 does not. + // + // libgit2 also ignores GIT_DIR, but this list intentionally excludes it. Git exports + // GIT_DIR to hooks in linked worktrees, so GVFS processes that hooks start often inherit + // it. GVFS reads a repository through an explicit path, which GIT_DIR does not change. + // Without a repository path, git.exe can read the local config of the repository that + // GIT_DIR names. That repository is not related to the read, so libgit2 reads only the + // global and system config. + private static readonly string[] ConfigEnvironmentVariablesLibGit2Ignores = + { + "GIT_CONFIG", + "GIT_CONFIG_COUNT", + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_NOSYSTEM", + "GIT_CONFIG_PARAMETERS", + "GIT_CONFIG_SYSTEM", + }; + + // Of the includeIf conditions, libgit2 evaluates only these the way git.exe does. It skips + // others, such as hasconfig:remote.*.url, and it can apply onbranch: on a detached HEAD. + private static readonly string[] IncludeConditionsLibGit2EvaluatesLikeGit = + { + "gitdir:", + "gitdir/i:", + }; + private bool disposedValue = false; + private bool initializedNative = false; private IntPtr odbHandle = IntPtr.Zero; + private readonly Action configReadDelay; + private readonly int configReadMaxAttempts; public delegate void MultiVarConfigCallback(string value); public LibGit2Repo(ITracer tracer, string repoPath) + : this(tracer, repoPath, GitConfigReadRetry.Delay, GitConfigReadRetry.MaxAttempts) + { + } + + internal LibGit2Repo(ITracer tracer, string repoPath, Action configReadDelay, int configReadMaxAttempts) { this.Tracer = tracer; + this.configReadDelay = configReadDelay ?? GitConfigReadRetry.Delay; + this.configReadMaxAttempts = configReadMaxAttempts; InitNative(); - - IntPtr repoHandle; - if (TryOpenRepo(repoPath, out repoHandle) != Native.ResultCode.Success) + this.initializedNative = true; + + // Opening the repo parses .git\config, so it is subject to the same transient failures as a config read. + IntPtr repoHandle = IntPtr.Zero; + string reason = null; + Native.ResultCode openResult = GitConfigReadRetry.InvokeResult( + "repository open", + () => + { + Native.ResultCode result = TryOpenRepo(repoPath, out repoHandle); + reason = result == Native.ResultCode.Success ? null : GetLastNativeError(); + return result; + }, + (Native.ResultCode result) => result != Native.ResultCode.Success && GitConfigReadRetry.IsTransientFileAccessError(reason), + tracer, + this.configReadDelay, + this.configReadMaxAttempts); + + if (openResult != Native.ResultCode.Success) { - string reason = GetLastNativeError(); string message = "Couldn't open repo at " + repoPath + ": " + reason; tracer.RelatedWarning(message); @@ -49,6 +102,10 @@ public LibGit2Repo(ITracer tracer, string repoPath) || !CheckSafeDirectoryConfigForCaseSensitivityIssue(tracer, repoPath, out repoHandle)) { ShutdownNative(); + + // The native reference is released, so the finalizer must not release it again. + this.disposedValue = true; + GC.SuppressFinalize(this); throw new InvalidDataException(message); } } @@ -62,8 +119,15 @@ protected LibGit2Repo() } protected LibGit2Repo(ITracer tracer) + : this(tracer, GitConfigReadRetry.Delay) + { + } + + protected LibGit2Repo(ITracer tracer, Action configReadDelay) { this.Tracer = tracer; + this.configReadDelay = configReadDelay ?? GitConfigReadRetry.Delay; + this.configReadMaxAttempts = GitConfigReadRetry.MaxAttempts; } ~LibGit2Repo() @@ -274,6 +338,22 @@ public virtual string[] GetMissingSubTrees(string treeSha) /// Name of the config entry /// The config value, or null if not found. public virtual string GetConfigString(string name) + { + return GitConfigReadRetry.InvokeCatching( + name, + () => this.GetConfigStringOnce(name), + GitConfigReadRetry.IsTransientLibGit2Error, + this.Tracer, + this.configReadDelay, + this.configReadMaxAttempts); + } + + protected virtual string GetConfigStringOnce(string name) + { + return this.ReadRepoConfigString(name, rejectIncludesLibGit2Ignores: false); + } + + private string ReadRepoConfigString(string name, bool rejectIncludesLibGit2Ignores) { IntPtr configHandle; if (Native.Config.GetConfig(out configHandle, this.RepoHandle) != Native.ResultCode.Success) @@ -282,47 +362,223 @@ public virtual string GetConfigString(string name) } try { - // git_config_get_string returns a borrowed pointer whose lifetime is tied to the - // config, so libgit2 only allows it on a snapshot (read-only) config. Calling it on - // the live config returned by git_repository_config fails with "get_string called on - // a live config object". Snapshot the config first, then read the string from it. - IntPtr snapshotHandle; - if (Native.Config.Snapshot(out snapshotHandle, configHandle) != Native.ResultCode.Success) + return GetStringFromSnapshot(configHandle, name, rejectIncludesLibGit2Ignores); + } + finally + { + Native.Config.Free(configHandle); + } + } + + private static string GetStringFromSnapshot(IntPtr configHandle, string name, bool rejectIncludesLibGit2Ignores) + { + // git_config_get_string returns a borrowed pointer whose lifetime is tied to the + // config, so libgit2 only allows it on a snapshot (read-only) config. Calling it on + // the live config returned by git_repository_config fails with "get_string called on + // a live config object". Snapshot the config first, then read the string from it. + IntPtr snapshotHandle; + if (Native.Config.Snapshot(out snapshotHandle, configHandle) != Native.ResultCode.Success) + { + throw new LibGit2Exception($"Failed to snapshot config for '{name}': {Native.GetLastError()}"); + } + + try + { + if (rejectIncludesLibGit2Ignores) + { + ThrowIfUserConfigFilesDifferFromGit(); + + string include = FindIncludeLibGit2EvaluatesDifferently(snapshotHandle); + if (include != null) + { + throw new LibGit2Exception($"Config has an include that libgit2 does not evaluate like git: {include}"); + } + } + + // git_config_get_string yields a borrowed pointer owned by the (snapshot) + // config, so it is retrieved as an IntPtr and copied manually. Marshalling it + // directly as an out string would make the interop marshaller free the pointer + // with CoTaskMemFree, corrupting libgit2's heap (mismatched allocator). + IntPtr valuePtr; + Native.ResultCode resultCode = Native.Config.GetString(out valuePtr, snapshotHandle, name); + if (resultCode == Native.ResultCode.NotFound) + { + return null; + } + else if (resultCode != Native.ResultCode.Success) + { + throw new LibGit2Exception($"Failed to get config value for '{name}': {Native.GetLastError()}"); + } + + return valuePtr == IntPtr.Zero ? null : Marshal.PtrToStringUTF8(valuePtr); + } + finally + { + Native.Config.Free(snapshotHandle); + } + } + + /// + /// Throws when libgit2 reads a different global or XDG config file than git.exe. git for + /// Windows uses one home directory: HOME, else HOMEDRIVE and HOMEPATH, else USERPROFILE. + /// libgit2 uses the first of these directories that contains the file. libgit2 also looks + /// for the XDG file under APPDATA and LOCALAPPDATA, which git does not. + /// + private static void ThrowIfUserConfigFilesDifferFromGit() + { + string home = GetGitForWindowsHomeDirectory(); + string xdgConfigHome = Environment.GetEnvironmentVariable("XDG_CONFIG_HOME"); + string gitGlobal = home == null ? null : Path.Combine(home, ".gitconfig"); + string gitXdg = !string.IsNullOrEmpty(xdgConfigHome) + ? Path.Combine(xdgConfigHome, "git", "config") + : home == null ? null : Path.Combine(home, ".config", "git", "config"); + + ThrowIfFileDiffersFromGit("global", FindUserConfigFile(isGlobal: true), gitGlobal); + ThrowIfFileDiffersFromGit("XDG", FindUserConfigFile(isGlobal: false), gitXdg); + } + + private static string GetGitForWindowsHomeDirectory() + { + string home = Environment.GetEnvironmentVariable("HOME"); + if (!string.IsNullOrEmpty(home)) + { + return home; + } + + string homeDrive = Environment.GetEnvironmentVariable("HOMEDRIVE"); + string homePath = Environment.GetEnvironmentVariable("HOMEPATH"); + if (!string.IsNullOrEmpty(homeDrive) && !string.IsNullOrEmpty(homePath) && Directory.Exists(homeDrive + homePath)) + { + return homeDrive + homePath; + } + + string userProfile = Environment.GetEnvironmentVariable("USERPROFILE"); + return string.IsNullOrEmpty(userProfile) ? null : userProfile; + } + + private static string FindUserConfigFile(bool isGlobal) + { + Native.Config.GitBuf buffer = default(Native.Config.GitBuf); + try + { + Native.ResultCode resultCode = isGlobal ? Native.Config.FindGlobal(ref buffer) : Native.Config.FindXdg(ref buffer); + if (resultCode == Native.ResultCode.NotFound) { - throw new LibGit2Exception($"Failed to snapshot config for '{name}': {Native.GetLastError()}"); + return null; } + if (resultCode != Native.ResultCode.Success) + { + throw new LibGit2Exception($"Failed to find the {(isGlobal ? "global" : "XDG")} config file: {Native.GetLastError()}"); + } + + return Marshal.PtrToStringUTF8(buffer.Ptr, checked((int)buffer.Size.ToUInt64())); + } + finally + { + Native.Config.DisposeBuf(ref buffer); + } + } + + private static void ThrowIfFileDiffersFromGit(string level, string libGit2Path, string gitPath) + { + if (gitPath != null && !File.Exists(gitPath)) + { + gitPath = null; + } + + libGit2Path = libGit2Path == null ? null : Path.GetFullPath(libGit2Path); + gitPath = gitPath == null ? null : Path.GetFullPath(gitPath); + if (!string.Equals(libGit2Path, gitPath, StringComparison.OrdinalIgnoreCase)) + { + throw new LibGit2Exception( + $"libgit2 reads the {level} config file '{libGit2Path ?? ""}', but git reads '{gitPath ?? ""}'"); + } + } + + /// + /// Returns the first include ("name = path") that libgit2 can evaluate differently from + /// git.exe, or null. + /// + private static string FindIncludeLibGit2EvaluatesDifferently(IntPtr configHandle) + { + const string ConditionalPrefix = "includeif."; + const string Suffix = ".path"; + + string differentInclude = null; + Native.Config.GitConfigMultivarCallback callback = (entryPtr, payload) => + { try { - // git_config_get_string yields a borrowed pointer owned by the (snapshot) - // config, so it is retrieved as an IntPtr and copied manually. Marshalling it - // directly as an out string would make the interop marshaller free the pointer - // with CoTaskMemFree, corrupting libgit2's heap (mismatched allocator). - IntPtr valuePtr; - Native.ResultCode resultCode = Native.Config.GetString(out valuePtr, snapshotHandle, name); - if (resultCode == Native.ResultCode.NotFound) - { - return null; - } - else if (resultCode != Native.ResultCode.Success) + Native.Config.GitConfigEntry entry = Marshal.PtrToStructure(entryPtr); + string entryName = entry.GetName(); + string path = entry.GetValue() ?? string.Empty; + if (IsIncludeLibGit2EvaluatesLikeGit(entryName, path, ConditionalPrefix, Suffix)) { - throw new LibGit2Exception($"Failed to get config value for '{name}': {Native.GetLastError()}"); + return Native.ResultCode.Success; } - return valuePtr == IntPtr.Zero ? null : Marshal.PtrToStringUTF8(valuePtr); + differentInclude = entryName + " = " + path; } - finally + catch (Exception e) { - Native.Config.Free(snapshotHandle); + // An exception must not cross the native callback boundary. + differentInclude = ""; } + + return Native.ResultCode.Failure; + }; + + Native.ResultCode resultCode = Native.Config.ForeachMatch(configHandle, @"^include(if\..*)?\.path$", callback, IntPtr.Zero); + GC.KeepAlive(callback); + if (differentInclude == null && resultCode != Native.ResultCode.Success) + { + throw new LibGit2Exception($"Failed to enumerate include config: {Native.GetLastError()}"); } - finally + + return differentInclude; + } + + private static bool IsIncludeLibGit2EvaluatesLikeGit(string entryName, string path, string conditionalPrefix, string suffix) + { + // git.exe expands %(prefix) to its installation directory. libgit2 skips the include. + if (path.IndexOf("%(prefix)", StringComparison.Ordinal) >= 0) { - Native.Config.Free(configHandle); + return false; } + + if (!entryName.StartsWith(conditionalPrefix, StringComparison.Ordinal)) + { + return true; + } + + string condition = entryName.Substring(conditionalPrefix.Length, entryName.Length - conditionalPrefix.Length - suffix.Length); + foreach (string evaluated in IncludeConditionsLibGit2EvaluatesLikeGit) + { + if (condition.StartsWith(evaluated, StringComparison.Ordinal)) + { + // git.exe treats a pattern that starts with a slash as a path without a drive. + // libgit2 matches it anywhere, as if it started with "**". + string pattern = condition.Substring(evaluated.Length); + return !pattern.StartsWith("/", StringComparison.Ordinal) && !pattern.StartsWith("\\", StringComparison.Ordinal); + } + } + + return false; } public virtual bool? GetConfigBool(string name) + { + return GitConfigReadRetry.InvokeCatching( + name, + () => this.GetConfigBoolOnce(name), + GitConfigReadRetry.IsTransientLibGit2Error, + this.Tracer, + this.configReadDelay, + this.configReadMaxAttempts); + } + + protected virtual bool? GetConfigBoolOnce(string name) { IntPtr configHandle; if (Native.Config.GetConfig(out configHandle, this.RepoHandle) != Native.ResultCode.Success) @@ -375,13 +631,14 @@ public bool GetConfigBoolOrDefault(string key, bool defaultValue) /// 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 /// if the repo can't be opened or the read fails for any - /// reason. + /// reason. bounds the transient-failure retries + /// for latency-sensitive callers. /// - public static bool GetConfigBoolOrDefault(ITracer tracer, string repoPath, string key, bool defaultValue) + public static bool GetConfigBoolOrDefault(ITracer tracer, string repoPath, string key, bool defaultValue, int configReadMaxAttempts = GitConfigReadRetry.MaxAttempts) { try { - using (LibGit2Repo repo = new LibGit2Repo(tracer, repoPath)) + using (LibGit2Repo repo = new LibGit2Repo(tracer, repoPath, GitConfigReadRetry.Delay, configReadMaxAttempts)) { return repo.GetConfigBoolOrDefault(key, defaultValue); } @@ -399,15 +656,147 @@ public static bool GetConfigBoolOrDefault(ITracer tracer, string repoPath, strin } } + /// + /// 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 + /// config when is null. is null when + /// the key is unset. Fails when the config has an include that libgit2 can evaluate + /// differently from git.exe. Callers must also check . + /// + internal static bool TryGetConfigString(ITracer tracer, string repoPath, string name, out string value, out string error) + { + tracer = tracer ?? NullTracer.Instance; + value = null; + error = null; + + try + { + if (repoPath == null) + { + value = GetGlobalAndSystemConfigString(tracer, name); + } + else + { + using (LibGit2Repo repo = new LibGit2Repo(tracer, repoPath)) + { + value = GitConfigReadRetry.InvokeCatching( + name, + () => repo.ReadRepoConfigString(name, rejectIncludesLibGit2Ignores: true), + GitConfigReadRetry.IsTransientLibGit2Error, + tracer, + repo.configReadDelay, + repo.configReadMaxAttempts); + } + } + + return true; + } + catch (Exception e) when (e is InvalidDataException || e is LibGit2Exception) + { + error = "Error while reading '" + name + "' from config: " + e.Message; + return false; + } + } + + /// + /// True when the process environment selects or supplies Git config that libgit2 does not read. + /// + internal static bool EnvironmentChangesGitConfig() + { + return EnvironmentChangesGitConfig(Environment.GetEnvironmentVariables()); + } + + internal static bool EnvironmentChangesGitConfig(IDictionary environment) + { + foreach (object key in environment.Keys) + { + string name = key as string; + foreach (string variable in ConfigEnvironmentVariablesLibGit2Ignores) + { + if (string.Equals(name, variable, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + } + + return false; + } + + private static string GetGlobalAndSystemConfigString(ITracer tracer, string name) + { + Native.Init(); + try + { + return GitConfigReadRetry.InvokeCatching( + name, + () => + { + if (Native.Config.GetGlobalAndSystemConfig(out IntPtr configHandle) != Native.ResultCode.Success) + { + throw new LibGit2Exception($"Failed to get global and system config handle: {Native.GetLastError()}"); + } + + try + { + return GetStringFromSnapshot(configHandle, name, rejectIncludesLibGit2Ignores: true); + } + finally + { + Native.Config.Free(configHandle); + } + }, + GitConfigReadRetry.IsTransientLibGit2Error, + tracer, + GitConfigReadRetry.Delay); + } + finally + { + Native.Shutdown(); + } + } + public void ForEachMultiVarConfig(string key, MultiVarConfigCallback callback) + { + List values = GitConfigReadRetry.InvokeCatching( + key, + () => this.GetMultiVarConfigValues(key), + GitConfigReadRetry.IsTransientLibGit2Error, + this.Tracer, + this.configReadDelay, + this.configReadMaxAttempts); + + foreach (string value in values) + { + callback(value); + } + } + + public static void ForEachMultiVarConfigInGlobalAndSystemConfig(string key, MultiVarConfigCallback callback) + { + List values = GitConfigReadRetry.InvokeCatching( + key, + () => GetGlobalAndSystemMultiVarConfigValues(key), + GitConfigReadRetry.IsTransientLibGit2Error, + NullTracer.Instance, + GitConfigReadRetry.Delay); + + foreach (string value in values) + { + callback(value); + } + } + + protected virtual List GetMultiVarConfigValues(string key) { if (Native.Config.GetConfig(out IntPtr configHandle, this.RepoHandle) != Native.ResultCode.Success) { throw new LibGit2Exception($"Failed to get config handle: {Native.GetLastError()}"); } + try { - ForEachMultiVarConfig(configHandle, key, callback); + return GetMultiVarConfigValues(configHandle, key); } finally { @@ -415,15 +804,16 @@ public void ForEachMultiVarConfig(string key, MultiVarConfigCallback callback) } } - public static void ForEachMultiVarConfigInGlobalAndSystemConfig(string key, MultiVarConfigCallback callback) + private static List GetGlobalAndSystemMultiVarConfigValues(string key) { if (Native.Config.GetGlobalAndSystemConfig(out IntPtr configHandle) != Native.ResultCode.Success) { throw new LibGit2Exception($"Failed to get global and system config handle: {Native.GetLastError()}"); } + try { - ForEachMultiVarConfig(configHandle, key, callback); + return GetMultiVarConfigValues(configHandle, key); } finally { @@ -431,14 +821,15 @@ public static void ForEachMultiVarConfigInGlobalAndSystemConfig(string key, Mult } } - private static void ForEachMultiVarConfig(IntPtr configHandle, string key, MultiVarConfigCallback callback) + private static List GetMultiVarConfigValues(IntPtr configHandle, string key) { + List values = new List(); Native.Config.GitConfigMultivarCallback nativeCallback = (entryPtr, payload) => { try { - var entry = Marshal.PtrToStructure(entryPtr); - callback(entry.GetValue()); + Native.Config.GitConfigEntry entry = Marshal.PtrToStructure(entryPtr); + values.Add(entry.GetValue()); } catch (Exception) { @@ -455,6 +846,8 @@ private static void ForEachMultiVarConfig(IntPtr configHandle, string key, Multi { throw new LibGit2Exception($"Failed to get multivar config for '{key}': {Native.GetLastError()}"); } + + return values; } /// @@ -510,8 +903,14 @@ protected virtual void Dispose(bool disposing) this.odbHandle = IntPtr.Zero; } - Native.Repo.Free(this.RepoHandle); - Native.Shutdown(); + // The protected constructors for test doubles do not initialize libgit2, so their + // instances have no libgit2 reference to release. + if (this.initializedNative) + { + Native.Repo.Free(this.RepoHandle); + Native.Shutdown(); + } + this.disposedValue = true; } } @@ -681,6 +1080,23 @@ public static class Odb public static class Config { + [DllImport(Git2NativeLibName, EntryPoint = "git_config_find_global")] + public static extern ResultCode FindGlobal(ref GitBuf path); + + [DllImport(Git2NativeLibName, EntryPoint = "git_config_find_xdg")] + public static extern ResultCode FindXdg(ref GitBuf path); + + [DllImport(Git2NativeLibName, EntryPoint = "git_buf_dispose")] + public static extern void DisposeBuf(ref GitBuf buffer); + + [StructLayout(LayoutKind.Sequential)] + public struct GitBuf + { + public IntPtr Ptr; + public UIntPtr Reserved; + public UIntPtr Size; + } + [DllImport(Git2NativeLibName, EntryPoint = "git_repository_config")] public static extern ResultCode GetConfig(out IntPtr configHandle, IntPtr repoHandle); @@ -701,6 +1117,13 @@ public static extern ResultCode GetMultivarForeach( GitConfigMultivarCallback callback, IntPtr payload); + [DllImport(Git2NativeLibName, EntryPoint = "git_config_foreach_match")] + public static extern ResultCode ForeachMatch( + IntPtr configHandle, + [MarshalAs(UnmanagedType.LPUTF8Str)] string regex, + GitConfigMultivarCallback callback, + IntPtr payload); + [UnmanagedFunctionPointer(CallingConvention.Cdecl)] public delegate ResultCode GitConfigMultivarCallback( IntPtr entryPtr, diff --git a/GVFS/GVFS.Common/GitStatusCacheConfig.cs b/GVFS/GVFS.Common/GitStatusCacheConfig.cs index 82598fb4a..415f1e40a 100644 --- a/GVFS/GVFS.Common/GitStatusCacheConfig.cs +++ b/GVFS/GVFS.Common/GitStatusCacheConfig.cs @@ -25,7 +25,7 @@ public GitStatusCacheConfig(TimeSpan backOffTime) public static bool TryLoadFromGitConfig(ITracer tracer, Enlistment enlistment, out GitStatusCacheConfig gitStatusCacheConfig, out string error) { - return TryLoadFromGitConfig(tracer, new GitProcess(enlistment), out gitStatusCacheConfig, out error); + return TryLoadFromGitConfig(tracer, enlistment.CreateGitProcess(tracer), out gitStatusCacheConfig, out error); } public static bool TryLoadFromGitConfig(ITracer tracer, GitProcess git, out GitStatusCacheConfig gitStatusCacheConfig, out string error) diff --git a/GVFS/GVFS.Common/Http/CacheServerResolver.cs b/GVFS/GVFS.Common/Http/CacheServerResolver.cs index 7a0c6e1a7..34a9c1721 100644 --- a/GVFS/GVFS.Common/Http/CacheServerResolver.cs +++ b/GVFS/GVFS.Common/Http/CacheServerResolver.cs @@ -18,10 +18,10 @@ public CacheServerResolver( this.enlistment = enlistment; } - public static CacheServerInfo GetCacheServerFromConfig(Enlistment enlistment) + public static CacheServerInfo GetCacheServerFromConfig(Enlistment enlistment, ITracer tracer = null) { - GitProcess git = enlistment.CreateGitProcess(); - string url = GetUrlFromConfig(enlistment); + GitProcess git = CreateGitProcess(enlistment, tracer); + string url = GetUrlFromConfig(enlistment, tracer); string prefetchCacheServerUrl = GetEndpointUrlFromConfig(git, GVFSConstants.GitConfig.PrefetchCacheServer); string getCacheServerUrl = GetEndpointUrlFromConfig(git, GVFSConstants.GitConfig.GetCacheServer); string postCacheServerUrl = GetEndpointUrlFromConfig(git, GVFSConstants.GitConfig.PostCacheServer); @@ -36,9 +36,9 @@ public static CacheServerInfo GetCacheServerFromConfig(Enlistment enlistment) sizesCacheServerUrl); } - public static string GetUrlFromConfig(Enlistment enlistment) + public static string GetUrlFromConfig(Enlistment enlistment, ITracer tracer = null) { - GitProcess git = enlistment.CreateGitProcess(); + GitProcess git = CreateGitProcess(enlistment, tracer); // TODO 1057500: Remove support for encoded-repo-url cache config setting return @@ -170,6 +170,13 @@ private static string GetValueFromConfig(GitProcess git, string configName, bool return value; } + private static GitProcess CreateGitProcess(Enlistment enlistment, ITracer tracer) + { + return tracer == null + ? enlistment.CreateGitProcess() + : enlistment.CreateGitProcess(tracer); + } + private static string GetEndpointUrlFromConfig(GitProcess git, string configName) { string url = GetValueFromConfig(git, configName, localOnly: true); diff --git a/GVFS/GVFS.Common/RetryConfig.cs b/GVFS/GVFS.Common/RetryConfig.cs index 7de0f1a61..4751c8cf4 100644 --- a/GVFS/GVFS.Common/RetryConfig.cs +++ b/GVFS/GVFS.Common/RetryConfig.cs @@ -1,6 +1,7 @@ using GVFS.Common.Git; using GVFS.Common.Tracing; using System; +using System.Collections; using System.ComponentModel; using System.IO; @@ -21,6 +22,8 @@ public class RetryConfig private static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(DefaultTimeoutSeconds); + internal delegate bool ConfigStringReader(string name, out string value, out string error); + public RetryConfig(int maxRetries = DefaultMaxRetries) : this(maxRetries, DefaultTimeout) { @@ -45,21 +48,89 @@ public int MaxAttempts public static RetryConfig LoadFromGitConfig(ITracer tracer, Enlistment enlistment) { ArgumentNullException.ThrowIfNull(enlistment); - return LoadFromGitConfig(tracer, enlistment.CreateGitProcess()); + return LoadFromGitConfig(tracer, CreateConfigReader(tracer, enlistment, Environment.GetEnvironmentVariables())); } /// - /// Invalid or unreadable retry settings use defaults and emit warnings. + /// Reads the settings with git.exe. Invalid or unreadable retry settings use defaults and + /// emit warnings. /// public static RetryConfig LoadFromGitConfig(ITracer tracer, GitProcess git) { ArgumentNullException.ThrowIfNull(git); + return LoadFromGitConfig( + tracer, + (string name, out string value, out string error) => git.GetFromConfig(name).TryParseAsString(out value, out error)); + } + + /// + /// Reads the settings in process with libgit2, and with git.exe when libgit2 cannot give + /// the value that git.exe would give. + /// + internal static ConfigStringReader CreateConfigReader(ITracer tracer, Enlistment enlistment, IDictionary environment) + { + GitProcess git = null; + ConfigStringReader readWithGit = (string name, out string value, out string error) => + { + git = git ?? enlistment.CreateGitProcess(tracer); + return git.GetFromConfig(name).TryParseAsString(out value, out error); + }; + + return LibGit2Repo.EnvironmentChangesGitConfig(environment) + ? readWithGit + : WithGitFallback(tracer, CreateLibGit2Reader(tracer, enlistment.WorkingDirectoryBackingRoot), readWithGit); + } + + internal static ConfigStringReader CreateLibGit2Reader(ITracer tracer, string workingDirectoryRoot) + { + // When the working directory does not exist, for example before clone creates it, + // read only the global and system config. See LibGit2Repo for why GIT_DIR is ignored. + string repoPath = Directory.Exists(workingDirectoryRoot) ? workingDirectoryRoot : null; + return (string name, out string value, out string error) => + LibGit2Repo.TryGetConfigString(tracer, repoPath, name, out value, out error); + } + + /// + /// Reads with git.exe when libgit2 fails, for example because it rejects the repository's + /// ownership or evaluates an include differently from git.exe. A transient file access + /// failure is not read again, because libgit2 already used the GitConfigReadRetry budget for it. + /// + internal static ConfigStringReader WithGitFallback(ITracer tracer, ConfigStringReader readWithLibGit2, ConfigStringReader readWithGit) + { + return (string name, out string value, out string error) => + { + if (readWithLibGit2(name, out value, out error)) + { + return true; + } + + if (GitConfigReadRetry.IsTransientFileAccessError(error)) + { + return false; + } + + tracer?.RelatedEvent( + EventLevel.Informational, + "RetryConfig_ReadWithGit", + new EventMetadata + { + { "Area", EtwArea }, + { "ConfigName", name }, + { "LibGit2Error", error }, + }); + + return readWithGit(name, out value, out error); + }; + } + + internal static RetryConfig LoadFromGitConfig(ITracer tracer, ConfigStringReader readConfig) + { int maxRetries = LoadIntOrDefault( - tracer, git, GVFSConstants.GitConfig.MaxRetriesConfig, DefaultMaxRetries, 0, MaxConfiguredRetries); + tracer, readConfig, GVFSConstants.GitConfig.MaxRetriesConfig, DefaultMaxRetries, 0, MaxConfiguredRetries); int timeoutSeconds = LoadIntOrDefault( - tracer, git, GVFSConstants.GitConfig.TimeoutSecondsConfig, DefaultTimeoutSeconds, 1, MaxTimeoutSeconds); + tracer, readConfig, GVFSConstants.GitConfig.TimeoutSecondsConfig, DefaultTimeoutSeconds, 1, MaxTimeoutSeconds); int readTimeoutSeconds = LoadIntOrDefault( - tracer, git, GVFSConstants.GitConfig.ReadTimeoutSecondsConfig, DefaultReadTimeoutSeconds, 0, MaxTimeoutSeconds); + tracer, readConfig, GVFSConstants.GitConfig.ReadTimeoutSecondsConfig, DefaultReadTimeoutSeconds, 0, MaxTimeoutSeconds); RetryConfig retryConfig = new RetryConfig(maxRetries, TimeSpan.FromSeconds(timeoutSeconds)) { ReadTimeout = readTimeoutSeconds == 0 @@ -86,20 +157,20 @@ public static RetryConfig LoadFromGitConfig(ITracer tracer, GitProcess git) } private static int LoadIntOrDefault( - ITracer tracer, GitProcess git, string configName, int defaultValue, int minValue, int maxValue) + ITracer tracer, ConfigStringReader readConfig, string configName, int defaultValue, int minValue, int maxValue) { string error; Exception readException = null; try { - GitProcess.ConfigResult result = git.GetFromConfig(configName); - if (result.TryParseAsInt(defaultValue, minValue, out int value, out error) && value <= maxValue) + if (readConfig(configName, out string valueString, out error) && + GitConfigHelper.TryParseIntSetting(configName, valueString, defaultValue, minValue, out int value, out error)) { - return value; - } + if (value <= maxValue) + { + return value; + } - if (string.IsNullOrEmpty(error)) - { error = $"Invalid value {value} for setting {configName}, value must be between {minValue} and {maxValue}"; } } diff --git a/GVFS/GVFS.Hooks/GVFS.Hooks.csproj b/GVFS/GVFS.Hooks/GVFS.Hooks.csproj index 3b996578e..1e0c60aaa 100644 --- a/GVFS/GVFS.Hooks/GVFS.Hooks.csproj +++ b/GVFS/GVFS.Hooks/GVFS.Hooks.csproj @@ -23,6 +23,9 @@ Common\Git\LibGit2Exception.cs + + Common\Git\GitConfigReadRetry.cs + Common\Git\LibGit2Repo.cs diff --git a/GVFS/GVFS.Hooks/Program.CommandLocking.cs b/GVFS/GVFS.Hooks/Program.CommandLocking.cs index f759ae75e..d53537332 100644 --- a/GVFS/GVFS.Hooks/Program.CommandLocking.cs +++ b/GVFS/GVFS.Hooks/Program.CommandLocking.cs @@ -1,10 +1,15 @@ using GVFS.Common; +using GVFS.Common.Git; +using GVFS.Common.Tracing; using System.Linq; namespace GVFS.Hooks { public partial class Program { + // Hooks run on the git command path, so config reads retry at most once. + private const int ConfigReadMaxAttempts = 2; + // The lock serializes changes to the enlistment's index and tracked working tree with GVFS projection updates. // Before adding an exemption, check all supported options, subcommands, and child-command paths. // Ref and object writes alone do not require this lock. Index refreshes do. @@ -111,7 +116,16 @@ private static string GetGitCommand(string[] args) private static bool IsAlias(string command) { - ProcessResult result = ProcessHelper.Run("git", "config --get alias." + command); + string aliasKey = "alias." + command; + ProcessResult result = GitConfigReadRetry.InvokeResult( + aliasKey, + () => ProcessHelper.Run("git", "config --get " + aliasKey), + (ProcessResult processResult) => + processResult.ExitCode != 0 && + GitConfigReadRetry.IsTransientFileAccessError(processResult.Errors, lineMustContain: "unable to access"), + NullTracer.Instance, + GitConfigReadRetry.Delay, + ConfigReadMaxAttempts); return !string.IsNullOrEmpty(result.Output); } diff --git a/GVFS/GVFS.Hooks/Program.cs b/GVFS/GVFS.Hooks/Program.cs index 49d72af06..3093ab15d 100644 --- a/GVFS/GVFS.Hooks/Program.cs +++ b/GVFS/GVFS.Hooks/Program.cs @@ -171,11 +171,13 @@ private static bool HasShortFlag(string arg, string flag) private static bool ConfigurationAllowsHydrationStatus() { + // See ConfigReadMaxAttempts for why the retry budget is small here. return LibGit2Repo.GetConfigBoolOrDefault( NullTracer.Instance, normalizedCurrentDirectory, GVFSConstants.GitConfig.ShowHydrationStatus, - GVFSConstants.GitConfig.ShowHydrationStatusDefault); + GVFSConstants.GitConfig.ShowHydrationStatusDefault, + ConfigReadMaxAttempts); } /// diff --git a/GVFS/GVFS.Mount/InProcessMount.cs b/GVFS/GVFS.Mount/InProcessMount.cs index 284b87c83..f5eae0859 100644 --- a/GVFS/GVFS.Mount/InProcessMount.cs +++ b/GVFS/GVFS.Mount/InProcessMount.cs @@ -1961,7 +1961,7 @@ private bool TrySetRequiredGitConfigSettings() { Dictionary requiredSettings = RequiredGitConfig.GetRequiredSettings(this.enlistment); - GitProcess git = new GitProcess(this.enlistment); + GitProcess git = this.enlistment.CreateGitProcess(this.tracer); Dictionary existingConfigSettings; if (!git.TryGetAllConfig(localOnly: true, configSettings: out existingConfigSettings)) diff --git a/GVFS/GVFS.Mount/InProcessMountVerb.cs b/GVFS/GVFS.Mount/InProcessMountVerb.cs index 9a40a00a7..cb8364cd3 100644 --- a/GVFS/GVFS.Mount/InProcessMountVerb.cs +++ b/GVFS/GVFS.Mount/InProcessMountVerb.cs @@ -131,7 +131,7 @@ public void Execute() // leaving behind an empty log with no explanation. try { - CacheServerInfo cacheServer = CacheServerResolver.GetCacheServerFromConfig(enlistment); + CacheServerInfo cacheServer = CacheServerResolver.GetCacheServerFromConfig(enlistment, tracer); tracer.WriteStartEvent( enlistment.WorkingDirectoryRoot, diff --git a/GVFS/GVFS.UnitTests/Common/ConfigReaderTracerTests.cs b/GVFS/GVFS.UnitTests/Common/ConfigReaderTracerTests.cs new file mode 100644 index 000000000..4cff31967 --- /dev/null +++ b/GVFS/GVFS.UnitTests/Common/ConfigReaderTracerTests.cs @@ -0,0 +1,102 @@ +using GVFS.Common; +using GVFS.Common.Git; +using GVFS.Common.Http; +using GVFS.Common.Tracing; +using GVFS.Tests.Should; +using GVFS.UnitTests.Mock.Common; +using GVFS.UnitTests.Mock.Git; +using NUnit.Framework; +using System.Collections; +using System.Collections.Generic; + +namespace GVFS.UnitTests.Common +{ + // The mount process passes its tracer to every config reader so that retries are traced. + [TestFixture] + public class ConfigReaderTracerTests + { + [TestCase] + public void CacheServerResolverCreatesItsGitProcessWithTheTracer() + { + MockTracer tracer = new MockTracer(); + TracerCapturingEnlistment enlistment = this.CreateEnlistment(); + + CacheServerResolver.GetCacheServerFromConfig(enlistment, tracer); + + (enlistment.CapturedTracers.Count > 0).ShouldBeTrue(); + foreach (ITracer capturedTracer in enlistment.CapturedTracers) + { + capturedTracer.ShouldBeSameAs(tracer); + } + } + + [TestCase] + public void RetryConfigCreatesItsGitProcessWithTheTracerWhenTheEnvironmentChangesGitConfig() + { + MockTracer tracer = new MockTracer(); + TracerCapturingEnlistment enlistment = this.CreateEnlistment(); + + RetryConfig.LoadFromGitConfig( + tracer, + RetryConfig.CreateConfigReader(tracer, enlistment, new Hashtable { { "GIT_CONFIG_COUNT", "1" } })); + + // One git.exe process reads all settings. + enlistment.CapturedTracers.Count.ShouldEqual(1); + enlistment.CapturedTracers[0].ShouldBeSameAs(tracer); + } + + [TestCase] + public void RetryConfigDoesNotStartGitWhenLibGit2ReadsTheSettings() + { + TracerCapturingEnlistment enlistment = this.CreateEnlistment(); + + // The mock working directory does not exist, so libgit2 reads the empty isolated global config. + using (new IsolatedLibGit2Config()) + { + RetryConfig.LoadFromGitConfig( + new MockTracer(), + RetryConfig.CreateConfigReader(new MockTracer(), enlistment, new Hashtable { { "PATH", "C:\\bin" } })); + } + + enlistment.CapturedTracers.Count.ShouldEqual(0); + } + + [TestCase] + public void GitStatusCacheConfigCreatesItsGitProcessWithTheTracer() + { + MockTracer tracer = new MockTracer(); + TracerCapturingEnlistment enlistment = this.CreateEnlistment(); + + GitStatusCacheConfig.TryLoadFromGitConfig(tracer, enlistment, out GitStatusCacheConfig _, out string _); + + enlistment.CapturedTracers.Count.ShouldEqual(1); + enlistment.CapturedTracers[0].ShouldBeSameAs(tracer); + } + + private TracerCapturingEnlistment CreateEnlistment() + { + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + "config", + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.GenericFailureCode), + matchPrefix: true); + return new TracerCapturingEnlistment(git); + } + + private class TracerCapturingEnlistment : MockGVFSEnlistment + { + public TracerCapturingEnlistment(MockGitProcess gitProcess) + : base(gitProcess) + { + } + + public List CapturedTracers { get; } = new List(); + + public override GitProcess CreateGitProcess(ITracer tracer) + { + this.CapturedTracers.Add(tracer); + return base.CreateGitProcess(tracer); + } + } + } +} diff --git a/GVFS/GVFS.UnitTests/Common/Git/GitProcessCredentialTests.cs b/GVFS/GVFS.UnitTests/Common/Git/GitProcessCredentialTests.cs index c3abf7178..39899ec0c 100644 --- a/GVFS/GVFS.UnitTests/Common/Git/GitProcessCredentialTests.cs +++ b/GVFS/GVFS.UnitTests/Common/Git/GitProcessCredentialTests.cs @@ -50,9 +50,11 @@ public void CredentialFillRetriesAConfigReadFailure(bool certificate, string rea password.ShouldEqual(SecretValue); attempts.ShouldEqual(2); gitProcess.InvocationsRun.Select(i => i.TimeoutMs).ShouldMatchInOrder(-1, -1); - tracer.RelatedEventNames.ShouldMatchInOrder("CredentialConfigReadFailure"); - ((bool)tracer.RelatedEventMetadata[0]["WillRetry"]).ShouldBeTrue(); - AssertConfigReadFailureTelemetry(tracer); + List retryTelemetry = ConfigReadRetryTelemetry(tracer); + retryTelemetry.Count.ShouldEqual(1); + retryTelemetry[0]["ConfigName"].ShouldEqual("credential fill"); + retryTelemetry[0]["AttemptNumber"].ShouldEqual(1); + AssertConfigReadRetryTelemetryHasNoStderr(tracer); AssertNoSecret(tracer); } @@ -65,11 +67,13 @@ public void CredentialFillStopsAfterTheConfigRetryBudget() gitProcess.TryGetCredential(tracer, "mock://repoUrl", out _, out _, out string error).ShouldBeFalse(); gitProcess.InvocationsRun.Count.ShouldEqual(5); - tracer.RelatedEventMetadata.Select(e => (bool)e["WillRetry"]).ShouldMatchInOrder(true, true, true, true, false); - tracer.RelatedEventMetadata.Select(e => (int)e["AttemptNumber"]).ShouldMatchInOrder(1, 2, 3, 4, 5); - tracer.RelatedEventMetadata.Select(e => (int)e["MaxAttempts"]).ShouldMatchInOrder(5, 5, 5, 5, 5); - tracer.RelatedEventMetadata.Select(e => (int)e["RetryDelayMilliseconds"]).ShouldMatchInOrder(20, 40, 80, 160, 0); - AssertConfigReadFailureTelemetry(tracer); + List retryTelemetry = ConfigReadRetryTelemetry(tracer); + retryTelemetry.Take(4).Select(e => (int)e["AttemptNumber"]).ShouldMatchInOrder(1, 2, 3, 4); + retryTelemetry.Take(4).Select(e => (double)e["RetryDelayMilliseconds"]).ShouldMatchInOrder(20.0, 40.0, 80.0, 160.0); + retryTelemetry.Take(4).Select(e => (int)e["MaxAttempts"]).ShouldMatchInOrder(5, 5, 5, 5); + retryTelemetry.Count.ShouldEqual(5); + retryTelemetry[4]["Attempts"].ShouldEqual(5); + AssertConfigReadRetryTelemetryHasNoStderr(tracer); AssertErrorMessageHasNoStderr(error); tracer.RelatedWarningEvents.Any(e => e.Contains("Permission denied")).ShouldBeTrue(); } @@ -95,7 +99,7 @@ public void CredentialFillDoesNotRetryOtherFailures(string errors, string output gitProcess.TryGetCredential(tracer, "mock://repoUrl", out _, out _, out _).ShouldBeFalse(); gitProcess.InvocationsRun.Count.ShouldEqual(1); - tracer.RelatedEventNames.ShouldBeEmpty(); + ConfigReadRetryTelemetry(tracer).ShouldBeEmpty(); } [TestCase] @@ -143,7 +147,7 @@ public void CredentialFillDoesNotRetryWhenTheTimeoutCannotCoverTheDelay() gitProcess.TryGetCredential(tracer, "mock://repoUrl", out _, out _, out _, timeoutMs: 1).ShouldBeFalse(); gitProcess.InvocationsRun.Count.ShouldEqual(1); - ((bool)tracer.RelatedEventMetadata.Single()["WillRetry"]).ShouldBeFalse(); + ConfigReadRetryTelemetry(tracer).Single()["Attempts"].ShouldEqual(1); } [TestCase(true, 30000)] @@ -430,13 +434,21 @@ public void TryGetCredentialFailsWhenNoLineStartsWithThePasswordKey(string outpu password.ShouldBeNull(); } - private static void AssertConfigReadFailureTelemetry(MockTracer tracer) + // The GitConfigReadRetry events are the telemetry warnings that name a config read. + private static List ConfigReadRetryTelemetry(MockTracer tracer) { - foreach (EventMetadata metadata in tracer.RelatedEventMetadata) + return tracer.TelemetryWarnings.Where(metadata => metadata.ContainsKey("ConfigName")).ToList(); + } + + private static void AssertConfigReadRetryTelemetryHasNoStderr(MockTracer tracer) + { + foreach (EventMetadata metadata in ConfigReadRetryTelemetry(tracer)) { - CollectionAssert.AreEquivalent( - new[] { "AttemptNumber", "MaxAttempts", "WillRetry", "RetryDelayMilliseconds" }, - metadata.Keys); + foreach (string key in metadata.Keys) + { + new[] { "ConfigName", "AttemptNumber", "MaxAttempts", "RetryDelayMilliseconds", "Attempts", "WarningMessage" } + .ShouldContain(allowed => allowed == key); + } } } diff --git a/GVFS/GVFS.UnitTests/Common/GitConfigHelperTests.cs b/GVFS/GVFS.UnitTests/Common/GitConfigHelperTests.cs index 6f0a6c141..4980fce7e 100644 --- a/GVFS/GVFS.UnitTests/Common/GitConfigHelperTests.cs +++ b/GVFS/GVFS.UnitTests/Common/GitConfigHelperTests.cs @@ -9,6 +9,29 @@ namespace GVFS.UnitTests.Common [TestFixture] public class GitConfigHelperTests { + [TestCase(null, 5)] + [TestCase("", 5)] + [TestCase(" ", 5)] + [TestCase("7", 7)] + [TestCase(" 7 ", 7)] + [TestCase("0", 0)] + public void TryParseIntSettingReturnsTheValueOrTheDefault(string valueString, int expected) + { + GitConfigHelper.TryParseIntSetting("gvfs.test", valueString, 5, 0, out int value, out string error).ShouldBeTrue(error); + value.ShouldEqual(expected); + error.ShouldEqual(string.Empty); + } + + [TestCase("abc", "Misconfigured config setting gvfs.test, could not parse value `abc` as an int")] + [TestCase("1k", "Misconfigured config setting gvfs.test, could not parse value `1k` as an int")] + [TestCase("99999999999", "Misconfigured config setting gvfs.test, could not parse value `99999999999` as an int")] + [TestCase("-1", "Invalid value -1 for setting gvfs.test, value must be greater than or equal to 0")] + public void TryParseIntSettingReportsInvalidValues(string valueString, string expectedError) + { + GitConfigHelper.TryParseIntSetting("gvfs.test", valueString, 5, 0, out int _, out string error).ShouldBeFalse(); + error.ShouldEqual(expectedError); + } + [TestCase] public void SanitizeEmptyString() { diff --git a/GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs b/GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs index 843d423b5..7fe8ba564 100644 --- a/GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs +++ b/GVFS/GVFS.UnitTests/Common/LibGit2RepoConfigLookupTests.cs @@ -3,6 +3,8 @@ using GVFS.UnitTests.Mock.Common; using NUnit.Framework; using System; +using System.Collections; +using System.Collections.Generic; using System.IO; namespace GVFS.UnitTests.Common @@ -10,6 +12,9 @@ namespace GVFS.UnitTests.Common [TestFixture] public class LibGit2RepoConfigLookupTests { + private const string LockedConfigError = + "Failed to get config handle: could not open 'C:\\repo\\.git\\config': The process cannot access the file because it is being used by another process."; + [TestCase] public void GetConfigBoolOrDefaultOnRepoReturnsConfiguredValue() { @@ -68,6 +73,186 @@ public void GetConfigBoolOrDefaultOnRepoReturnsDefaultOnInvalidDataExceptionAndL } } + [TestCase("GIT_CONFIG_PARAMETERS")] + [TestCase("GIT_CONFIG_COUNT")] + [TestCase("GIT_CONFIG_GLOBAL")] + [TestCase("GIT_CONFIG_SYSTEM")] + [TestCase("GIT_CONFIG_NOSYSTEM")] + [TestCase("GIT_CONFIG")] + [TestCase("git_config_parameters")] + public void EnvironmentChangesGitConfigDetectsVariablesThatLibGit2Ignores(string name) + { + Hashtable environment = new Hashtable { { "PATH", "C:\\bin" }, { name, "1" } }; + + LibGit2Repo.EnvironmentChangesGitConfig(environment).ShouldBeTrue(); + } + + [TestCase] + public void EnvironmentChangesGitConfigIgnoresOtherGitVariables() + { + Hashtable environment = new Hashtable + { + { "GIT_DIR", "C:\\repo\\.git\\worktrees\\wt" }, + { "GIT_TRACE", "1" }, + { "GIT_CONFIG_KEY_0", "gvfs.max-retries" }, + { "GIT_WORK_TREE", "C:\\repo" }, + }; + + LibGit2Repo.EnvironmentChangesGitConfig(environment).ShouldBeFalse(); + } + + [TestCase] + public void TryGetConfigStringReportsAnUnopenableRepository() + { + string missingRepoPath = Path.Combine( + Path.GetTempPath(), + "LibGit2RepoConfigLookupTests_" + Guid.NewGuid().ToString("N")); + + LibGit2Repo.TryGetConfigString(new MockTracer(), missingRepoPath, "gvfs.test", out string value, out string error).ShouldBeFalse(); + + value.ShouldBeNull(); + error.ShouldContain("Error while reading 'gvfs.test' from config: Couldn't open repo at"); + } + + [TestCase] + public void GetConfigBoolRetriesTransientLibGit2Failure() + { + MockTracer tracer = new MockTracer(); + int delayCount = 0; + Queue results = new Queue(); + results.Enqueue(new LibGit2Exception(LockedConfigError)); + results.Enqueue(true); + + using (MockConfigRepo repo = new MockConfigRepo(tracer, results, delay => ++delayCount)) + { + repo.GetConfigBool("gvfs.test").ShouldEqual(true); + } + + delayCount.ShouldEqual(1); + tracer.RelatedWarningEvents.Count.ShouldEqual(1); + tracer.RelatedWarningEvents[0].ShouldContain("Transient failure while reading git config"); + } + + [TestCase] + public void GetConfigStringRetriesTransientLibGit2Failure() + { + int delayCount = 0; + Queue results = new Queue(); + results.Enqueue(new LibGit2Exception(LockedConfigError)); + results.Enqueue("configured-value"); + + using (MockConfigRepo repo = new MockConfigRepo(new MockTracer(), results, delay => ++delayCount)) + { + repo.GetConfigString("gvfs.test").ShouldEqual("configured-value"); + } + + delayCount.ShouldEqual(1); + } + + [TestCase] + public void GetConfigBoolRethrowsAfterRetryBudget() + { + MockTracer tracer = new MockTracer(); + int delayCount = 0; + Queue results = new Queue(); + for (int i = 0; i < GitConfigReadRetry.MaxAttempts; ++i) + { + results.Enqueue(new LibGit2Exception(LockedConfigError)); + } + + using (MockConfigRepo repo = new MockConfigRepo(tracer, results, delay => ++delayCount)) + { + Assert.Throws(() => repo.GetConfigBool("gvfs.test")); + } + + delayCount.ShouldEqual(GitConfigReadRetry.MaxAttempts - 1); + results.Count.ShouldEqual(0); + tracer.RelatedWarningEvents.Count.ShouldEqual(GitConfigReadRetry.MaxAttempts); + tracer.RelatedWarningEvents[GitConfigReadRetry.MaxAttempts - 1].ShouldContain("persisted after all attempts"); + } + + [TestCase] + public void GetConfigBoolDoesNotRetryNonTransientLibGit2Failure() + { + int delayCount = 0; + Queue results = new Queue(); + results.Enqueue(new LibGit2Exception("Failed to get config handle: could not open 'C:\\repo\\.git\\config': The system cannot find the file specified.")); + results.Enqueue(true); + + using (MockConfigRepo repo = new MockConfigRepo(new MockTracer(), results, delay => ++delayCount)) + { + Assert.Throws(() => repo.GetConfigBool("gvfs.test")); + } + + delayCount.ShouldEqual(0); + results.Count.ShouldEqual(1); + } + + [TestCase] + public void ForEachMultiVarConfigInvokesCallbackOncePerValueAcrossRetries() + { + int delayCount = 0; + Queue results = new Queue(); + results.Enqueue(new LibGit2Exception(LockedConfigError)); + results.Enqueue(new List { "first", "second" }); + List callbackValues = new List(); + + using (MockConfigRepo repo = new MockConfigRepo(new MockTracer(), results, delay => ++delayCount)) + { + repo.ForEachMultiVarConfig("safe.directory", callbackValues.Add); + } + + delayCount.ShouldEqual(1); + callbackValues.ShouldMatchInOrder("first", "second"); + } + + [TestCase] + public void RepositoryOpenRetriesTransientFailure() + { + MockTracer tracer = new MockTracer(); + int delayCount = 0; + Queue openErrors = new Queue(); + openErrors.Enqueue("could not open 'C:\\repo\\.git\\config': The process cannot access the file because it is being used by another process."); + openErrors.Enqueue("could not open 'C:\\repo\\.git\\config': Permission denied"); + + using (MockOpenRepo.Create(tracer, openErrors, delay => ++delayCount, GitConfigReadRetry.MaxAttempts)) + { + } + + delayCount.ShouldEqual(2); + openErrors.Count.ShouldEqual(0); + } + + [TestCase] + public void RepositoryOpenStopsAfterConfiguredAttempts() + { + int delayCount = 0; + Queue openErrors = new Queue(); + for (int i = 0; i < 5; ++i) + { + openErrors.Enqueue("could not open 'C:\\repo\\.git\\config': Permission denied"); + } + + Assert.Throws( + () => MockOpenRepo.Create(new MockTracer(), openErrors, delay => ++delayCount, maxAttempts: 2)); + + delayCount.ShouldEqual(1); + openErrors.Count.ShouldEqual(3); + } + + [TestCase] + public void RepositoryOpenDoesNotRetryNonTransientFailure() + { + int delayCount = 0; + Queue openErrors = new Queue(); + openErrors.Enqueue("could not find repository at 'C:\\repo'"); + + Assert.Throws( + () => MockOpenRepo.Create(new MockTracer(), openErrors, delay => ++delayCount, GitConfigReadRetry.MaxAttempts)); + + delayCount.ShouldEqual(0); + } + [TestCase] public void GetConfigBoolOrDefaultOnPathReturnsDefaultForMissingRepoAndLogsExactlyOnce() { @@ -100,6 +285,7 @@ private class MockConfigRepo : LibGit2Repo { private readonly bool? value; private readonly Exception exceptionToThrow; + private readonly Queue results; public MockConfigRepo(MockTracer tracer, bool? value) : base(tracer) @@ -113,8 +299,25 @@ public MockConfigRepo(MockTracer tracer, Exception exceptionToThrow) this.exceptionToThrow = exceptionToThrow; } - public override bool? GetConfigBool(string name) + public MockConfigRepo(MockTracer tracer, Queue results, Action configReadDelay) + : base(tracer, configReadDelay) + { + this.results = results; + } + + protected override bool? GetConfigBoolOnce(string name) { + if (this.results != null) + { + object result = this.results.Dequeue(); + if (result is Exception exception) + { + throw exception; + } + + return (bool?)result; + } + if (this.exceptionToThrow != null) { throw this.exceptionToThrow; @@ -123,6 +326,28 @@ public MockConfigRepo(MockTracer tracer, Exception exceptionToThrow) return this.value; } + protected override string GetConfigStringOnce(string name) + { + object result = this.results.Dequeue(); + if (result is Exception exception) + { + throw exception; + } + + return (string)result; + } + + protected override List GetMultiVarConfigValues(string key) + { + object result = this.results.Dequeue(); + if (result is Exception exception) + { + throw exception; + } + + return (List)result; + } + // This mock never calls the base LibGit2Repo(tracer, repoPath) constructor, so it // never initializes native libgit2 state or a real RepoHandle. Override Dispose(bool) // to skip the base implementation's native Free/Shutdown calls, which would otherwise @@ -132,5 +357,58 @@ protected override void Dispose(bool disposing) { } } + + // Exercises the real LibGit2Repo(tracer, repoPath, ...) constructor with the native open + // replaced. State is passed through thread-static fields because the base constructor + // calls the virtual members before derived constructors run. + private class MockOpenRepo : LibGit2Repo + { + [ThreadStatic] + private static Queue pendingOpenErrors; + + private MockOpenRepo(MockTracer tracer, Action configReadDelay, int maxAttempts) + : base(tracer, "C:\\repo", configReadDelay, maxAttempts) + { + } + + private string LastOpenError { get; set; } + + public static MockOpenRepo Create(MockTracer tracer, Queue openErrors, Action configReadDelay, int maxAttempts) + { + pendingOpenErrors = openErrors; + try + { + return new MockOpenRepo(tracer, configReadDelay, maxAttempts); + } + finally + { + pendingOpenErrors = null; + } + } + + protected override void InitNative() + { + } + + protected override void ShutdownNative() + { + } + + protected override Native.ResultCode TryOpenRepo(string path, out IntPtr repoHandle) + { + repoHandle = IntPtr.Zero; + this.LastOpenError = pendingOpenErrors.Count > 0 ? pendingOpenErrors.Dequeue() : null; + return this.LastOpenError == null ? Native.ResultCode.Success : Native.ResultCode.Failure; + } + + protected override string GetLastNativeError() + { + return this.LastOpenError; + } + + protected override void Dispose(bool disposing) + { + } + } } } diff --git a/GVFS/GVFS.UnitTests/Common/RetryConfigTests.cs b/GVFS/GVFS.UnitTests/Common/RetryConfigTests.cs index ad619d354..64ab99190 100644 --- a/GVFS/GVFS.UnitTests/Common/RetryConfigTests.cs +++ b/GVFS/GVFS.UnitTests/Common/RetryConfigTests.cs @@ -159,6 +159,11 @@ public void CommandOverridesDoNotDiscardLoadedReadTimeout() { using MockTracer tracer = new MockTracer(); MockGitProcess git = CreateGitProcess(); + + // The enlistment's working directory does not exist, so libgit2 reads the isolated + // global config. git.exe reads the same values when the environment changes Git config. + using IsolatedLibGit2Config machineConfig = new IsolatedLibGit2Config(); + File.WriteAllText(machineConfig.GlobalConfigPath, "[gvfs]\n\tmax-retries = 9\n\ttimeout-seconds = 45\n\tread-timeout-seconds = 90\n"); RetryConfig config = new RetryConfigVerb().Load( tracer, new MockGVFSEnlistment(git), TimeSpan.FromMinutes(10)); config.MaxRetries.ShouldEqual(9); @@ -190,6 +195,414 @@ public void CompleteLoaderRejectsMissingDependencies() Assert.Throws(() => RetryConfig.LoadFromGitConfig(null, (Enlistment)null)); } + [TestCase] + public void LibGit2ReaderReadsRepositoryValues() + { + using (TestRepository repo = new TestRepository("[gvfs]\n\tmax-retries = 7\n\ttimeout-seconds = 31\n\tread-timeout-seconds = 0\n")) + { + MockTracer tracer = new MockTracer(); + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, RetryConfig.CreateLibGit2Reader(tracer, repo.WorkingDirectory)); + + config.MaxRetries.ShouldEqual(7); + config.Timeout.ShouldEqual(TimeSpan.FromSeconds(31)); + config.ReadTimeout.ShouldEqual(System.Threading.Timeout.InfiniteTimeSpan); + tracer.RelatedWarningEvents.Count.ShouldEqual(0); + } + } + + [TestCase] + public void LibGit2ReaderUsesDefaultValuesWhenEntriesAreBlank() + { + using (TestRepository repo = new TestRepository("[gvfs]\n\tmax-retries =\n\ttimeout-seconds\n")) + { + MockTracer tracer = new MockTracer(); + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, RetryConfig.CreateLibGit2Reader(tracer, repo.WorkingDirectory)); + + config.MaxRetries.ShouldEqual(RetryConfig.DefaultMaxRetries); + config.Timeout.ShouldEqual(TimeSpan.FromSeconds(RetryConfig.DefaultTimeoutSeconds)); + config.ReadTimeout.ShouldEqual(TimeSpan.FromSeconds(RetryConfig.DefaultReadTimeoutSeconds)); + tracer.RelatedWarningEvents.Count.ShouldEqual(0); + } + } + + [TestCase("max-retries = abc", "gvfs.max-retries", "Misconfigured config setting gvfs.max-retries, could not parse value `abc` as an int")] + [TestCase("timeout-seconds = 0", "gvfs.timeout-seconds", "Invalid value 0 for setting gvfs.timeout-seconds, value must be greater than or equal to 1")] + [TestCase("read-timeout-seconds = 2147484", "gvfs.read-timeout-seconds", "Invalid value 2147484 for setting gvfs.read-timeout-seconds, value must be between 0 and 2147483")] + public void LibGit2ReaderReportsTheSameInvalidValueErrorsAsGit(string setting, string configName, string expectedError) + { + using (TestRepository repo = new TestRepository("[gvfs]\n\t" + setting + "\n")) + { + MockTracer tracer = new MockTracer(); + RetryConfig.LoadFromGitConfig(tracer, RetryConfig.CreateLibGit2Reader(tracer, repo.WorkingDirectory)); + + tracer.RelatedWarningEvents.Count.ShouldEqual(1); + tracer.RelatedWarningEvents[0].ShouldContain("\"ConfigName\":\"" + configName + "\"", expectedError.Replace("`", "\\u0060")); + } + } + + [TestCase] + public void LibGit2ReaderUsesDefaultsWhenConfigIsCorrupt() + { + using (TestRepository repo = new TestRepository("[gvfs\n")) + { + MockTracer tracer = new MockTracer(); + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, RetryConfig.CreateLibGit2Reader(tracer, repo.WorkingDirectory)); + + config.MaxRetries.ShouldEqual(RetryConfig.DefaultMaxRetries); + config.Timeout.ShouldEqual(TimeSpan.FromSeconds(RetryConfig.DefaultTimeoutSeconds)); + List settingWarnings = tracer.RelatedWarningEvents.FindAll(warning => warning.Contains("The retry setting is invalid or unavailable")); + settingWarnings.Count.ShouldEqual(3); + settingWarnings[0].ShouldContain("Error while reading", "failed to parse config file"); + } + } + + [TestCase] + public void GitFallbackReadsWithGitWhenLibGit2Fails() + { + MockTracer tracer = new MockTracer(); + List gitReads = new List(); + RetryConfig.ConfigStringReader readConfig = RetryConfig.WithGitFallback( + tracer, + (string name, out string value, out string error) => + { + value = null; + error = "Error while reading '" + name + "' from config: Couldn't open repo"; + return false; + }, + (string name, out string value, out string error) => + { + gitReads.Add(name); + value = "4"; + error = string.Empty; + return true; + }); + + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, readConfig); + + config.MaxRetries.ShouldEqual(4); + config.Timeout.ShouldEqual(TimeSpan.FromSeconds(4)); + config.ReadTimeout.ShouldEqual(TimeSpan.FromSeconds(4)); + gitReads.ShouldMatchInOrder("gvfs.max-retries", "gvfs.timeout-seconds", "gvfs.read-timeout-seconds"); + int readWithGitEvent = tracer.RelatedEventNames.IndexOf("RetryConfig_ReadWithGit"); + readWithGitEvent.ShouldEqual(0); + tracer.RelatedEventNames.FindAll(eventName => eventName == "RetryConfig_ReadWithGit").Count.ShouldEqual(3); + EventMetadata metadata = tracer.RelatedEventMetadata[readWithGitEvent]; + metadata["Area"].ShouldEqual("RetryConfig"); + metadata["ConfigName"].ShouldEqual("gvfs.max-retries"); + ((string)metadata["LibGit2Error"]).ShouldContain("Couldn't open repo"); + tracer.RelatedWarningEvents.Count.ShouldEqual(0); + } + + [TestCase] + public void GitFallbackDoesNotReadAgainAfterATransientFailure() + { + MockTracer tracer = new MockTracer(); + int gitReads = 0; + RetryConfig.ConfigStringReader readConfig = RetryConfig.WithGitFallback( + tracer, + (string name, out string value, out string error) => + { + value = null; + error = "Error while reading '" + name + "' from config: could not open 'C:/repo/.git/config': " + + "The process cannot access the file because it is being used by another process."; + return false; + }, + (string name, out string value, out string error) => + { + ++gitReads; + value = null; + error = null; + return true; + }); + + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, readConfig); + + gitReads.ShouldEqual(0); + config.MaxRetries.ShouldEqual(RetryConfig.DefaultMaxRetries); + tracer.RelatedWarningEvents.Count.ShouldEqual(3); + tracer.RelatedWarningEvents[0].ShouldContain("being used by another process"); + } + + [TestCase] + public void GitFallbackReportsTheGitErrorWhenBothReadersFail() + { + MockTracer tracer = new MockTracer(); + RetryConfig.ConfigStringReader readConfig = RetryConfig.WithGitFallback( + tracer, + (string name, out string value, out string error) => + { + value = null; + error = "libgit2 error"; + return false; + }, + (string name, out string value, out string error) => + { + value = null; + error = "git error"; + return false; + }); + + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, readConfig); + + config.MaxRetries.ShouldEqual(RetryConfig.DefaultMaxRetries); + tracer.RelatedWarningEvents.Count.ShouldEqual(3); + tracer.RelatedWarningEvents[0].ShouldContain("\"Error\":\"git error\""); + } + + [TestCase] + public void GitFallbackDoesNotReadWithGitWhenLibGit2Succeeds() + { + MockTracer tracer = new MockTracer(); + int gitReads = 0; + RetryConfig.ConfigStringReader readConfig = RetryConfig.WithGitFallback( + tracer, + (string name, out string value, out string error) => + { + value = null; + error = null; + return true; + }, + (string name, out string value, out string error) => + { + ++gitReads; + value = null; + error = null; + return true; + }); + + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, readConfig); + + config.MaxRetries.ShouldEqual(RetryConfig.DefaultMaxRetries); + gitReads.ShouldEqual(0); + tracer.RelatedEventNames.Contains("RetryConfig_ReadWithGit").ShouldBeFalse(); + } + + [TestCase] + public void LibGit2ReaderReadsGlobalConfigBeforeTheRepositoryExists() + { + using (IsolatedLibGit2Config machineConfig = new IsolatedLibGit2Config()) + { + File.WriteAllText(machineConfig.GlobalConfigPath, "[gvfs]\n\tmax-retries = 9\n\ttimeout-seconds = 41\n\tread-timeout-seconds = 42\n"); + string missingWorkingDirectory = Path.Combine(Path.GetTempPath(), "RetryConfigTests_" + Guid.NewGuid().ToString("N")); + + MockTracer tracer = new MockTracer(); + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, RetryConfig.CreateLibGit2Reader(tracer, missingWorkingDirectory)); + + config.MaxRetries.ShouldEqual(9); + config.Timeout.ShouldEqual(TimeSpan.FromSeconds(41)); + config.ReadTimeout.ShouldEqual(TimeSpan.FromSeconds(42)); + tracer.RelatedWarningEvents.Count.ShouldEqual(0); + } + } + + [TestCase] + public void UserConfigThatGitDoesNotReadIsReadWithGit() + { + string gitPath = GetGitPathOrIgnore(); + string[] variables = { "HOMEDRIVE", "HOMEPATH", "USERPROFILE" }; + string[] originalValues = Array.ConvertAll(variables, Environment.GetEnvironmentVariable); + using (IsolatedLibGit2Config machineConfig = new IsolatedLibGit2Config()) + { + // git reads only HOME\.gitconfig. libgit2 also falls back to USERPROFILE\.gitconfig. + string emptyHome = Path.Combine(machineConfig.Directory, "home"); + string profile = Path.Combine(machineConfig.Directory, "profile"); + Directory.CreateDirectory(emptyHome); + Directory.CreateDirectory(profile); + File.WriteAllText(Path.Combine(profile, ".gitconfig"), "[gvfs]\n\ttimeout-seconds = 1\n"); + try + { + Environment.SetEnvironmentVariable("HOME", emptyHome); + Environment.SetEnvironmentVariable("HOMEDRIVE", null); + Environment.SetEnvironmentVariable("HOMEPATH", null); + Environment.SetEnvironmentVariable("USERPROFILE", profile); + IsolatedLibGit2Config.ResetUserSearchPathsFromEnvironment(); + + string missingWorkingDirectory = Path.Combine(Path.GetTempPath(), "RetryConfigTests_" + Guid.NewGuid().ToString("N")); + MockTracer tracer = new MockTracer(); + RetryConfig.ConfigStringReader readWithLibGit2 = RetryConfig.CreateLibGit2Reader(tracer, missingWorkingDirectory); + GitProcess git = new GitProcess(gitPath, workingDirectoryRoot: null); + RetryConfig.ConfigStringReader readWithGit = (string name, out string value, out string error) => + git.GetFromConfig(name).TryParseAsString(out value, out error); + + readWithLibGit2("gvfs.timeout-seconds", out string _, out string libGit2Error).ShouldBeFalse(); + libGit2Error.ShouldContain("libgit2 reads the global config file", Path.Combine(profile, ".gitconfig")); + readWithGit("gvfs.timeout-seconds", out string gitValue, out string gitError).ShouldBeTrue(gitError); + gitValue.ShouldBeNull(); + + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, RetryConfig.WithGitFallback(tracer, readWithLibGit2, readWithGit)); + config.Timeout.ShouldEqual(TimeSpan.FromSeconds(RetryConfig.DefaultTimeoutSeconds)); + } + finally + { + for (int i = 0; i < variables.Length; ++i) + { + Environment.SetEnvironmentVariable(variables[i], originalValues[i]); + } + } + } + } + + [TestCase] + public void DisposingARepositoryWithoutNativeStateKeepsTheIsolatedConfig() + { + using (IsolatedLibGit2Config machineConfig = new IsolatedLibGit2Config()) + { + File.WriteAllText(machineConfig.GlobalConfigPath, "[gvfs]\n\tmax-retries = 9\n"); + + // This repository never initialized libgit2, so disposing it must not shut libgit2 down. + new MockLibGit2Repo(new MockTracer()).Dispose(); + + string missingWorkingDirectory = Path.Combine(Path.GetTempPath(), "RetryConfigTests_" + Guid.NewGuid().ToString("N")); + RetryConfig.CreateLibGit2Reader(new MockTracer(), missingWorkingDirectory)("gvfs.max-retries", out string value, out string error) + .ShouldBeTrue(error); + value.ShouldEqual("9"); + } + } + + [TestCase] + public void LibGit2ReaderRejectsAGlobalIncludeConditionItIgnores() + { + using (IsolatedLibGit2Config machineConfig = new IsolatedLibGit2Config()) + { + File.WriteAllText( + machineConfig.GlobalConfigPath, + "[includeIf \"hasconfig:remote.*.url:https://example.com/**\"]\n\tpath = C:/does-not-matter.gitconfig\n"); + string missingWorkingDirectory = Path.Combine(Path.GetTempPath(), "RetryConfigTests_" + Guid.NewGuid().ToString("N")); + + RetryConfig.CreateLibGit2Reader(new MockTracer(), missingWorkingDirectory)("gvfs.max-retries", out string _, out string error) + .ShouldBeFalse(); + + error.ShouldContain("libgit2 does not evaluate like git: includeif.hasconfig:"); + } + } + + [TestCase] + public void LibGit2AndGitReadTheSameRepositoryValues() + { + string gitPath = GetGitPathOrIgnore(); + using (TestRepository repo = new TestRepository("[gvfs]\n\tmax-retries = 7\n\tmax-retries = 8\n\ttimeout-seconds = 31\n\tread-timeout-seconds = 32\n")) + { + RetryConfig libGit2Config = RetryConfig.LoadFromGitConfig(new MockTracer(), RetryConfig.CreateLibGit2Reader(new MockTracer(), repo.WorkingDirectory)); + RetryConfig gitConfig = RetryConfig.LoadFromGitConfig(new MockTracer(), new GitProcess(gitPath, repo.WorkingDirectory)); + + libGit2Config.MaxRetries.ShouldEqual(8); + gitConfig.MaxRetries.ShouldEqual(8); + libGit2Config.Timeout.ShouldEqual(TimeSpan.FromSeconds(31)); + gitConfig.Timeout.ShouldEqual(TimeSpan.FromSeconds(31)); + libGit2Config.ReadTimeout.ShouldEqual(TimeSpan.FromSeconds(32)); + gitConfig.ReadTimeout.ShouldEqual(TimeSpan.FromSeconds(32)); + } + } + + [TestCase] + public void ConfigWithAnIncludeConditionLibGit2IgnoresIsReadWithGit() + { + string gitPath = GetGitPathOrIgnore(); + using (TestRepository repo = new TestRepository(string.Empty)) + { + string includedPath = Path.Combine(repo.WorkingDirectory, "included.gitconfig"); + File.WriteAllText(includedPath, "[gvfs]\n\tmax-retries = 12\n"); + File.AppendAllText( + Path.Combine(repo.WorkingDirectory, ".git", "config"), + "[remote \"origin\"]\n\turl = https://example.com/repo\n" + + "[includeIf \"hasconfig:remote.*.url:https://example.com/**\"]\n\tpath = " + includedPath.Replace('\\', '/') + "\n"); + + MockTracer tracer = new MockTracer(); + RetryConfig.ConfigStringReader readWithLibGit2 = RetryConfig.CreateLibGit2Reader(tracer, repo.WorkingDirectory); + GitProcess git = new GitProcess(gitPath, repo.WorkingDirectory); + RetryConfig.ConfigStringReader readWithGit = (string name, out string value, out string error) => + git.GetFromConfig(name).TryParseAsString(out value, out error); + + readWithLibGit2("gvfs.max-retries", out string _, out string libGit2Error).ShouldBeFalse(); + libGit2Error.ShouldContain("libgit2 does not evaluate like git: includeif.hasconfig:remote.*.url:https://example.com/**.path"); + + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, RetryConfig.WithGitFallback(tracer, readWithLibGit2, readWithGit)); + config.MaxRetries.ShouldEqual(12); + tracer.RelatedWarningEvents.Count.ShouldEqual(0); + } + } + + // In each layout, libgit2 evaluates the include differently from git.exe. + [TestCase("onbranch:main", false)] + [TestCase("onbranch:main", true)] + [TestCase("gitdir:", false)] + [TestCase("gitdir/i:", false)] + public void IncludeLayoutsLibGit2EvaluatesDifferentlyAreReadWithGit(string condition, bool headOnBranch) + { + string gitPath = GetGitPathOrIgnore(); + using (TestRepository repo = new TestRepository(string.Empty)) + { + if (!headOnBranch) + { + File.WriteAllText(Path.Combine(repo.WorkingDirectory, ".git", "HEAD"), new string('1', 40) + "\n"); + } + + string gitDir = Path.Combine(repo.WorkingDirectory, ".git").Replace('\\', '/') + "/"; + string includedPath = Path.Combine(repo.WorkingDirectory, "included.gitconfig"); + File.WriteAllText(includedPath, "[gvfs]\n\tmax-retries = 13\n"); + File.AppendAllText( + Path.Combine(repo.WorkingDirectory, ".git", "config"), + "[includeIf \"" + condition.Replace("", gitDir.Substring(2)) + "\"]\n\tpath = " + + includedPath.Replace('\\', '/') + "\n"); + + MockTracer tracer = new MockTracer(); + RetryConfig.ConfigStringReader readWithLibGit2 = RetryConfig.CreateLibGit2Reader(tracer, repo.WorkingDirectory); + GitProcess git = new GitProcess(gitPath, repo.WorkingDirectory); + RetryConfig.ConfigStringReader readWithGit = (string name, out string value, out string error) => + git.GetFromConfig(name).TryParseAsString(out value, out error); + + readWithLibGit2("gvfs.max-retries", out string _, out string libGit2Error).ShouldBeFalse(); + libGit2Error.ShouldContain("libgit2 does not evaluate like git"); + + readWithGit("gvfs.max-retries", out string gitValue, out string gitError).ShouldBeTrue(gitError); + RetryConfig config = RetryConfig.LoadFromGitConfig(tracer, RetryConfig.WithGitFallback(tracer, readWithLibGit2, readWithGit)); + config.MaxRetries.ShouldEqual(gitValue == null ? RetryConfig.DefaultMaxRetries : int.Parse(gitValue)); + config.MaxRetries.ShouldEqual(headOnBranch && condition.StartsWith("onbranch:") ? 13 : RetryConfig.DefaultMaxRetries); + tracer.RelatedWarningEvents.Count.ShouldEqual(0); + } + } + + // git.exe expands %(prefix) to its installation directory. libgit2 skips the include. + [TestCase("[include]\n\tpath = %(prefix)/etc/gvfs.gitconfig\n")] + [TestCase("[includeIf \"gitdir:**/\"]\n\tpath = %(prefix)/etc/gvfs.gitconfig\n")] + public void IncludePathsWithAGitPrefixAreReadWithGit(string includeConfig) + { + using (TestRepository repo = new TestRepository(includeConfig)) + { + RetryConfig.CreateLibGit2Reader(new MockTracer(), repo.WorkingDirectory)("gvfs.max-retries", out string _, out string error) + .ShouldBeFalse(); + error.ShouldContain("libgit2 does not evaluate like git", "%(prefix)/etc/gvfs.gitconfig"); + } + } + + [TestCase("gitdir:**/")] + [TestCase("gitdir/i:**/")] + public void LibGit2ReadsConfigWithIncludeConditionsItEvaluates(string condition) + { + using (TestRepository repo = new TestRepository(string.Empty)) + { + string includedPath = Path.Combine(repo.WorkingDirectory, "included.gitconfig"); + File.WriteAllText(includedPath, "[gvfs]\n\tmax-retries = 13\n"); + File.AppendAllText( + Path.Combine(repo.WorkingDirectory, ".git", "config"), + "[includeIf \"" + condition + "\"]\n\tpath = " + includedPath.Replace('\\', '/') + "\n"); + + RetryConfig.CreateLibGit2Reader(new MockTracer(), repo.WorkingDirectory)("gvfs.max-retries", out string value, out string error) + .ShouldBeTrue(error); + value.ShouldEqual("13"); + } + } + + private static string GetGitPathOrIgnore() + { + string gitPath = "C:\\Program Files\\Git\\cmd\\git.exe"; + if (!File.Exists(gitPath)) + { + Assert.Ignore("Git not found at expected path — skipping integration test"); + } + + return gitPath; + } private class RetryConfigVerb : GVFSVerb { public override string EnlistmentRootPathParameter { get; set; } @@ -246,5 +659,33 @@ public override ConfigResult GetFromConfig( return base.GetFromConfig(settingName, forceOutsideEnlistment, fileSystem); } } + + // A minimal repository that libgit2 can open without git.exe. libgit2 does not read the + // machine's config while the repository exists. The non-ASCII path checks the UTF-8 + // marshalling of the repository path. + private sealed class TestRepository : IDisposable + { + private readonly IsolatedLibGit2Config machineConfig = new IsolatedLibGit2Config(); + + public TestRepository(string extraConfig) + { + this.WorkingDirectory = Path.Combine(Path.GetTempPath(), "RetryConfigTests_ü_" + Guid.NewGuid().ToString("N")); + string dotGit = Path.Combine(this.WorkingDirectory, ".git"); + Directory.CreateDirectory(Path.Combine(dotGit, "objects")); + Directory.CreateDirectory(Path.Combine(dotGit, "refs", "heads")); + File.WriteAllText(Path.Combine(dotGit, "HEAD"), "ref: refs/heads/main\n"); + File.WriteAllText( + Path.Combine(dotGit, "config"), + "[core]\n\trepositoryformatversion = 0\n\tbare = false\n" + extraConfig); + } + + public string WorkingDirectory { get; } + + public void Dispose() + { + Directory.Delete(this.WorkingDirectory, recursive: true); + this.machineConfig.Dispose(); + } + } } } diff --git a/GVFS/GVFS.UnitTests/Git/GitConfigReadRetryTests.cs b/GVFS/GVFS.UnitTests/Git/GitConfigReadRetryTests.cs new file mode 100644 index 000000000..fffe3349c --- /dev/null +++ b/GVFS/GVFS.UnitTests/Git/GitConfigReadRetryTests.cs @@ -0,0 +1,168 @@ +using GVFS.Common.Git; +using GVFS.Tests.Should; +using GVFS.UnitTests.Mock.Common; +using NUnit.Framework; +using System; +using System.Collections.Generic; + +namespace GVFS.UnitTests.Git +{ + [TestFixture] + public class GitConfigReadRetryTests + { + [TestCase("warning: unable to access '.git/config': Permission denied", true)] + [TestCase("warning: unable to access '.git/config': Permission denied\r\nfatal: unknown error occurred while reading the configuration files", true)] + [TestCase("could not open 'C:\\repo\\.git\\config': The process cannot access the file because it is being used by another process.", true)] + [TestCase("error: unable to access 'C:/repo/.git/config': Device or resource busy", true)] + [TestCase("fatal: unable to access 'C:/Permission denied/.git/config': No such file or directory", false)] + [TestCase("error: could not open 'C:/Device or resource busy/config': The system cannot find the file specified.", false)] + [TestCase("Permission denied", false)] + [TestCase("fatal: bad config line 4 in file .git/config", false)] + [TestCase("", false)] + [TestCase(null, false)] + public void IsTransientFileAccessErrorMatchesOnlyTheFailureReason(string error, bool expected) + { + GitConfigReadRetry.IsTransientFileAccessError(error).ShouldEqual(expected); + } + + [TestCase] + public void IsTransientFileAccessErrorHonorsRequiredLinePhrase() + { + const string Error = "warning: could not open 'config': Permission denied"; + + GitConfigReadRetry.IsTransientFileAccessError(Error).ShouldBeTrue(); + GitConfigReadRetry.IsTransientFileAccessError(Error, lineMustContain: "unable to access").ShouldBeFalse(); + } + + [TestCase] + public void InvokeResultStopsAtTheRequestedAttemptLimit() + { + MockTracer tracer = new MockTracer(); + int reads = 0; + int delays = 0; + + int result = GitConfigReadRetry.InvokeResult( + "gvfs.test", + () => ++reads, + (int _) => true, + tracer, + delay => ++delays, + maxAttempts: 2); + + result.ShouldEqual(2); + reads.ShouldEqual(2); + delays.ShouldEqual(1); + } + + [TestCase] + public void InvokeResultStopsWhenTheCallerCannotWaitForTheDelay() + { + MockTracer tracer = new MockTracer(); + List requestedWaits = new List(); + List retriedResults = new List(); + int reads = 0; + int delays = 0; + + GitConfigReadRetry.InvokeResult( + "gvfs.test", + () => ++reads, + (int _) => true, + tracer, + delay => ++delays, + canWait: wait => + { + requestedWaits.Add(wait); + return requestedWaits.Count < 3; + }, + beforeRetry: retriedResults.Add); + + reads.ShouldEqual(3); + delays.ShouldEqual(2); + retriedResults.ShouldMatchInOrder(1, 2); + requestedWaits.ShouldMatchInOrder(TimeSpan.FromMilliseconds(20), TimeSpan.FromMilliseconds(40), TimeSpan.FromMilliseconds(80)); + tracer.RelatedWarningEvents.Count.ShouldEqual(3); + tracer.RelatedWarningEvents[2].ShouldContain("persisted after all attempts"); + } + + [TestCase] + public void InvokeResultDoesNotRetryWhenOneAttemptIsAllowed() + { + int reads = 0; + int delays = 0; + + GitConfigReadRetry.InvokeResult( + "gvfs.test", + () => ++reads, + (int _) => true, + new MockTracer(), + delay => ++delays, + maxAttempts: 1); + + reads.ShouldEqual(1); + delays.ShouldEqual(0); + } + + [TestCase] + public void InvokeResultUsesExponentialDelays() + { + List delays = new List(); + + GitConfigReadRetry.InvokeResult( + "gvfs.test", + () => 0, + (int _) => true, + new MockTracer(), + delays.Add); + + delays.ShouldMatchInOrder( + TimeSpan.FromMilliseconds(20), + TimeSpan.FromMilliseconds(40), + TimeSpan.FromMilliseconds(80), + TimeSpan.FromMilliseconds(160)); + } + + [TestCase] + public void InvokeCatchingRethrowsTheOriginalExceptionAfterTheLastAttempt() + { + int reads = 0; + InvalidOperationException lastException = null; + + Assert.Throws( + () => GitConfigReadRetry.InvokeCatching( + "gvfs.test", + () => + { + lastException = new InvalidOperationException("attempt " + ++reads); + throw lastException; + }, + (Exception _) => true, + new MockTracer(), + delay => { }, + maxAttempts: 3)); + + reads.ShouldEqual(3); + lastException.Message.ShouldEqual("attempt 3"); + } + + [TestCase] + public void TracedConfigNameDoesNotContainUrlCredentials() + { + MockTracer tracer = new MockTracer(); + + GitConfigReadRetry.InvokeResult( + "gvfs.user:token@example.com.repo.cache-server-url", + () => 0, + (int _) => true, + tracer, + delay => { }, + maxAttempts: 2); + + tracer.RelatedWarningEvents.Count.ShouldEqual(2); + foreach (string warning in tracer.RelatedWarningEvents) + { + warning.Contains("token").ShouldBeFalse(); + warning.ShouldContain("redacted"); + } + } + } +} diff --git a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs index 85668c6ad..4dd4b0fec 100644 --- a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs @@ -4,6 +4,7 @@ using GVFS.UnitTests.Mock.Git; using NUnit.Framework; using System; +using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Threading; @@ -14,6 +15,12 @@ namespace GVFS.UnitTests.Git [TestFixture] public class GitProcessTests { + // What git.exe prints on Windows while another process holds .git\config. + private const string LockedConfigErrors = + "warning: unable to access '.git/config': Permission denied\n" + + "warning: unable to access '.git/config': Permission denied\n" + + "fatal: unknown error occurred while reading the configuration files"; + [TestCase] public void BoundedGitOutputBuffer_KeepsShortOutput() { @@ -151,6 +158,163 @@ public void ResultHasNoErrors() result.StderrContainsErrors().ShouldBeFalse(); } + [TestCase] + public void GetFromLocalConfigRetriesTransientFailureAndReturnsValue() + { + MockTracer tracer = new MockTracer(); + int delayCount = 0; + MockGitProcess process = new MockGitProcess(tracer, delay => ++delayCount); + Queue results = new Queue(); + results.Enqueue(new GitProcess.Result( + string.Empty, + LockedConfigErrors, + GitProcess.Result.GenericFailureCode)); + results.Enqueue(new GitProcess.Result("configured-value\n", string.Empty, GitProcess.Result.SuccessCode)); + process.SetExpectedCommandResult("config --local gvfs.test", () => results.Dequeue()); + + GitProcess.ConfigResult result = process.GetFromLocalConfig("gvfs.test"); + + result.TryParseAsString(out string value, out string error).ShouldBeTrue(error); + value.ShouldEqual("configured-value"); + process.CommandsRun.Count.ShouldEqual(2); + delayCount.ShouldEqual(1); + tracer.RelatedWarningEvents.Count.ShouldEqual(1); + tracer.RelatedWarningEvents[0].ShouldContain("Transient failure while reading git config"); + } + + [TestCase] + public void GetFromLocalConfigDoesNotRetryMissingKey() + { + int delayCount = 0; + MockGitProcess process = new MockGitProcess(new MockTracer(), delay => ++delayCount); + process.SetExpectedCommandResult( + "config --local gvfs.test", + () => new GitProcess.Result(string.Empty, string.Empty, GitProcess.Result.GenericFailureCode)); + + GitProcess.ConfigResult result = process.GetFromLocalConfig("gvfs.test"); + + result.TryParseAsString(out string value, out string error).ShouldBeTrue(error); + value.ShouldBeNull(); + process.CommandsRun.Count.ShouldEqual(1); + delayCount.ShouldEqual(0); + } + + [TestCase] + public void GetFromLocalConfigStopsAfterRetryBudget() + { + MockTracer tracer = new MockTracer(); + int delayCount = 0; + MockGitProcess process = new MockGitProcess(tracer, delay => ++delayCount); + process.SetExpectedCommandResult( + "config --local gvfs.test", + () => new GitProcess.Result(string.Empty, LockedConfigErrors, GitProcess.Result.GenericFailureCode)); + + GitProcess.ConfigResult result = process.GetFromLocalConfig("gvfs.test"); + + result.TryParseAsString(out string _, out string error).ShouldBeFalse(); + error.ShouldContain("Permission denied"); + process.CommandsRun.Count.ShouldEqual(GitConfigReadRetry.MaxAttempts); + delayCount.ShouldEqual(GitConfigReadRetry.MaxAttempts - 1); + tracer.RelatedWarningEvents.Count.ShouldEqual(GitConfigReadRetry.MaxAttempts); + tracer.RelatedWarningEvents[GitConfigReadRetry.MaxAttempts - 1].ShouldContain("persisted after all attempts"); + } + + [TestCase] + public void GetFromLocalConfigDoesNotRetryWhenOnlyThePathContainsATransientPhrase() + { + int delayCount = 0; + MockGitProcess process = new MockGitProcess(new MockTracer(), delay => ++delayCount); + process.SetExpectedCommandResult( + "config --local gvfs.test", + () => new GitProcess.Result( + string.Empty, + "fatal: unable to access 'C:/Permission denied/.git/config': No such file or directory", + GitProcess.Result.GenericFailureCode)); + + process.GetFromLocalConfig("gvfs.test").TryParseAsString(out string _, out string _).ShouldBeFalse(); + + process.CommandsRun.Count.ShouldEqual(1); + delayCount.ShouldEqual(0); + } + + [TestCase] + public void GetFromConfigRetriesTransientFailure() + { + MockGitProcess process = this.CreateProcessWithTransientThenSuccess( + "config gvfs.test", + "configured-value\n", + out Func getDelayCount); + + GitProcess.ConfigResult result = process.GetFromConfig("gvfs.test", forceOutsideEnlistment: true); + + result.TryParseAsString(out string value, out string error).ShouldBeTrue(error); + value.ShouldEqual("configured-value"); + getDelayCount().ShouldEqual(1); + } + + [TestCase] + public void GetOriginUrlRetriesTransientFailure() + { + MockGitProcess process = this.CreateProcessWithTransientThenSuccess( + "config --local remote.origin.url", + "https://example.com/repo\n", + out Func getDelayCount); + + GitProcess.ConfigResult result = process.GetOriginUrl(); + + result.TryParseAsString(out string value, out string error).ShouldBeTrue(error); + value.ShouldEqual("https://example.com/repo"); + getDelayCount().ShouldEqual(1); + } + + [TestCase] + public void TryGetAllConfigRetriesTransientFailure() + { + MockGitProcess process = this.CreateProcessWithTransientThenSuccess( + "config --list --local", + "gvfs.test=configured-value\n", + out Func getDelayCount); + + process.TryGetAllConfig(localOnly: true, configSettings: out Dictionary settings).ShouldBeTrue(); + + settings.ContainsKey("gvfs.test").ShouldBeTrue(); + getDelayCount().ShouldEqual(1); + } + + [TestCase] + public void TryGetConfigUrlMatchRetriesTransientFailure() + { + MockGitProcess process = this.CreateProcessWithTransientThenSuccess( + "config --get-urlmatch http https://example.com/repo", + "http.proxy configured-value\n", + out Func getDelayCount); + + process.TryGetConfigUrlMatch("http", "https://example.com/repo", out Dictionary settings).ShouldBeTrue(); + + settings.ContainsKey("http.proxy").ShouldBeTrue(); + getDelayCount().ShouldEqual(1); + } + + [TestCase] + public void GetFromLocalConfigDoesNotRetryNonTransientFailure() + { + int delayCount = 0; + MockGitProcess process = new MockGitProcess(new MockTracer(), delay => ++delayCount); + process.SetExpectedCommandResult( + "config --local gvfs.test", + () => new GitProcess.Result( + string.Empty, + "fatal: bad config line 4 in file .git/config", + GitProcess.Result.GenericFailureCode)); + + GitProcess.ConfigResult result = process.GetFromLocalConfig("gvfs.test"); + + result.TryParseAsString(out string _, out string error).ShouldBeFalse(); + error.ShouldContain("bad config line"); + process.CommandsRun.Count.ShouldEqual(1); + delayCount.ShouldEqual(0); + } + [TestCase] public void ResultHasWarnings() { @@ -479,6 +643,18 @@ public void QuoteGitPath_RoundTripThroughProcess() } } + private MockGitProcess CreateProcessWithTransientThenSuccess(string command, string successOutput, out Func getDelayCount) + { + int delayCount = 0; + MockGitProcess process = new MockGitProcess(new MockTracer(), delay => ++delayCount); + Queue results = new Queue(); + results.Enqueue(new GitProcess.Result(string.Empty, LockedConfigErrors, GitProcess.Result.GenericFailureCode)); + results.Enqueue(new GitProcess.Result(successOutput, string.Empty, GitProcess.Result.SuccessCode)); + process.SetExpectedCommandResult(command, () => results.Dequeue()); + getDelayCount = () => delayCount; + return process; + } + private sealed class OutputCallbackGitProcess : GitProcess { public OutputCallbackGitProcess(string gitPath) diff --git a/GVFS/GVFS.UnitTests/Http/HttpResponseStreamTests.cs b/GVFS/GVFS.UnitTests/Http/HttpResponseStreamTests.cs index 2eb95bc11..1ee094a44 100644 --- a/GVFS/GVFS.UnitTests/Http/HttpResponseStreamTests.cs +++ b/GVFS/GVFS.UnitTests/Http/HttpResponseStreamTests.cs @@ -638,13 +638,18 @@ public async Task AuthenticationInitializationLoadsAllRetrySettings() GitAuthentication authentication = new GitAuthentication(git, endpoint); MockGVFSEnlistment enlistment = new MockGVFSEnlistment( "mock:\\repo", endpoint, "mock:\\git", git, authentication); + + // libgit2 reads the settings from the isolated global config. When the environment + // changes Git config, git.exe reads the same values from the mock. + using IsolatedLibGit2Config machineConfig = new IsolatedLibGit2Config(); + File.WriteAllText(machineConfig.GlobalConfigPath, "[gvfs]\n\tmax-retries = 0\n\ttimeout-seconds = 2\n\tread-timeout-seconds = 1\n"); try { authentication.TryInitialize(tracer, enlistment, out string error).ShouldEqual(true, error); - foreach (string key in new[] { "gvfs.max-retries", "gvfs.timeout-seconds", "gvfs.read-timeout-seconds" }) - { - git.CommandsRun.Count(command => command == "config " + key).ShouldEqual(1); - } + EventMetadata loaded = tracer.RelatedEventMetadata[tracer.RelatedEventNames.IndexOf("RetryConfig_LoadedRetryConfig")]; + loaded["MaxRetries"].ShouldEqual(0); + loaded["Timeout"].ShouldEqual(TimeSpan.FromSeconds(2)); + loaded["ReadTimeout"].ShouldEqual(TimeSpan.FromSeconds(1)); } finally { diff --git a/GVFS/GVFS.UnitTests/Mock/Common/MockGVFSEnlistment.cs b/GVFS/GVFS.UnitTests/Mock/Common/MockGVFSEnlistment.cs index 0df52b732..c98c37ceb 100644 --- a/GVFS/GVFS.UnitTests/Mock/Common/MockGVFSEnlistment.cs +++ b/GVFS/GVFS.UnitTests/Mock/Common/MockGVFSEnlistment.cs @@ -1,5 +1,6 @@ using GVFS.Common; using GVFS.Common.Git; +using GVFS.Common.Tracing; using GVFS.UnitTests.Mock.Git; using System.IO; @@ -40,5 +41,10 @@ public override GitProcess CreateGitProcess() { return this.gitProcess ?? new MockGitProcess(); } + + public override GitProcess CreateGitProcess(ITracer tracer) + { + return this.CreateGitProcess(); + } } } diff --git a/GVFS/GVFS.UnitTests/Mock/Git/IsolatedLibGit2Config.cs b/GVFS/GVFS.UnitTests/Mock/Git/IsolatedLibGit2Config.cs new file mode 100644 index 000000000..49d3c870a --- /dev/null +++ b/GVFS/GVFS.UnitTests/Mock/Git/IsolatedLibGit2Config.cs @@ -0,0 +1,77 @@ +using GVFS.Common; +using GVFS.Common.Git; +using System; +using System.IO; +using System.Runtime.InteropServices; + +namespace GVFS.UnitTests.Mock.Git +{ + /// + /// Points libgit2's global, XDG, system, and ProgramData config search paths at an empty + /// temporary directory, so a test does not read the machine's config. It also sets HOME and + /// XDG_CONFIG_HOME, so git.exe and the libgit2 reader agree on the global and XDG files. The + /// search paths last only while libgit2 stays initialized, so this object holds a libgit2 + /// reference until it is disposed. Unit tests run sequentially, so other tests do not observe + /// the change. + /// + public sealed class IsolatedLibGit2Config : IDisposable + { + private const int SetSearchPathOption = 5; + private const int XdgLevel = 3; + + // GIT_CONFIG_LEVEL_PROGRAMDATA, SYSTEM, XDG, and GLOBAL. + private static readonly int[] ConfigLevels = { 1, 2, XdgLevel, 4 }; + + private readonly string originalHome = Environment.GetEnvironmentVariable("HOME"); + private readonly string originalXdgConfigHome = Environment.GetEnvironmentVariable("XDG_CONFIG_HOME"); + + public IsolatedLibGit2Config() + { + this.Directory = Path.Combine(Path.GetTempPath(), "IsolatedLibGit2Config_" + Guid.NewGuid().ToString("N")); + System.IO.Directory.CreateDirectory(this.Directory); + string xdgConfigHome = Path.Combine(this.Directory, "xdg"); + Environment.SetEnvironmentVariable("HOME", this.Directory); + Environment.SetEnvironmentVariable("XDG_CONFIG_HOME", xdgConfigHome); + + LibGit2Repo.Native.Init(); + foreach (int level in ConfigLevels) + { + string path = level == XdgLevel ? Path.Combine(xdgConfigHome, "git") : this.Directory; + if (SetSearchPath(SetSearchPathOption, level, path) != 0) + { + throw new InvalidOperationException("git_libgit2_opts failed: " + LibGit2Repo.Native.GetLastError()); + } + } + } + + public string Directory { get; } + + public string GlobalConfigPath => Path.Combine(this.Directory, ".gitconfig"); + + /// + /// Makes libgit2 find the global and XDG config from the current environment variables. + /// The caller must hold a libgit2 reference. + /// + public static void ResetUserSearchPathsFromEnvironment() + { + SetSearchPath(SetSearchPathOption, XdgLevel, null); + SetSearchPath(SetSearchPathOption, 4, null); + } + + public void Dispose() + { + foreach (int level in ConfigLevels) + { + SetSearchPath(SetSearchPathOption, level, null); + } + + LibGit2Repo.Native.Shutdown(); + Environment.SetEnvironmentVariable("HOME", this.originalHome); + Environment.SetEnvironmentVariable("XDG_CONFIG_HOME", this.originalXdgConfigHome); + System.IO.Directory.Delete(this.Directory, recursive: true); + } + + [DllImport(GVFSConstants.LibGit2LibraryName, EntryPoint = "git_libgit2_opts")] + private static extern int SetSearchPath(int option, int level, [MarshalAs(UnmanagedType.LPUTF8Str)] string path); + } +} diff --git a/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs b/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs index 1f1854c8a..0d9b39fb3 100644 --- a/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs +++ b/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs @@ -20,6 +20,11 @@ public MockGitProcess() { } + internal MockGitProcess(ITracer tracer, Action configReadDelay) + : base(new MockGVFSEnlistment(), tracer, configReadDelay) + { + } + public MockGitProcess(Enlistment enlistment) : base(enlistment) {