[pull] master from microsoft:master#112
Merged
Merged
Conversation
The Yes/No/Cancel MsgBox shown when mounted repos are detected during
install was confusing -- the Yes option meant "keep repos mounted"
(the less-common, advanced staging case), which inverted user
expectations and required reading the message body carefully to map
button semantics to outcomes.
Replace it with a custom modal containing two radio buttons and
Continue/Cancel:
(*) Remount repos as part of the installation
They will be temporarily unavailable.
( ) Keep repos mounted
The upgrade will complete automatically when all repos are
unmounted, or at next reboot.
The remount option is selected by default, matching the previous IDYES
default's intent (proceed with the common path).
Silent-mode STAGEIFMOUNTED=true|false behavior is unchanged.
Assisted-by: Claude Opus 4.7
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
When no previous commit exists to diff against (sourceTreeSha == null), DiffHelper.PerformDiff previously ran 'git ls-tree -r -t HEAD' which walks all tree objects. On a large repo with ~2.5M files, this takes ~24s. Replace with 'git ls-files -s' which reads the index instead of walking tree objects. Benchmarked at ~6.5s on the same repo — a 3.7x speedup. The optimization is only applied when targetTreeSha matches HEAD's tree, since ls-files reads the index (which reflects HEAD). When they differ (e.g., FastFetch checking out a non-HEAD commit), falls back to ls-tree to preserve correctness. Also falls back to ls-tree if ls-files fails (e.g., index does not exist on fresh git init before first checkout). Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The modified-paths database file is an append-only log of "A path" /
"D path" entries that ModifiedPathsDatabase compacts at the end of each
background-op batch via WriteAllEntriesAndFlush in PostBackgroundOperation.
BackgroundOperationCount, however, drops to 0 inside DequeueAndFlush for
the last task -- before the post-callback runs. WaitForBackgroundOperations
polls until count == 0 and can therefore return between dequeue and
compaction, leaving the file in a state like:
A temp.txt
D temp.txt
ModifiedPathsShouldNotContain matched both lines, fed two results into
EnumerableShouldExtensions.ShouldNotContain, and the SingleOrDefault call
threw "Sequence contains more than one matching element" -- masking the
underlying race with a confusing exception.
The product code is correct: FileBasedCollection.TryLoadFromDisk replays
the A/D log on mount, so the on-disk state is always recoverable. This
change fixes the tests:
* GVFSHelpers.ModifiedPathsShouldContain / ModifiedPathsShouldNotContain
now build the current set by replaying the A/D log (same algorithm as
TryLoadFromDisk) and check membership in that set. This matches the
semantic intent of every existing caller and is robust to the
flush-race window.
* ModifiedPathsContentsShouldEqual is renamed to
ModifiedPathsRawFileContentsShouldEqual and gains an XML doc comment
pointing callers at the semantic helpers unless they specifically need
to validate the compacted file layout. The two existing callers in
CheckoutTests are updated.
* EnumerableShouldExtensions.ShouldNotContain uses Where(predicate)
instead of SingleOrDefault(predicate) so multi-match cases produce a
useful Assert.Fail message instead of InvalidOperationException.
* A new ModifiedPathsDatabaseTests.AddFollowedByDeleteIsRecoveredOnLoad
unit test pins down the recovery contract -- loading
"A temp.txt\r\nD temp.txt\r\n" produces an empty set
(only the auto-added .gitattributes entry).
Assisted-by: Claude Opus 4.7
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Per Keith's review, the two CheckoutTests callsites that used ModifiedPathsRawFileContentsShouldEqual were exposed to the same compaction race the rest of this PR fixes -- WaitForBackgroundOperations can return before PostBackgroundOperation rewrites the file, so any transient "A path / D path" cycle (today: none; future: easy to introduce) would break the exact-equals raw-bytes assertion. Those callers' real intent is semantic: "after this checkout sequence, the only modified path is .gitattributes". Replace the raw helper with a new ModifiedPathsShouldOnlyContain that compares against the replayed A/D log as a set. The raw helper now has zero callers and is removed. Assisted-by: Claude Opus 4.7 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Fix flaky ModifiedPathsTests by replaying the on-disk log
…ged entries Fix three issues raised in PR review: 1. TargetMatchesHeadTree compared HEAD's tree SHA against a commit SHA (callers pass commit IDs, not tree IDs). The comparison never matched, so the optimization silently fell back to ls-tree every time. Fix by resolving targetTreeSha to its tree SHA via GetTreeSha() before comparing. 2. Add comments clarifying that the ls-files path intentionally skips directory operations. This is safe because the path only fires for gvfs prefetch on GVFS-mounted repos where directories are virtualized by PrjFlt. FastFetch force-checkout (which needs directory ops) targets a non-HEAD commit and falls back to ls-tree. 3. Filter out non-zero stage entries (unmerged) in the ls-files parser. During merge conflicts the same path appears at stages 1/2/3 with different SHAs. While GVFS repos shouldn't have conflicts, filtering defensively avoids duplicate adds with wrong blob SHAs. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Installer: replace confusing mount prompt with radio dialog
Use git ls-files -s instead of ls-tree for full-tree enumeration
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )