Release M275#1988
Merged
Merged
Conversation
Previously, FastFetch detected case-only renames (e.g. TestFolder -> testfolder) but suppressed them with a warning, leaving the working directory with stale casing. This change makes FastFetch propagate case renames to disk, matching the behavior of regular git checkout. Changes: - DiffTreeResult: Add SourcePath property to carry old-cased path for directory case renames - DiffHelper: Detect case-only renames and flow them through the pipeline instead of suppressing. Track case-renamed directories to properly filter child operations in FlushStagedQueues - CheckoutStage: Perform two-step directory rename (old -> temp -> new) to work around Directory.Move() failing on case-only renames on Windows - Update unit tests to verify SourcePath, delete counts, and directory operation filtering for case renames - Update functional test to verify casing with ignoreCase: false Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Context: EnqueueOperationsFromDiffTreeLine handles the case where adding a new DiffTreeResult to stagedDirectoryOperations collides with an already- staged entry under the case-insensitive DiffTreeByNameComparer. When the collision happens, the code needs to find the existing staged entry (by case-insensitive path equality) so it can capture the old casing for a directory case rename. Today the Modify/Add branch is the only collision branch that performs that lookup, and the lookup is written inline as a foreach scan over the staged set. A symmetric Delete-after-Add ordering also needs the same lookup, so the inline pattern is about to be duplicated. Justification: Extracting the lookup before introducing the second caller keeps the upcoming bugfix focused on the behavior change. The helper has a single responsibility — find a staged DiffTreeResult by case- insensitive TargetPath — which is meaningful on its own and easy to test in isolation if needed. Mutating an item already in the HashSet remains safe because DiffTreeByNameComparer keys on TargetPath only; SourcePath (the field the callers will mutate next) is not part of the hash or equality. Implementation: Adds private FindStagedDirectoryOperation(string targetPath) which returns the first staged DiffTreeResult whose TargetPath matches under GVFSPlatform.Instance.Constants.PathComparison, or null if none. The existing inline foreach in the Modify/Add collision branch is replaced with a call to this helper. No observable behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context:
Branch eric-pr added handling for case-only directory renames in
FastFetch on Windows: when diff-tree reports a Delete and an Add for
the same case-insensitive path, DiffHelper collapses the pair into a
single Add carrying the old-cased path on SourcePath, and CheckoutStage
performs a two-step Directory.Move to apply the casing change on disk.
That logic lives in the Modify/Add collision branch of
EnqueueOperationsFromDiffTreeLine: when an Add arrives and finds a
Delete already staged for the same path, it captures the Delete's
TargetPath onto SourcePath. The symmetric Delete branch was a no-op
with a comment claiming "diff-tree outputs Deletes before Adds".
That assumption is wrong. git diff-tree compares tree entries by byte
order of the path name, so for a case-only rename the Add or Delete
can appear first depending on which casing sorts lower. For example
"GVFLT_*" -> "GVFlt_*" emits the Delete (uppercase 'L', 0x4C) before
the Add (lowercase 'l', 0x6C), but the reverse rename "GVFlt_*" ->
"GVFLT_*" emits the Add first. In the latter case the Delete branch
fires, the staged Add is left without a SourcePath, and CheckoutStage
falls back to plain Directory.CreateDirectory — the original-cased
directory is never renamed and remains on disk.
Justification:
Two approaches were considered: (a) buffer the entire diff-tree
output and sort it before parsing, forcing a stable ordering invariant;
(b) make the staging code order-independent so it handles either
arrival order correctly.
Option (a) regresses memory: FastFetch streams diff-tree line-by-line
today and is run against multi-million-file repos, so buffering the
whole output is meaningful. Option (a) also relies on a git emit-order
invariant that git does not document — making the C# layer robust is
the right place to defend against future diff-tree changes.
Option (b) is the smaller, more localized fix and is implemented here.
Implementation:
EnqueueOperationsFromDiffTreeLine's Delete-collision branch now mirrors
the Modify/Add branch: if a HashSet collision under
DiffTreeByNameComparer indicates an Add was already staged for the
same case-insensitive path, find that Add via
FindStagedDirectoryOperation and — when the existing TargetPath
differs from the incoming TargetPath under ordinal comparison —
annotate the staged Add's SourcePath with the old (incoming Delete's)
casing, and register the old-cased path in caseRenamedDirectoryDeletes
so FlushStagedQueues can filter children of the renamed directory.
The staged entry is mutated in place. This is safe because
DiffTreeByNameComparer hashes and compares only on TargetPath; the
SourcePath field is a runtime annotation that the comparer does not
see, so mutation does not corrupt the HashSet.
A new fixture caseChangeAddFirst.txt exercises the reverse-direction
rename ("GVFlt_*" -> "GVFLT_*") with the Add lines emitted before the
Delete lines, and the matching test ParsesCaseChangesWhenAddPrecedesDelete
asserts the staged directory operation carries the correct SourcePath
and that child operations are filtered exactly as in the Delete-first
case. Without this fix, the test fails because SourcePath is null.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context: CheckoutStage performs case-only directory renames via a two-step Directory.Move: source -> temp (a sibling of the target named with a random GUID suffix), then temp -> target. The two-step is required on Windows because Directory.Move throws IOException when the source and destination differ only in case. If the first move succeeds but the second fails — for example because an antivirus or indexer has opened a handle on the moved tree, the target path has unexpected ACLs, or transient I/O fails the second call — the existing catch handler logs the error, sets HasFailures, and leaves the directory stranded at the temp path. A subsequent FastFetch run sees no source at the original casing and falls through to Directory.CreateDirectory on the target path. The end state is an empty target directory next to a "*_caseRename_<guid>" directory holding the user's actual content, with no automatic recovery. Justification: The cleanest recovery is to undo the half-applied rename: if the second move throws, attempt to move the tree back from the temp path to the original source casing. A subsequent run then sees the same state as before the failed attempt and can retry deterministically. The restoration attempt itself can fail (same underlying I/O issue that broke the second move) so it is best-effort and swallows exceptions, leaving the outer catch handler to log the original failure. Compared to alternatives — staging the rename under .git/ instead of as a sibling, or running a cleanup pass for stranded temp directories on startup — this is the smallest change that addresses the data-loss path without introducing new state. Implementation: The two Directory.Move calls are wrapped so that a failure of the second Move triggers a best-effort Directory.Move(tempPath, trimmedSourcePath). Guarded by Directory.Exists checks so we never clobber a partially-restored source. After the recovery attempt, the original exception is re-thrown so the outer catch logs the actual failure (not a synthetic restore-success). The trimmed source and target paths are now hoisted to locals to keep the inner block readable. No behavior change on the success path or for non-rename Adds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context: EnqueueFileAddOperation iterates stagedFileDeletes to decide whether a staged delete with a matching path differs from the incoming add only in case. The loop captures the matched delete path into a local named existingDeletePath, but nothing after the loop reads that local — only exactMatch is consumed. Justification: Dead state is misleading: a future reader expects an unused capture to be the seed for a planned use (logging the old casing, returning it from a helper) and may build new code around it. Removing the local makes the loop's actual contract — set exactMatch, then break — self-evident. Implementation: Drops the existingDeletePath declaration and the one assignment that populated it. No behavior change; tests continue to pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context: DiffHelper accumulates diff results into internal staged sets (stagedDirectoryOperations, stagedFileDeletes, filesAdded, and the new caseRenamedDirectoryDeletes) and into four public output collections: DirectoryOperations, FileDeleteOperations, FileAddOperations, and RequiredBlobs. FlushStagedQueues calls RequiredBlobs.CompleteAdding() at the end of each diff, which permanently closes the BlockingCollection. A second call to PerformDiff or ParseDiffFile on the same instance would also flow values into the now-frozen internal staged sets, which were never cleared between runs. caseRenamedDirectoryDeletes in particular would carry forward old-cased paths from the first diff and mis-filter children in the second. Today every caller in the tree constructs a fresh DiffHelper per diff, so the bug is latent. But the class never declared this constraint, the field initializers and the BlockingCollection together silently assume single-use, and a future caller could re-enter the methods expecting a usable result. Justification: Two approaches were considered: (a) reset internal staged sets at the start of each call and reinstantiate the output collections, or (b) detect reuse and throw at the entry points. Option (a) would invalidate any external code holding references to the output collections (they are exposed as public properties and consumed concurrently by other prefetch stages), so it would silently change semantics for anyone observing the queues across the boundary. Option (b) preserves the existing single-use intent, surfaces misuse loudly and early instead of via an obscure BlockingCollection exception deep in parsing, and requires no behavior change for current callers. Implementation: Adds a diffPerformed bool, set at the start of EnsureSingleUse — the new private helper called at the top of PerformDiff(string, string) and ParseDiffFile. A second invocation throws InvalidOperationException with a message that names the class and suggests constructing a new instance. The one-arg PerformDiff overload delegates to the two-arg overload and so is covered transitively. A new unit test DiffHelperThrowsOnReuse calls ParseDiffFile twice on the same instance and asserts the second call throws. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context: The internal HashSet introduced for case-rename support was named caseRenamedDirectoryDeletes, but it does not store Delete operations or paths of directories slated for deletion. It stores the old-cased TargetPath of each directory whose Delete entry was collapsed into a case-rename Add — paths that FlushStagedQueues uses to recognize and filter out child operations that would otherwise look like they belong to a deleted directory. The mismatch between the name and the contents made the two read sites — FlushStagedQueues's parent-path check and the Modify/Add and Delete collision branches — harder to follow on first reading. The field is also referenced in the comment chain of two new commits, so readers chasing context bounce between "deletes" in the name and "replaced by a rename" in the comments. Justification: Renaming clarifies what the set represents at every call site. The new name "directoriesReplacedByCaseRename" reads as a description of its contents and aligns with the comment that already explains it. The XML-comment block above the field is also expanded so the contract — *old-cased paths, used to suppress child operations* — is documented in one place instead of being implied across three unrelated branches. Implementation: Mechanical rename of all three references (the field declaration, the UnionWith call in FlushStagedQueues, the Add calls in the Modify/Add and Delete collision branches). No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context:
DiffTreeResult.SourcePath was introduced as a parser annotation:
DiffHelper sets it when it collapses a Delete+Add pair under the
case-insensitive comparer into a single case-rename Add, and
CheckoutStage reads it to perform the rename on disk. The setter
was declared public, which exposed a runtime-only field as part of
the type's external contract.
Public mutability invites future code outside DiffHelper to set the
field on operations the parser would never have annotated — for
example on a Delete or a Modify entry, or on a file operation — and
CheckoutStage's branches would then behave in ways that were never
validated. The XML doc previously said only "Used on case-
insensitive file systems to carry the old-cased path for directory
renames," which did not communicate the intended single-producer
contract.
Justification:
Restricting the setter to the assembly keeps the producer
(DiffHelper) able to write it while ruling out external producers.
The getter stays public because consumers in other assemblies
(CheckoutStage in FastFetch, the existing DiffHelperTests
assertions) need to read it. The single-producer rule is also
documented in an expanded XML comment that names the parser and
describes when the field is null versus non-null.
Implementation:
Changes "public string SourcePath { get; set; }" to
"public string SourcePath { get; internal set; }" and rewrites the
XML doc to describe the contract: who sets it, who reads it, when
it is null. No build-time consumer outside the assembly was writing
to it (only DiffHelper writes; CheckoutStage and the unit tests
read), so this is a non-breaking visibility tightening.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…fHelper
Context:
Case-rename detection needs both case-insensitive containment (do we
have *some* entry matching this path under PathComparer?) and
case-sensitive comparison (does the stored casing exactly match the
incoming path?). HashSet provides the first in O(1) but the second
required a foreach scan of the entire set because HashSet has no API
to retrieve the stored value from a case-insensitive lookup.
Three diff-tree code paths inherited that scan:
- EnqueueFileAddOperation, scanning stagedFileDeletes when an add
arrives whose path matches a staged delete case-insensitively
- EnqueueFileDeleteOperation, scanning filesAdded for the same
reason on the inverse direction
- The Modify/Add and Delete collision branches in
EnqueueOperationsFromDiffTreeLine, scanning stagedDirectoryOperations
via FindStagedDirectoryOperation to recover the staged
DiffTreeResult and its TargetPath casing
These scans are gated by a case-insensitive Contains check, so the
inner loop only runs when there is actually a collision — but on each
collision it walks every staged entry. The staged sets can reach
millions of paths on first checkouts of large repositories (the
Windows monorepo, Office, etc.), so a worst-case parse with many
collisions degrades from O(N) to O(N^2).
Justification:
Dictionary<string, T> keyed by GVFSPlatform.Instance.Constants.PathComparer
solves both needs at once: the comparer drives O(1) case-insensitive
key matching, and TryGetValue returns the originally stored value so
callers can compare its casing ordinally without iterating. The
storage shape lines up exactly with what the existing logic needs.
DiffTreeByNameComparer becomes unused after stagedDirectoryOperations
is rekeyed by TargetPath string, so it is removed in this commit
along with FindStagedDirectoryOperation (now a single TryGetValue
call). No public surface area changes.
Implementation:
- filesAdded, stagedDirectoryOperations, and stagedFileDeletes are
declared as Dictionary instances. The two string-valued
dictionaries map a path under the case-insensitive comparer to
its original casing; the directory-valued dictionary maps the
case-insensitive path to its staged DiffTreeResult.
- HashSet.Add returning false becomes Dictionary.TryAdd returning
false at every call site. The Remove+Add idiom used to "replace"
a staged entry becomes a single indexer assignment.
- Two foreach scans collapse into TryGetValue + ordinal compare;
the collision branches in EnqueueOperationsFromDiffTreeLine
consult the dictionary directly via the indexer because the
immediately preceding TryAdd has already proven the key is
present.
- FlushStagedQueues iterates .Values for both stagedDirectoryOperations
and stagedFileDeletes.
- The dead helper FindStagedDirectoryOperation and the
DiffTreeByNameComparer inner class are deleted.
All tests continue to pass; no behavior change is intended beyond
the performance characteristic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context:
The original case-rename commit on this branch removed three
RelatedEvent(Warning, "CaseConflict", ...) calls from the diff-tree
parsing path. Those traces had two jobs: signaling that a case-only
rename had been observed, and signaling a true duplicate diff entry
for the same path. After removal, both outcomes were silent — case
renames now happened invisibly, and any genuine diff anomaly that
collapsed into a "case rename" was undetectable in logs.
Silent success is mildly annoying for an operator; silent anomaly is
a debuggability hole, because the same code path now handles two
distinct conditions and the original logs no longer distinguish them.
Justification:
The two conditions deserve different log levels: a case rename is
expected on case-insensitive filesystems and should be Informational
so the operator can count them per fetch without alarm; a true
duplicate (incoming and staged paths match ordinally) is rare and
suggests a malformed diff stream, so it should be Warning so it
surfaces in normal triage.
The original "CaseConflict" event name conflated both. Replacing it
with two distinct events ("CaseRename" and "DuplicateDiffEntry")
gives each its own grep target without breaking the surviving
ls-tree path's "CaseConflict" event (which keeps its original
meaning — two sibling entries in the same tree differing only in
case).
Implementation:
Two small static helpers — TraceCaseRename and TraceDuplicate —
build EventMetadata with the kind ("File" or "Directory"), the
operation, and the old/new paths or the conflicting path. Each
collision branch in EnqueueOperationsFromDiffTreeLine and the
case-rename branches in EnqueueFileAddOperation /
EnqueueFileDeleteOperation now call one of the helpers depending
on whether the comparison is ordinally equal or not.
No behavior change beyond observability: the staged data structures,
the FlushStagedQueues output, and the existing unit tests are
unaffected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…yRename
Context:
HandleAllDirectoryOperations dequeues directory operations from the
diff and dispatches them by Operation kind. The Modify/Add arm grew
to a ~50-line inline body covering three subcases: a case-only
rename (two-step Directory.Move through a temp path, plus rollback
if the second move fails), a fallback for missing source (a parent
rename already moved this child), and the plain Add path.
That body sits inside a try/catch that also handles the plain Add,
and the catch block walks the same treeOp.SourcePath branches to
shape its EventMetadata. As the rename logic grew (rollback on
failure, separate trimmed-path locals, nested try/catch), the
overall switch arm became hard to scan: a reader has to mentally
peel four levels of nesting to find the simple "is this an Add or
a rename?" decision.
Justification:
The rename has a clear contract — given a DiffTreeResult with
SourcePath set, apply it on disk — that fits naturally into a
private method. Extracting it leaves the switch arm at one level of
nesting and keeps the catch block at the call site so its
EventMetadata shaping does not duplicate inside the helper.
The behavior of the existing branches is preserved exactly,
including the rollback-then-rethrow path so the outer catch still
sees the original exception (not a synthesized restore failure).
Implementation:
ApplyCaseOnlyDirectoryRename takes the DiffTreeResult and the
already-computed absoluteTargetPath. It returns early after a
Directory.CreateDirectory fallback when the source is missing, and
otherwise performs the trimmed-path two-step Move with the inner
rollback try/catch. The caller's switch arm now reads as:
if (treeOp.SourcePath != null)
{
this.ApplyCaseOnlyDirectoryRename(treeOp, absoluteTargetPath);
}
else
{
Directory.CreateDirectory(absoluteTargetPath);
}
No tests were added; all 796 existing tests continue to pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context: Case-rename detection lives in three places in DiffHelper: EnqueueOperationsFromDiffTreeLine's directory branches, EnqueueFileAddOperation, and EnqueueFileDeleteOperation. The existing fixtures (caseChange.txt, caseChangeAddFirst.txt) exercise only the directory branches — their test files all sit inside a case-renamed directory, so the file-level case-rename paths are indirectly suppressed by the parent-directory filtering in FlushStagedQueues. That leaves the standalone "rename foo.txt to FOO.txt with no directory casing changes" path completely uncovered. Both EnqueueFileAddOperation's case-rename branch and EnqueueFileDeleteOperation's case-rename branch ship without a test that exercises them end-to-end. Justification: A two-line fixture isolates the file-level path: one Delete entry for the old casing, one Add entry for the new casing, no directory operations. The resulting test pins the contract that DiffHelper both stages the delete (old casing, so the on-disk file is removed) and stages the add (new casing, so the replacement file is written), and that neither is filtered out as a duplicate. Implementation: New fixture Data/fileCaseChange.txt with a Delete for "foo.txt" followed by an Add for "FOO.txt" (the typical diff-tree emit order for that direction since "F" sorts before "f" byte-wise). New unit test ParsesFileOnlyCaseRename asserts RequiredBlobs has one entry, FileAddOperations and FileDeleteOperations each have one entry, and the casings flow through correctly: the delete carries "foo.txt" and the add carries "FOO.txt". The test is gated to case-insensitive filesystems because on a case-sensitive filesystem the two paths are distinct files and no case-rename detection runs (the existing ParsesCaseChangesAsRenames test covers the case-sensitive behavior). The csproj is updated so the fixture is copied to the test output directory. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Context:
The existing case-rename fixtures change only the top-level
directory's casing. Their children sit inside the renamed parent but
retain the parent's old casing in the diff. Nothing in the existing
test suite exercises a diff where both an outer directory and an
inner directory change casing in the same operation.
That scenario is reachable in production whenever a contributor
renames an entire path's casing (e.g., "Docs/API/" to "docs/api/")
and is the most interesting interaction between the case-rename
detection in EnqueueOperationsFromDiffTreeLine and the parent-path
filter in FlushStagedQueues: the inner rename's parent path is in
directoriesReplacedByCaseRename, so its Add must be suppressed
because the outer rename's Directory.Move already carries the child
on disk. If that suppression breaks, FastFetch would attempt to
CreateDirectory the inner path after the outer rename has moved
the entire subtree — at best a no-op, at worst a layered race.
Justification:
A six-line fixture (three Deletes, three Adds) is the smallest input
that captures the nested case where both renames need to be detected
*and* the inner one needs to be filtered. Asserting all of
FileAddOperations, FileDeleteOperations, TotalFileDeletes,
TotalDirectoryOperations, the enqueued DirectoryOperations count,
and the SourcePath/TargetPath on the surviving op pins the contract
end-to-end.
Implementation:
New Data/nestedCaseChange.txt with a "Outer/Inner/test" source tree
and an "outer/inner/test" target tree. New unit test
ParsesNestedCaseChanges asserts:
- One blob is required (the file under the renamed subtree)
- FileAddOperations carries the new-cased path
- The file delete is filtered out by parent-path matching
- TotalDirectoryOperations is 2 (both case-renames are staged)
- DirectoryOperations.Count is 1 (only the outermost survives
the parent-path filter)
- The surviving directory op carries the old casing on SourcePath
The test is gated to case-insensitive filesystems because on a
case-sensitive filesystem the two paths are independent and no
case-rename detection runs. The csproj is updated so the fixture is
copied to the test output directory.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review of case-rename support: ordering, rollback, observability, performance, tests
Replace the LibGit2Sharp.NativeBinaries NuGet package (which shipped a
dynamic git2-*.dll) with a vcpkg-built static git2.lib that the NativeAOT
linker embeds directly into the executable. This eliminates libgit2
DLL-missing crashes we see in telemetry.
A side benefit of this approach is that we can apply patches to libgit2
that haven't been included in an official release yet, via the vcpkg
overlay port in overlays/libgit2/.
How it works:
- vcpkg manifest (vcpkg.json) declares libgit2 as a dependency
- Custom triplet (x64-windows-static-aot) builds with static CRT and
library linkage, producing git2.lib instead of git2.dll
- Overlay port (overlays/libgit2/) pins libgit2 v1.9.3 and can carry
patches ahead of upstream releases
- Directory.Build.props wires <DirectPInvoke> and <NativeLibrary>
items so the NativeAOT linker resolves P/Invoke calls at link time
and embeds git2 + transitive deps (pcre, zlib, http-parser)
To rebuild vcpkg libs locally:
vcpkg install --triplet x64-windows-static-aot \
--overlay-triplets=triplets --overlay-ports=overlays
Changes:
- Directory.Build.props: DirectPInvoke + NativeLibrary for AOT link,
unconditional libgit2_filename=git2 (was git2-a418d9d from NuGet)
- Directory.Packages.props: remove LibGit2Sharp.NativeBinaries
- GVFS.Common.csproj, GVFS.Hooks.csproj: remove NuGet package ref
- Readme.md: reference THIRD-PARTY-NOTICES.md from license section
- THIRD-PARTY-NOTICES.md: document libgit2 (GPLv2 + linking exception),
SQLite (public domain), SQLitePCLRaw (Apache 2.0)
- vcpkg.json, vcpkg-configuration.json: vcpkg manifest + baseline
- triplets/x64-windows-static-aot.cmake: static linkage triplet
- overlays/libgit2/: port overlay (v1.9.3) from devprod.razzletools
- .gitignore: exclude vcpkg_installed/
License: libgit2's COPYING includes an explicit linking exception that
permits static linking without imposing GPL on the consuming program.
Assisted-by: Claude Opus 4.6
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Build script improvements: - Add vcpkg install to Build.bat with vswhere-based vcpkg discovery - Add _RestoreVcpkgDependencies MSBuild target (modeled after official vcpkg VcpkgInstallManifestDependencies) that auto-runs vcpkg install on first VS/dotnet build — uses stamp file for incrementality - Scope vcpkg restore target to GVFS.Common to avoid parallel conflicts - Move vcpkg output from src/vcpkg_installed/ to out/vcpkg_installed/ - Declare overlay-ports and overlay-triplets in vcpkg-configuration.json - Restore explicit CI vcpkg step using preinstalled VCPKG_INSTALLATION_ROOT - Exclude GVFS.MSBuild from libgit2 Content items (IncludeBuildOutput=false) - Remove vcpkg_installed/ from .gitignore (no longer in git root) Code fix: - Update giterr_last P/Invoke to git_error_last (deprecated since libgit2 0.28) Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Statically link libgit2 into NativeAOT executables via vcpkg
The installer replaces GVFS.Service.exe in-place even during a staged upgrade, so the running service is always the new version. Add a PendingUpgradeMonitor that registers Process.Exited callbacks on GVFS.Mount processes and applies the staged upgrade once all have exited. This is event-driven (no polling) and works regardless of which version of gvfs.exe triggered the unmount. Previously only 'gvfs unmount' triggered the upgrade check (via UnregisterRepoRequest). 'gvfs service --unmount-all' sets SkipUnregister=true to preserve automount registration, so no message reached the service and staged upgrades sat in PendingUpgrade\ indefinitely. Changes: - PendingUpgradeHandler: return UpgradeResult enum instead of void, add concurrency lock, add IsPending() helper - PendingUpgradeMonitor: new class — monitors mount process exits, debounces (5s), retries on new mounts, self-stops on completion - GVFSService: start monitor when startup upgrade is deferred, dispose on shutdown - upgrade-tests.yaml: add unmount-all-triggers-upgrade CI scenario Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…check Apply staged upgrade when mount processes exit
.NET 9+ introduced System.Threading.Lock as a dedicated, lightweight synchronization primitive. Migrate all private lock fields from `object` + `new object()` to `Lock` + `new Lock()`. Also replace Monitor.IsEntered() assertions in GitStatusCache with Lock.IsHeldByCurrentThread, since Lock is not compatible with Monitor APIs. 20 fields across 19 files updated. No Monitor.Wait/Pulse usage found in the codebase, so all instances are safe to migrate. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Replace object locks with System.Threading.Lock
The fileHandle obtained from CreateFileW was never closed after calling GetFinalPathNameByHandleW, leaking a kernel handle on every invocation. Add CloseHandle(fileHandle) on both the success path and the error path before die(). Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…river code
The ProjFS filter driver and native library are no longer bundled with
the GVFS installer — ProjFS is required as a Windows Optional Feature
(available since Windows 10 1809, our minimum supported OS).
Fix FilterAttach ACCESS_DENIED (bug 62349777):
- Add TryAttachToVolume() that tolerates ACCESS_DENIED when the ProjFS
service is already running (the filter is already attached to the
volume but the caller lacks SE_LOAD_DRIVER_PRIVILEGE).
- The service-side EnableAndAttachProjFSHandler.Run() now uses
TryAttachToVolume instead of raw TryAttach, matching the tolerance
already present in the client-side IsReady() path.
- Fix Run() to preserve the first error from TryEnablePrjFlt instead
of potentially overwriting it with a subsequent attach error.
Remove dead non-inbox ProjFS code paths:
- Remove TryInstallProjFSViaINF (referenced {app}\Filter\prjflt.inf
which is no longer shipped).
- Remove TryCopyNativeLibIfDriverVersionsMatch and supporting methods
(referenced {app}\Filter\prjflt.sys and {app}\ProjFS\ProjectedFSLib.dll
which are no longer shipped).
- Simplify TryEnableOrInstallDriver to always use the Windows Optional
Feature path; keep build number logging as best-effort telemetry.
- Simplify IsNativeLibInstalled to require System32; warn if stale
app-local DLL found from a legacy non-inbox install.
Improve error messaging:
- When ProjFS native library is missing, error now includes the
PowerShell command to enable the optional feature.
- When ProjFS cannot be enabled, error directs user to the optional
feature instead of referencing non-existent bundled files.
Assisted-by: Claude Opus 4.6
Signed-off-by: Tyler Vella <tyrielv@gmail.com>
Handle case-only file and directory renames in FastFetch on Windows
Add patch from libgit2/libgit2#7200 to vcpkg overlay. Allows non-elevated processes run by Administrators group members to be considered the owner of repositories owned by that group. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…mount TryUpdateHooks (called at mount time) updated the 3 native hooks (read-object, virtual-filesystem, post-index-change) but skipped the pre-command.exe and post-command.exe copies of GitHooksLoader.exe. Those were only deployed at clone time via InstallHooks, so upgrading GVFS and remounting left stale loader binaries in the enlistment. Refactor TryUpdateHook into an overload that takes explicit source and target paths, then call it from TryUpdateHooks for both command hook loaders using GitHooksLoader.exe as the source. The HookData overload becomes a thin wrapper that delegates to the new one. Add Assert-HookVersionsMatch to the staging-upgrade and clean-upgrade test scenarios: after upgrade completes, remount and verify all 5 hook binaries match the installed versions. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Transient failures in actions/download-artifact are a frequent source of flaky CI runs. Add a retry step after each download that triggers only when the first attempt fails (continue-on-error + outcome check). Applied to both functional-tests.yaml and upgrade-tests.yaml. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
When GVFS.Mount.exe fails due to authentication (credential fetch failure or 401/403 after credentialed retry), it now exits with ReturnCode.AuthenticationError (9) instead of GenericError (3). The gvfs.exe mount verb captures the GVFS.Mount.exe process handle and propagates its exit code on mount failure, so callers of 'gvfs mount' can distinguish auth failures from other errors. Changes: - Add ReturnCode.AuthenticationError = 9 - Add 'out bool isAuthFailure' to TryInitializeAndQueryGVFSConfig to explicitly classify auth failures (credential fetch failure or 401/403 on credentialed retry) - InProcessMount uses isAuthFailure to select the exit code - StartBackgroundVFS4GProcess returns Process so MountVerb can read the mount process exit code after failure Resolves AB#61375690 Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
upgrade-tests: verify hook binary versions after upgrade+remount
Add non-elevated admin owner patch to libgit2 overlay
ci: add retry for artifact download steps
…lpathname Fix handle leak in GetFinalPathName
GVFS.Service runs as SYSTEM and cannot read the user's global git config (where gvfs.telemetry-pipe is set) at startup. This meant TelemetryDaemonEventListener was never created, and all service telemetry events (PendingUpgradeHandler, etc.) were silently lost. Add two new classes in GVFS.Common.Tracing: - BufferingTelemetryListener: an EventListener that buffers telemetry messages in a bounded ConcurrentQueue, then replays them to a real listener on demand. - DeferredTelemetryAttacher: manages the lifecycle of deferred telemetry pipe attachment. Adds a BufferingTelemetryListener to the tracer, then periodically retries creating the real daemon listener (exponential backoff: 10s, 30s, 1m, 5m steady state). On success, replays buffered messages and stops the timer. Checks HasTelemetryDaemonListener to prevent duplicate listeners when the JsonTracer constructor already attached one. Designed for reuse by both GVFS.Service and GVFS.Mount. TelemetryDaemonEventListener gains a globalConfigPath parameter so callers can read a specific .gitconfig file via --file instead of --global. This lets GVFSService read the logged-on user's config without mutating the process-wide HOME environment variable. GVFSService creates a DeferredTelemetryAttacher at startup and calls TryAttach on session logon events, passing the user's .gitconfig path resolved via RunImpersonated. JsonTracer gains only HasTelemetryDaemonListener (one-liner property) to support the duplicate-listener guard. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Test BufferingTelemetryListener: buffer and replay, stop after replay, bounded cap, second replay returns zero. Test DeferredTelemetryAttacher: null gitBinRoot handling, retry interval exponential backoff values. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Add unique exit code for mount authentication failures
ProjFS: tolerate ACCESS_DENIED on FilterAttach, remove dead bundled-driver code
GVFS.Service: defer and retry telemetry pipe attachment
KeithIsSleeping
approved these changes
May 26, 2026
derrickstolee
approved these changes
May 26, 2026
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
4 participants
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.
Changes: