From cfd3298cb8d92425877c93d2efc94114813ecb99 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 5 Feb 2026 09:37:59 -0500 Subject: [PATCH 1/6] refactor: Add config caching to reduce git process calls Context: Git configuration queries currently spawn a new git process for each TryGet(), GetAll(), or Enumerate() call. In scenarios where multiple config values are needed (e.g., credential helper initialization), this results in dozens of git process spawns, each parsing the same config files repeatedly. This impacts performance, especially on Windows where process creation is more expensive. Justification: Rather than caching individual queries, we cache the entire config output from a single 'git config list --show-origin -z' call. This approach provides several benefits: - Single process spawn loads all config values at once - Origin information allows accurate level filtering (system/global/local) - Cache invalidation on write operations keeps data consistent - Thread-safe implementation supports concurrent access We only cache Raw type queries since Bool and Path types require Git's canonicalization logic. Cache is loaded lazily on first access and invalidated on any write operation (Set, Add, Unset, etc.). Implementation: Added ConfigCacheEntry class to store origin, value, and level for each config entry. The ConfigCache class parses the NUL-delimited output from 'git config list --show-origin -z' (format: origin\0key\nvalue\0) and stores entries in a case-insensitive dictionary keyed by config name. Level detection examines the file path in the origin to determine System/Global/Local classification. Fallback to individual git config commands occurs if cache load fails or for typed (Bool/Path) queries. Co-Authored-By: Claude Sonnet 4.5 --- src/shared/Core/GitConfiguration.cs | 301 +++++++++++++++++++++++++++- 1 file changed, 300 insertions(+), 1 deletion(-) diff --git a/src/shared/Core/GitConfiguration.cs b/src/shared/Core/GitConfiguration.cs index 9603b2db5..52b42e541 100644 --- a/src/shared/Core/GitConfiguration.cs +++ b/src/shared/Core/GitConfiguration.cs @@ -108,24 +108,291 @@ public interface IGitConfiguration void UnsetAll(GitConfigurationLevel level, string name, string valueRegex); } + /// + /// Represents a single configuration entry with its origin and level. + /// + internal class ConfigCacheEntry + { + public string Origin { get; set; } + public string Value { get; set; } + public GitConfigurationLevel Level { get; set; } + + public ConfigCacheEntry(string origin, string value) + { + Origin = origin; + Value = value; + Level = DetermineLevel(origin); + } + + private static GitConfigurationLevel DetermineLevel(string origin) + { + if (string.IsNullOrEmpty(origin)) + return GitConfigurationLevel.Unknown; + + // Origins look like: "file:/path/to/config", "command line:", "standard input:" + if (!origin.StartsWith("file:")) + return GitConfigurationLevel.Unknown; + + string path = origin.Substring(5); // Remove "file:" prefix + + // System config is typically in /etc/gitconfig or $(prefix)/etc/gitconfig + if (path.Contains("/etc/gitconfig") || path.EndsWith("/gitconfig")) + return GitConfigurationLevel.System; + + // Global config is typically in ~/.gitconfig or ~/.config/git/config + if (path.Contains("/.gitconfig") || path.Contains("/.config/git/config")) + return GitConfigurationLevel.Global; + + // Local config is typically in .git/config within a repository + if (path.Contains("/.git/config")) + return GitConfigurationLevel.Local; + + return GitConfigurationLevel.Unknown; + } + } + + /// + /// Cache for Git configuration entries loaded from 'git config list --show-origin -z'. + /// + internal class ConfigCache + { + private Dictionary> _entries; + private readonly object _lock = new object(); + + public bool IsLoaded => _entries != null; + + public void Load(string data, ITrace trace) + { + lock (_lock) + { + var entries = new Dictionary>(GitConfigurationKeyComparer.Instance); + + var origin = new StringBuilder(); + var key = new StringBuilder(); + var value = new StringBuilder(); + + int i = 0; + while (i < data.Length) + { + origin.Clear(); + key.Clear(); + value.Clear(); + + // Read origin (NUL terminated) + while (i < data.Length && data[i] != '\0') + { + origin.Append(data[i++]); + } + + if (i >= data.Length) + { + trace.WriteLine("Invalid Git configuration output. Expected null terminator (\\0) after origin."); + break; + } + + // Skip the NUL terminator + i++; + + // Read key (newline terminated) + while (i < data.Length && data[i] != '\n') + { + key.Append(data[i++]); + } + + if (i >= data.Length) + { + trace.WriteLine("Invalid Git configuration output. Expected newline terminator (\\n) after key."); + break; + } + + // Skip the newline terminator + i++; + + // Read value (NUL terminated) + while (i < data.Length && data[i] != '\0') + { + value.Append(data[i++]); + } + + if (i >= data.Length) + { + trace.WriteLine("Invalid Git configuration output. Expected null terminator (\\0) after value."); + break; + } + + // Skip the NUL terminator + i++; + + string keyStr = key.ToString(); + var entry = new ConfigCacheEntry(origin.ToString(), value.ToString()); + + if (!entries.ContainsKey(keyStr)) + { + entries[keyStr] = new List(); + } + entries[keyStr].Add(entry); + } + + _entries = entries; + } + } + + public bool TryGet(string name, GitConfigurationLevel level, out string value) + { + lock (_lock) + { + if (_entries == null) + { + value = null; + return false; + } + + if (!_entries.TryGetValue(name, out var entryList)) + { + value = null; + return false; + } + + // Find the first entry matching the level filter + foreach (var entry in entryList) + { + if (level == GitConfigurationLevel.All || entry.Level == level) + { + value = entry.Value; + return true; + } + } + + value = null; + return false; + } + } + + public IEnumerable GetAll(string name, GitConfigurationLevel level) + { + lock (_lock) + { + if (_entries == null || !_entries.TryGetValue(name, out var entryList)) + { + return Array.Empty(); + } + + var results = new List(); + foreach (var entry in entryList) + { + if (level == GitConfigurationLevel.All || entry.Level == level) + { + results.Add(entry.Value); + } + } + + return results; + } + } + + public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCallback cb) + { + lock (_lock) + { + if (_entries == null) + return; + + foreach (var kvp in _entries) + { + foreach (var entry in kvp.Value) + { + if (level == GitConfigurationLevel.All || entry.Level == level) + { + var configEntry = new GitConfigurationEntry(kvp.Key, entry.Value); + if (!cb(configEntry)) + { + return; + } + } + } + } + } + } + + public void Clear() + { + lock (_lock) + { + _entries = null; + } + } + } + public class GitProcessConfiguration : IGitConfiguration { private static readonly GitVersion TypeConfigMinVersion = new GitVersion(2, 18, 0); private readonly ITrace _trace; private readonly GitProcess _git; + private readonly ConfigCache _cache; + private readonly bool _useCache; + + internal GitProcessConfiguration(ITrace trace, GitProcess git) : this(trace, git, useCache: true) + { + } - internal GitProcessConfiguration(ITrace trace, GitProcess git) + internal GitProcessConfiguration(ITrace trace, GitProcess git, bool useCache) { EnsureArgument.NotNull(trace, nameof(trace)); EnsureArgument.NotNull(git, nameof(git)); _trace = trace; _git = git; + _useCache = useCache; + _cache = useCache ? new ConfigCache() : null; + } + + private void EnsureCacheLoaded() + { + if (!_useCache || _cache.IsLoaded) + return; + + using (ChildProcess git = _git.CreateProcess("config list --show-origin -z")) + { + git.Start(Trace2ProcessClass.Git); + // To avoid deadlocks, always read the output stream first and then wait + string data = git.StandardOutput.ReadToEnd(); + git.WaitForExit(); + + switch (git.ExitCode) + { + case 0: // OK + _cache.Load(data, _trace); + break; + default: + _trace.WriteLine($"Failed to load config cache (exit={git.ExitCode})"); + // Don't throw - fall back to individual commands + break; + } + } + } + + private void InvalidateCache() + { + if (_useCache) + { + _cache.Clear(); + } } public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCallback cb) { + if (_useCache) + { + EnsureCacheLoaded(); + if (_cache.IsLoaded) + { + _cache.Enumerate(level, cb); + return; + } + } + + // Fall back to original implementation string levelArg = GetLevelFilterArg(level); using (ChildProcess git = _git.CreateProcess($"config --null {levelArg} --list")) { @@ -194,6 +461,17 @@ public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCa public bool TryGet(GitConfigurationLevel level, GitConfigurationType type, string name, out string value) { + // Use cache for raw types only - typed queries need Git's canonicalization + if (_useCache && type == GitConfigurationType.Raw) + { + EnsureCacheLoaded(); + if (_cache.IsLoaded && _cache.TryGet(name, level, out value)) + { + return true; + } + } + + // Fall back to individual git config command for typed queries or cache miss string levelArg = GetLevelFilterArg(level); string typeArg = GetCanonicalizeTypeArg(type); using (ChildProcess git = _git.CreateProcess($"config --null {levelArg} {typeArg} {QuoteCmdArg(name)}")) @@ -242,6 +520,7 @@ public void Set(GitConfigurationLevel level, string name, string value) switch (git.ExitCode) { case 0: // OK + InvalidateCache(); break; default: _trace.WriteLine($"Failed to set config entry '{name}' to value '{value}' (exit={git.ExitCode}, level={level})"); @@ -263,6 +542,7 @@ public void Add(GitConfigurationLevel level, string name, string value) switch (git.ExitCode) { case 0: // OK + InvalidateCache(); break; default: _trace.WriteLine($"Failed to add config entry '{name}' with value '{value}' (exit={git.ExitCode}, level={level})"); @@ -285,6 +565,7 @@ public void Unset(GitConfigurationLevel level, string name) { case 0: // OK case 5: // Trying to unset a value that does not exist + InvalidateCache(); break; default: _trace.WriteLine($"Failed to unset config entry '{name}' (exit={git.ExitCode}, level={level})"); @@ -295,6 +576,22 @@ public void Unset(GitConfigurationLevel level, string name) public IEnumerable GetAll(GitConfigurationLevel level, GitConfigurationType type, string name) { + // Use cache for raw types only - typed queries need Git's canonicalization + if (_useCache && type == GitConfigurationType.Raw) + { + EnsureCacheLoaded(); + if (_cache.IsLoaded) + { + var cachedValues = _cache.GetAll(name, level); + foreach (var val in cachedValues) + { + yield return val; + } + yield break; + } + } + + // Fall back to individual git config command string levelArg = GetLevelFilterArg(level); string typeArg = GetCanonicalizeTypeArg(type); @@ -392,6 +689,7 @@ public void ReplaceAll(GitConfigurationLevel level, string name, string valueReg switch (git.ExitCode) { case 0: // OK + InvalidateCache(); break; default: _trace.WriteLine($"Failed to replace all multivar '{name}' and value regex '{valueRegex}' with new value '{value}' (exit={git.ExitCode}, level={level})"); @@ -420,6 +718,7 @@ public void UnsetAll(GitConfigurationLevel level, string name, string valueRegex { case 0: // OK case 5: // Trying to unset a value that does not exist + InvalidateCache(); break; default: _trace.WriteLine($"Failed to unset all multivar '{name}' with value regex '{valueRegex}' (exit={git.ExitCode}, level={level})"); From 625d2541d644075ed1c803c7eba09e5ef72741bc Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 5 Feb 2026 09:39:52 -0500 Subject: [PATCH 2/6] test: Add cache tests and fix precedence bug Context: The caching implementation needed comprehensive tests to verify correct behavior across different scenarios: cache hits, cache invalidation, level filtering, and multivar handling. Tests revealed a critical bug where the cache returned the wrong value when multiple config levels (system/global/local) defined the same key. Justification: Tests follow existing patterns in GitConfigurationTests.cs, creating temporary repositories and verifying cache behavior through the public IGitConfiguration interface rather than testing internal cache classes directly. This ensures we test the actual behavior users will experience. The precedence bug occurred because ConfigCache.TryGet() returned the first matching entry when it should return the last one. Git outputs config values in precedence order (system, global, local), with later values overriding earlier ones. Returning the last match correctly implements Git's precedence rules. Implementation: Added 8 new test methods covering: - Cache loading and retrieval (TryGet, GetAll, Enumerate) - Cache invalidation on write operations (Set, Add, Unset) - Level filtering to isolate local/global/system values - Typed queries that bypass cache for Git's canonicalization Fixed ConfigCache.TryGet() to iterate through all matching entries and return the last one instead of the first, ensuring local config wins over global, which wins over system. All 805 tests pass with the fix applied. Co-Authored-By: Claude Sonnet 4.5 --- .../Core.Tests/GitConfigurationTests.cs | 219 ++++++++++++++++++ src/shared/Core/GitConfiguration.cs | 13 +- 2 files changed, 229 insertions(+), 3 deletions(-) diff --git a/src/shared/Core.Tests/GitConfigurationTests.cs b/src/shared/Core.Tests/GitConfigurationTests.cs index 498651f73..e4ca872de 100644 --- a/src/shared/Core.Tests/GitConfigurationTests.cs +++ b/src/shared/Core.Tests/GitConfigurationTests.cs @@ -436,5 +436,224 @@ public void GitConfiguration_UnsetAll_All_ThrowsException() Assert.Throws(() => config.UnsetAll(GitConfigurationLevel.All, "core.foobar", Constants.RegexPatterns.Any)); } + + [Fact] + public void GitConfiguration_CacheTryGet_ReturnsValueFromCache() + { + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local user.email john@example.com").AssertSuccess(); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var trace2 = new NullTrace2(); + var processManager = new TestProcessManager(); + + var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); + IGitConfiguration config = git.GetConfiguration(); + + // First access loads cache + bool result1 = config.TryGet("user.name", false, out string value1); + Assert.True(result1); + Assert.Equal("john.doe", value1); + + // Second access should use cache + bool result2 = config.TryGet("user.email", false, out string value2); + Assert.True(result2); + Assert.Equal("john@example.com", value2); + } + + [Fact] + public void GitConfiguration_CacheGetAll_ReturnsAllValuesFromCache() + { + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, "config --local --add test.multi value1").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local --add test.multi value2").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local --add test.multi value3").AssertSuccess(); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var trace2 = new NullTrace2(); + var processManager = new TestProcessManager(); + + var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); + IGitConfiguration config = git.GetConfiguration(); + + var values = new List(config.GetAll("test.multi")); + + Assert.Equal(3, values.Count); + Assert.Equal("value1", values[0]); + Assert.Equal("value2", values[1]); + Assert.Equal("value3", values[2]); + } + + [Fact] + public void GitConfiguration_CacheEnumerate_EnumeratesFromCache() + { + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, "config --local cache.name test-value").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local cache.enabled true").AssertSuccess(); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var trace2 = new NullTrace2(); + var processManager = new TestProcessManager(); + + var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); + IGitConfiguration config = git.GetConfiguration(); + + var cacheEntries = new List<(string key, string value)>(); + config.Enumerate(entry => + { + if (entry.Key.StartsWith("cache.")) + { + cacheEntries.Add((entry.Key, entry.Value)); + } + return true; + }); + + Assert.Equal(2, cacheEntries.Count); + Assert.Contains(("cache.name", "test-value"), cacheEntries); + Assert.Contains(("cache.enabled", "true"), cacheEntries); + } + + [Fact] + public void GitConfiguration_CacheInvalidation_SetInvalidatesCache() + { + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, "config --local test.value initial").AssertSuccess(); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var trace2 = new NullTrace2(); + var processManager = new TestProcessManager(); + + var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); + IGitConfiguration config = git.GetConfiguration(); + + // Load cache with initial value + bool result1 = config.TryGet("test.value", false, out string value1); + Assert.True(result1); + Assert.Equal("initial", value1); + + // Set new value (should invalidate cache) + config.Set(GitConfigurationLevel.Local, "test.value", "updated"); + + // Next read should get updated value + bool result2 = config.TryGet("test.value", false, out string value2); + Assert.True(result2); + Assert.Equal("updated", value2); + } + + [Fact] + public void GitConfiguration_CacheInvalidation_AddInvalidatesCache() + { + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, "config --local test.multi first").AssertSuccess(); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var trace2 = new NullTrace2(); + var processManager = new TestProcessManager(); + + var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); + IGitConfiguration config = git.GetConfiguration(); + + // Load cache + var values1 = new List(config.GetAll("test.multi")); + Assert.Single(values1); + Assert.Equal("first", values1[0]); + + // Add new value (should invalidate cache) + config.Add(GitConfigurationLevel.Local, "test.multi", "second"); + + // Next read should include new value + var values2 = new List(config.GetAll("test.multi")); + Assert.Equal(2, values2.Count); + Assert.Equal("first", values2[0]); + Assert.Equal("second", values2[1]); + } + + [Fact] + public void GitConfiguration_CacheInvalidation_UnsetInvalidatesCache() + { + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, "config --local test.value exists").AssertSuccess(); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var trace2 = new NullTrace2(); + var processManager = new TestProcessManager(); + + var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); + IGitConfiguration config = git.GetConfiguration(); + + // Load cache + bool result1 = config.TryGet("test.value", false, out string value1); + Assert.True(result1); + Assert.Equal("exists", value1); + + // Unset value (should invalidate cache) + config.Unset(GitConfigurationLevel.Local, "test.value"); + + // Next read should not find value + bool result2 = config.TryGet("test.value", false, out string value2); + Assert.False(result2); + Assert.Null(value2); + } + + [Fact] + public void GitConfiguration_CacheLevelFilter_ReturnsOnlyLocalValues() + { + string repoPath = CreateRepository(out string workDirPath); + + try + { + ExecGit(repoPath, workDirPath, "config --global test.level global-value").AssertSuccess(); + ExecGit(repoPath, workDirPath, "config --local test.level local-value").AssertSuccess(); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var trace2 = new NullTrace2(); + var processManager = new TestProcessManager(); + + var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); + IGitConfiguration config = git.GetConfiguration(); + + // Get local value only + bool result = config.TryGet(GitConfigurationLevel.Local, GitConfigurationType.Raw, + "test.level", out string value); + Assert.True(result); + Assert.Equal("local-value", value); + } + finally + { + // Cleanup global config + ExecGit(repoPath, workDirPath, "config --global --unset test.level"); + } + } + + [Fact] + public void GitConfiguration_TypedQuery_DoesNotUseCache() + { + string repoPath = CreateRepository(out string workDirPath); + ExecGit(repoPath, workDirPath, "config --local test.path ~/example").AssertSuccess(); + + string gitPath = GetGitPath(); + var trace = new NullTrace(); + var trace2 = new NullTrace2(); + var processManager = new TestProcessManager(); + + var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); + IGitConfiguration config = git.GetConfiguration(); + + // Path type should not use cache (needs Git's canonicalization) + bool result = config.TryGet(GitConfigurationLevel.Local, GitConfigurationType.Path, + "test.path", out string value); + Assert.True(result); + Assert.NotNull(value); + // Value should be canonicalized path, not raw "~/example" + Assert.NotEqual("~/example", value); + } } } diff --git a/src/shared/Core/GitConfiguration.cs b/src/shared/Core/GitConfiguration.cs index 52b42e541..89cf020ed 100644 --- a/src/shared/Core/GitConfiguration.cs +++ b/src/shared/Core/GitConfiguration.cs @@ -253,16 +253,23 @@ public bool TryGet(string name, GitConfigurationLevel level, out string value) return false; } - // Find the first entry matching the level filter + // Find the last entry matching the level filter (respects Git's precedence) + // Git config precedence: system < global < local, so last match wins + ConfigCacheEntry lastMatch = null; foreach (var entry in entryList) { if (level == GitConfigurationLevel.All || entry.Level == level) { - value = entry.Value; - return true; + lastMatch = entry; } } + if (lastMatch != null) + { + value = lastMatch.Value; + return true; + } + value = null; return false; } From 4f37e544c1969c5d7ffd1aa98726c3879abfeddc Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sun, 1 Mar 2026 12:56:48 -0500 Subject: [PATCH 3/6] feat: Cache config entries by type for typed queries Context: The config cache only stored raw (untyped) values, so Bool and Path queries always fell back to spawning individual git processes. Since Git's --type flag canonicalizes values (e.g., expanding ~/... for paths, normalizing yes/on/1 to true for bools), serving these from the raw cache would return incorrect values. Justification: Instead of bypassing the cache for typed queries, we maintain a separate cache per GitConfigurationType. Each cache is loaded with the appropriate --type flag passed to 'git config list', so Git performs canonicalization during the bulk load. This preserves the correctness guarantee while extending the performance benefit to all query types. The cache result is now authoritative when loaded: if a key is not found in the cache, we return 'not found' directly rather than falling back to an individual git process call. This avoids a redundant process spawn when the key genuinely doesn't exist. Implementation: Changed _cache from a single ConfigCache to a Dictionary keyed by GitConfigurationType. EnsureCacheLoaded() now accepts a type parameter and passes --no-type, --type=bool, or --type=path to the git config list command. InvalidateCache() clears all type-specific caches on any write operation. Renamed TypedQuery_DoesNotUseCache test to TypedQuery_CanonicalizesValues since typed queries now use their own type-specific cache rather than bypassing the cache entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Core.Tests/GitConfigurationTests.cs | 5 +- src/shared/Core/GitConfiguration.cs | 81 ++++++++++++++----- 2 files changed, 63 insertions(+), 23 deletions(-) diff --git a/src/shared/Core.Tests/GitConfigurationTests.cs b/src/shared/Core.Tests/GitConfigurationTests.cs index e4ca872de..5005cf431 100644 --- a/src/shared/Core.Tests/GitConfigurationTests.cs +++ b/src/shared/Core.Tests/GitConfigurationTests.cs @@ -634,7 +634,7 @@ public void GitConfiguration_CacheLevelFilter_ReturnsOnlyLocalValues() } [Fact] - public void GitConfiguration_TypedQuery_DoesNotUseCache() + public void GitConfiguration_TypedQuery_CanonicalizesValues() { string repoPath = CreateRepository(out string workDirPath); ExecGit(repoPath, workDirPath, "config --local test.path ~/example").AssertSuccess(); @@ -647,7 +647,8 @@ public void GitConfiguration_TypedQuery_DoesNotUseCache() var git = new GitProcess(trace, trace2, processManager, gitPath, repoPath); IGitConfiguration config = git.GetConfiguration(); - // Path type should not use cache (needs Git's canonicalization) + // Path type queries use a separate cache loaded with --type=path, + // so Git canonicalizes the values during cache load. bool result = config.TryGet(GitConfigurationLevel.Local, GitConfigurationType.Path, "test.path", out string value); Assert.True(result); diff --git a/src/shared/Core/GitConfiguration.cs b/src/shared/Core/GitConfiguration.cs index 89cf020ed..3e7878f89 100644 --- a/src/shared/Core/GitConfiguration.cs +++ b/src/shared/Core/GitConfiguration.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Linq; using System.Text; namespace GitCredentialManager @@ -336,7 +337,7 @@ public class GitProcessConfiguration : IGitConfiguration private readonly ITrace _trace; private readonly GitProcess _git; - private readonly ConfigCache _cache; + private readonly Dictionary _cache; private readonly bool _useCache; internal GitProcessConfiguration(ITrace trace, GitProcess git) : this(trace, git, useCache: true) @@ -351,15 +352,44 @@ internal GitProcessConfiguration(ITrace trace, GitProcess git, bool useCache) _trace = trace; _git = git; _useCache = useCache; - _cache = useCache ? new ConfigCache() : null; + _cache = useCache ? new Dictionary() : null; } - private void EnsureCacheLoaded() + private void EnsureCacheLoaded(GitConfigurationType type) { - if (!_useCache || _cache.IsLoaded) + ConfigCache cache; + if (!_useCache || (_cache.TryGetValue(type, out cache) && cache.IsLoaded)) + { + return; + } + + if (cache == null) + { + cache = new ConfigCache(); + _cache[type] = cache; + } + + string typeArg; + + switch (type) + { + case GitConfigurationType.Raw: + typeArg = "--no-type"; + break; + + case GitConfigurationType.Path: + typeArg = "--type=path"; + break; + + case GitConfigurationType.Bool: + typeArg = "--type=bool"; + break; + + default: return; + } - using (ChildProcess git = _git.CreateProcess("config list --show-origin -z")) + using (ChildProcess git = _git.CreateProcess($"config list --show-origin -z {typeArg}")) { git.Start(Trace2ProcessClass.Git); // To avoid deadlocks, always read the output stream first and then wait @@ -369,7 +399,7 @@ private void EnsureCacheLoaded() switch (git.ExitCode) { case 0: // OK - _cache.Load(data, _trace); + cache.Load(data, _trace); break; default: _trace.WriteLine($"Failed to load config cache (exit={git.ExitCode})"); @@ -383,7 +413,10 @@ private void InvalidateCache() { if (_useCache) { - _cache.Clear(); + foreach (ConfigCache cache in _cache.Values) + { + cache.Clear(); + } } } @@ -391,10 +424,13 @@ public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCa { if (_useCache) { - EnsureCacheLoaded(); - if (_cache.IsLoaded) + EnsureCacheLoaded(GitConfigurationType.Raw); + + ConfigCache cache = _cache[GitConfigurationType.Raw]; + + if (cache.IsLoaded) { - _cache.Enumerate(level, cb); + cache.Enumerate(level, cb); return; } } @@ -468,17 +504,19 @@ public void Enumerate(GitConfigurationLevel level, GitConfigurationEnumerationCa public bool TryGet(GitConfigurationLevel level, GitConfigurationType type, string name, out string value) { - // Use cache for raw types only - typed queries need Git's canonicalization - if (_useCache && type == GitConfigurationType.Raw) + if (_useCache) { - EnsureCacheLoaded(); - if (_cache.IsLoaded && _cache.TryGet(name, level, out value)) + EnsureCacheLoaded(type); + + ConfigCache cache = _cache[type]; + if (cache.IsLoaded) { - return true; + // Cache is loaded, use it for the result (whether found or not) + return cache.TryGet(name, level, out value); } } - // Fall back to individual git config command for typed queries or cache miss + // Fall back to individual git config command if cache not available string levelArg = GetLevelFilterArg(level); string typeArg = GetCanonicalizeTypeArg(type); using (ChildProcess git = _git.CreateProcess($"config --null {levelArg} {typeArg} {QuoteCmdArg(name)}")) @@ -583,13 +621,14 @@ public void Unset(GitConfigurationLevel level, string name) public IEnumerable GetAll(GitConfigurationLevel level, GitConfigurationType type, string name) { - // Use cache for raw types only - typed queries need Git's canonicalization - if (_useCache && type == GitConfigurationType.Raw) + if (_useCache) { - EnsureCacheLoaded(); - if (_cache.IsLoaded) + EnsureCacheLoaded(type); + + ConfigCache cache = _cache[type]; + if (cache.IsLoaded) { - var cachedValues = _cache.GetAll(name, level); + var cachedValues = cache.GetAll(name, level); foreach (var val in cachedValues) { yield return val; From 3125246e0210cc3c88498f03ea00455bdc5cedf4 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Sun, 1 Mar 2026 12:58:45 -0500 Subject: [PATCH 4/6] feat: Gate config cache on Git 2.54.0+ version check Context: The config cache uses 'git config list --type=' to load type-specific caches (raw, bool, path). The --type flag for the 'list' subcommand requires a fix queued for the Git 2.54.0 release. On Git 2.53 and earlier, the command succeeds but silently ignores the --type parameter, returning raw values instead of canonicalized ones. This means bool caches would contain 'yes'/'on' instead of 'true', and path caches would contain unexpanded '~/...' instead of absolute paths. Justification: Because the command exits 0 on older Git, the cache appears to load successfully and the fallback paths never trigger. This makes the bug silent and data-dependent: lookups work for values that happen to already be in canonical form but return wrong results for others. A version gate is the only reliable way to avoid this. The check is in the constructor body rather than the constructor chain so we can log a trace message when caching is disabled. The explicit useCache parameter is preserved for tests that need to control caching behavior independently of version. Implementation: Added ConfigListTypeMinVersion constant (2.54.0) and a version comparison in the GitProcessConfiguration constructor. When useCache is requested but git.Version is below the minimum, the constructor overrides useCache to false and emits a trace line. All existing fallback paths continue to work unchanged for users on older Git, who will benefit from the cache automatically once they upgrade. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/shared/Core/GitConfiguration.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/shared/Core/GitConfiguration.cs b/src/shared/Core/GitConfiguration.cs index 3e7878f89..725e30f67 100644 --- a/src/shared/Core/GitConfiguration.cs +++ b/src/shared/Core/GitConfiguration.cs @@ -334,6 +334,7 @@ public void Clear() public class GitProcessConfiguration : IGitConfiguration { private static readonly GitVersion TypeConfigMinVersion = new GitVersion(2, 18, 0); + private static readonly GitVersion ConfigListTypeMinVersion = new GitVersion(2, 54, 0); private readonly ITrace _trace; private readonly GitProcess _git; @@ -351,6 +352,14 @@ internal GitProcessConfiguration(ITrace trace, GitProcess git, bool useCache) _trace = trace; _git = git; + + // 'git config list --type=' requires Git 2.54.0+ + if (useCache && git.Version < ConfigListTypeMinVersion) + { + trace.WriteLine($"Git version {git.Version} is below {ConfigListTypeMinVersion}; config cache disabled"); + useCache = false; + } + _useCache = useCache; _cache = useCache ? new Dictionary() : null; } From efec51379e09d15dafc846e4c8bc78227ae9ac8d Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 17 Mar 2026 12:56:43 -0400 Subject: [PATCH 5/6] config: use --show-scope instead of --show-origin in cache Context: The config cache path already gates on Git 2.54.0+ (or microsoft/git 2.53.0.vfs.0.1+) to use 'git config list --type='. These versions also support --show-scope, which directly reports the scope (system, global, local) as a simple string rather than a file path. Justification: The --show-origin approach required heuristic path matching to guess the scope from origin paths like 'file:/etc/gitconfig' or 'file:~/.gitconfig'. This was fragile: it could fail on non-standard install prefixes or platform-specific paths (e.g., Windows). Since --show-scope is available at the same Git versions we already require, we can use it for reliable scope detection with no guesswork. Implementation: - Replace --show-origin with --show-scope in the git config list call - Replace DetermineLevel() path heuristic with ParseScope() that does a direct string match on 'system', 'global', 'local' - Remove the now-unused Origin property from ConfigCacheEntry - Rename origin variables to scope throughout ConfigCache.Load() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/shared/Core/GitConfiguration.cs | 57 +++++++++++------------------ 1 file changed, 22 insertions(+), 35 deletions(-) diff --git a/src/shared/Core/GitConfiguration.cs b/src/shared/Core/GitConfiguration.cs index 725e30f67..f8dbeb63e 100644 --- a/src/shared/Core/GitConfiguration.cs +++ b/src/shared/Core/GitConfiguration.cs @@ -114,46 +114,33 @@ public interface IGitConfiguration /// internal class ConfigCacheEntry { - public string Origin { get; set; } public string Value { get; set; } public GitConfigurationLevel Level { get; set; } - public ConfigCacheEntry(string origin, string value) + public ConfigCacheEntry(string scope, string value) { - Origin = origin; Value = value; - Level = DetermineLevel(origin); + Level = ParseScope(scope); } - private static GitConfigurationLevel DetermineLevel(string origin) + private static GitConfigurationLevel ParseScope(string scope) { - if (string.IsNullOrEmpty(origin)) - return GitConfigurationLevel.Unknown; - - // Origins look like: "file:/path/to/config", "command line:", "standard input:" - if (!origin.StartsWith("file:")) - return GitConfigurationLevel.Unknown; - - string path = origin.Substring(5); // Remove "file:" prefix - - // System config is typically in /etc/gitconfig or $(prefix)/etc/gitconfig - if (path.Contains("/etc/gitconfig") || path.EndsWith("/gitconfig")) - return GitConfigurationLevel.System; - - // Global config is typically in ~/.gitconfig or ~/.config/git/config - if (path.Contains("/.gitconfig") || path.Contains("/.config/git/config")) - return GitConfigurationLevel.Global; - - // Local config is typically in .git/config within a repository - if (path.Contains("/.git/config")) - return GitConfigurationLevel.Local; - - return GitConfigurationLevel.Unknown; + switch (scope) + { + case "system": + return GitConfigurationLevel.System; + case "global": + return GitConfigurationLevel.Global; + case "local": + return GitConfigurationLevel.Local; + default: + return GitConfigurationLevel.Unknown; + } } } /// - /// Cache for Git configuration entries loaded from 'git config list --show-origin -z'. + /// Cache for Git configuration entries loaded from 'git config list --show-scope -z'. /// internal class ConfigCache { @@ -168,26 +155,26 @@ public void Load(string data, ITrace trace) { var entries = new Dictionary>(GitConfigurationKeyComparer.Instance); - var origin = new StringBuilder(); + var scope = new StringBuilder(); var key = new StringBuilder(); var value = new StringBuilder(); int i = 0; while (i < data.Length) { - origin.Clear(); + scope.Clear(); key.Clear(); value.Clear(); - // Read origin (NUL terminated) + // Read scope (NUL terminated) while (i < data.Length && data[i] != '\0') { - origin.Append(data[i++]); + scope.Append(data[i++]); } if (i >= data.Length) { - trace.WriteLine("Invalid Git configuration output. Expected null terminator (\\0) after origin."); + trace.WriteLine("Invalid Git configuration output. Expected null terminator (\\0) after scope."); break; } @@ -225,7 +212,7 @@ public void Load(string data, ITrace trace) i++; string keyStr = key.ToString(); - var entry = new ConfigCacheEntry(origin.ToString(), value.ToString()); + var entry = new ConfigCacheEntry(scope.ToString(), value.ToString()); if (!entries.ContainsKey(keyStr)) { @@ -398,7 +385,7 @@ private void EnsureCacheLoaded(GitConfigurationType type) return; } - using (ChildProcess git = _git.CreateProcess($"config list --show-origin -z {typeArg}")) + using (ChildProcess git = _git.CreateProcess($"config list --show-scope -z {typeArg}")) { git.Start(Trace2ProcessClass.Git); // To avoid deadlocks, always read the output stream first and then wait From 2d2658e31dc91277521bb23fb37faeae787ecc73 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 17 Mar 2026 12:58:47 -0400 Subject: [PATCH 6/6] GitConfiguration: 'worktree' and 'command' count as local --- src/shared/Core/GitConfiguration.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/shared/Core/GitConfiguration.cs b/src/shared/Core/GitConfiguration.cs index f8dbeb63e..e5eb08d6d 100644 --- a/src/shared/Core/GitConfiguration.cs +++ b/src/shared/Core/GitConfiguration.cs @@ -132,6 +132,8 @@ private static GitConfigurationLevel ParseScope(string scope) case "global": return GitConfigurationLevel.Global; case "local": + case "worktree": + case "command": return GitConfigurationLevel.Local; default: return GitConfigurationLevel.Unknown;