Share transient libgit2 config lookup helper#2066
Draft
tyrielv wants to merge 2 commits into
Draft
Conversation
Add a shared helper for transient config-only LibGit2Repo usage and switch short-lived call sites away from LibGit2RepoInvoker/object-store preload or duplicated try/catch blocks. Assisted-by: GPT-5 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Replace the standalone LibGit2RepoExtensions static class with instance and static GetConfigBoolOrDefault methods directly on LibGit2Repo. This matches the existing repo.GetConfigBoolOrDefault(name, default) convention already documented in AGENTS.md and used by LibGit2RepoInvoker, and avoids an unnecessary extension-method indirection for a class we own in the same assembly. Also fixes a regression reintroduced by the earlier refactor: CloneVerb called LibGit2Repo.GetConfigBoolOrDefault using enlistment.WorkingDirectoryBackingRoot unconditionally, even when TryCreateEnlistment failed and enlistment was null. The call is now correctly gated on cloneResult.Success. Added a protected LibGit2Repo(ITracer tracer) constructor so test doubles can inject a mock tracer and assert on RelatedWarning calls from the new instance method, and rewrote the test file accordingly. Assisted-by: Claude Sonnet 5 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
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
1 participant
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.
Summary
GVFS.Common.Git.LibGit2RepoExtensions.GetConfigBoolOrDefault(...)for transient config-only libgit2 readsLibGit2RepoInvokerobject-store preload and duplicated open/try/catch/dispose codeMotivation
LibGit2RepoInvoker.InitializeSharedRepo()intentionally forces an object-store probe so long-lived/shared callers can amortize object-store load costs. That is wasted work for one-off config reads that immediately dispose the repo. This change centralizes the short-lived config-read pattern behind a helper that opens a temporaryLibGit2Repo, reads the config value, and falls back cleanly onInvalidDataException/LibGit2Exception.Helper
Added
GVFS/GVFS.Common/Git/LibGit2RepoExtensions.cs:public static bool GetConfigBoolOrDefault(this LibGit2Repo repo, ITracer tracer, string key, bool defaultValue)public static bool GetConfigBoolOrDefault(ITracer tracer, string repoPath, string key, bool defaultValue)The path overload handles the transient open/dispose flow. The instance overload applies the same config-read fallback logic when a repo is already open.
Refactored call sites
GVFS/GVFS/CommandLine/CloneVerb.csLibGit2RepoInvokerjust to readgvfs.trust-pack-indexesenlistment.WorkingDirectoryBackingRootGVFS/GVFS.Hooks/Program.csLibGit2RepoInvokerjust to readgvfs.show-hydration-statusGVFS/GVFS.Mount/InProcessMount.csLibGit2Repo+ try/catch forgvfs.background-cache-authGVFS/GVFS/CommandLine/PrefetchVerb.csLibGit2Repo+ try/catch forgvfs.prefetch-offloadGVFS/GVFS.Hooks/GVFS.Hooks.csprojGVFS.Commonsource files directly rather than via project reference)Survey notes / intentionally left alone
I surveyed
masterfor short-lived config-onlyLibGit2Repo/LibGit2RepoInvokerusage. The narrow transient call sites above were refactored. I left these alone intentionally because they are shared/long-lived repo access, not the transient anti-pattern:GVFS/GVFS.Common/Maintenance/PrefetchStep.csGVFS/GVFS.Common/GitStatusCache.csGVFS/GVFS.Common/Git/GitRepo.csOpen PR FYI (not modified here)
These branches are already under active PR review, so this PR only flags them for maintainers/reviewers:
Fix NullReferenceException when cloning into a non-empty directoryby@tyrielvLibGit2Repopattern inCloneVerb.GetTrustPackIndexes(...)as a one-off local fix on that branchValidation
dotnet build GVFS\GVFS.UnitTests\GVFS.UnitTests.csproj -c Debugout\GVFS.UnitTests\bin\Debug\net10.0-windows10.0.17763.0\win-x64\GVFS.UnitTests.exe