Cache typed config with git config list --type=<X>#2268
Open
derrickstolee wants to merge 5 commits intogit-ecosystem:mainfrom
Open
Cache typed config with git config list --type=<X>#2268derrickstolee wants to merge 5 commits intogit-ecosystem:mainfrom
git config list --type=<X>#2268derrickstolee wants to merge 5 commits intogit-ecosystem:mainfrom
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Context: The original implementation inferred GitConfigurationLevel by parsing file paths from config origins (e.g., checking if path contains "/.gitconfig" for global). This approach is error-prone because: - Path conventions vary across platforms and Git installations - System config can be in multiple locations (prefix-dependent) - Custom config paths break the path-based detection - Worktree configs weren't handled Justification: Git's --show-scope flag directly reports whether each config entry is from system, global, local, worktree, or command line scope. Using this flag eliminates path-parsing heuristics and defers to Git's authoritative knowledge of config scope. Implementation: - Replaced DetermineLevel() path-parsing logic with ParseScope() that maps Git's scope strings directly to GitConfigurationLevel enum - Updated ConfigCache.Load() to parse scope field from git output - Updated EnsureCacheLoaded() to include --show-scope flag - ConfigCacheEntry constructor now accepts level directly Scope parsing maps: system/global/local/worktree → enum values, treating worktree as local (both are repository-specific). All 52 tests pass with this change. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
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>
Context: The config cache uses 'git config list --type=<X>' 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>
e0f4e13 to
0fea6fa
Compare
git config list --type=<X>git config list --type=<X>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In certain repos and on the Windows platform,
git-credential-managercan take 8-15 seconds before looking up a credential or having any user-facing interaction. This is due to dozens ofgit config --getprocesses that take 150-250 milliseconds each. The config keys that cause this pain arehttp.<url>.sslCAinfoandhttp.<url>.cookieFile. Whencredential.useHttpPathis enabled, each key is checked as<url>is incrementally truncated by directory segment.It would be best if we could call a single Git process to send multiple config queries instead of running multiple processes. gitgitgadget/git#2033 suggested this direction of a single process solution, but it's very complicated! During review of that effort, it was recommended to use
git config listinstead.But then there's a different problem! In all released versions of Git,
git config listsilently ignores the--typeargument. We need the--typeargument to guarantee that the resulting output string matches theboolorpathformats.The core Git change in gitgitgadget/git#2044 is now merged to
nextand thus is queued for Git 2.54.0. (We should wait until it merges tomasterbefore merging this change, just in case.) We can only check compatibility using a version check since the original command silently misbehaves.This pull request allows for caching the list of all config values that match the given types: bool, path, or none. These are loaded lazily so if a command doesn't need one of the types then the command doesn't run. We are also careful to clear this cache if GCM mutates the config.
Since we ask for Git config values using
--type=bool,--type=path, and--no-type, we may launch threegit config listcommands to satisfy these needs.There is a possibility that this feature is fast-tracked into microsoft/git, in which case the version check would need augmentation. I have that available in derrickstolee#1 as an example.
Disclaimer: I used Claude Code and GitHub Copilot CLI to assist with this change. I carefully reviewed the changes and made adjustments based on my own taste and testing.
I did an end-to-end performance test on a local monorepo and got these results for improvements to no-op
git fetchcalls: