[pull] master from microsoft:master#113
Merged
Merged
Conversation
Two related fixes that together let `gvfs mount` succeed against an
enlistment whose pre-command hook is stale - typically because the GVFS
install location baked into .git/hooks/pre-command.hooks at clone time
has since moved (re-install, version-junction swap, system-to-user
migration, etc.).
Before this change, a stale .hooks text file causes every git invocation
that fires the pre-command hook to fail with:
fatal: pre-command hook aborted command
which makes the mount path unrecoverable.
Changes:
HooksInstaller.TryUpdateHooks
Also refresh the .hooks text files. Previously TryUpdateHooks only
refreshed the .exe copies of GitHooksLoader; the .hooks text file
(containing the absolute path of GVFS.Hooks.exe that the loader
execs) was only written at clone time by InstallHooks. When the
GVFS install moves, the .exe copies stay valid but the .hooks
path goes stale - and gvfs.mount.exe's existing TryUpdateHooks
call didn't repair it.
The new TryInstallGitCommandHooks calls are idempotent: when the
GVFS install path hasn't changed, the file is rewritten with the
same content.
GitProcess
Pass usePreCommandHook:false on all git operations that run during
the mount bootstrap path. These calls happen before gvfs.mount.exe
reaches TryUpdateHooks, so without this flag they trip over the
very stale-hook config we're trying to repair.
Affected: SetInLocalConfig, AddInLocalConfig, DeleteFromLocalConfig,
TryGetAllConfig, TryGetConfigUrlMatch, TryGetCredential,
TryGetCertificatePassword, TryDeleteCredential, TryStoreCredential.
GetFromConfig, GetFromLocalConfig and IsValidRepo also gain the
flag (some via the existing GetOriginUrl pattern, some new). None
of these operations mutate the working tree, so pre-command hook
is semantically inappropriate anyway - skipping it is correct
independent of the stale-hook scenario.
The mechanism: usePreCommandHook:false sets the COMMAND_HOOK_LOCK
environment variable, which Microsoft Git itself reads to suppress
pre-command hook invocation. So the failure is bypassed at the
git layer, not just inside GVFS.Hooks.exe.
Testing:
- 818/818 unit tests pass
- Manually verified end-to-end with a real enlistment whose
pre-command.hooks was corrupted to point at a non-existent path
(C:\NonExistent\Path\GVFS.Hooks.exe). Before this change, `gvfs
mount` failed with "pre-command hook aborted command". After this
change, mount succeeds and the .hooks file is rewritten to point
at the currently-running GVFS install.
Assisted-by: Claude Opus 4.7
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Mount: self-heal stale hook configurations
Replace the heavyweight git_revparse_single call in LibGit2Repo.ObjectExists with git_odb_exists, a purpose-built existence check that skips revparse expression parsing and git_object handle allocation. Benchmarked on an os.2020 enlistment (59.7M objects, 14 packs): - Existing objects: ~800 ns/op (comparable) - Missing objects: 1.3ms vs 2.8ms (2.1x faster) The ODB handle is lazily acquired on first ObjectExists call via git_repository_odb (returns the repo's internal ODB, ref-counted) and freed in Dispose. Concurrent first-time calls are safe via Interlocked.CompareExchange — the loser frees its duplicate handle. Falls back to revparse if ODB acquisition fails. Add ObjectCanBeParsed method that retains the old revparse behavior for callers that need corruption detection (LooseObjectsStep) or that may receive refs/abbreviated SHAs rather than full 40-char hex (BlobPrefetcher.DownloadMissingCommit, which takes raw CLI input). Assisted-by: Claude Opus 4.6 Signed-off-by: Tyler Vella <tyrielv@gmail.com>
Replace Stream.Read with Stream.ReadExactly where the caller expects all requested bytes (EnlistmentHydrationSummary, ReusableMemoryStream). In GitRepo.ReadLooseObjectHeader, check the Read return value instead of switching to ReadExactly — ReadExactly would throw EndOfStreamException on a truncated header, routing to the IOException catch (LooseBlobState.Unknown) instead of the header-mismatch path (LooseBlobState.Corrupt) that quarantines the file. Suppress CA2022 in GitIndexParser.ReadNextPage where partial last-page reads are intentional and the parser stops after entryCount entries. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Use git_odb_exists instead of git_revparse_single for ObjectExists
Fix CA2022 warnings: avoid inexact Stream.Read calls
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 : )