[pull] master from microsoft:master#105
Merged
Merged
Conversation
All native hooks (virtual-filesystem, read-object, post-index-changed) and managed hooks (GVFS.Hooks) resolved the primary enlistment root by walking up from CWD looking for a .gvfs/ directory. This meant worktrees placed outside the enlistment directory tree (e.g. in a temp directory or as a sibling) could not use git commands — the hooks would fail with 'must be run from inside a GVFS enlistment'. Fix by adding a worktree fallback path: when .gvfs walk-up fails, walk up looking for a .git file (indicating a linked worktree), then resolve the primary enlistment root through the gitdir chain — first from the gvfs-enlistment-root marker file, then by deriving from commondir. The resolved root is validated by checking that .gvfs/ exists there. Native hooks (common.windows.cpp): - Extract ReadFirstLine, Utf8ToWide, TryParseGitFile helpers - Refactor GetWorktreePipeSuffix to use shared helpers - Add TryResolveFromWorktree that returns both enlistment root and pipe suffix - GetGVFSPipeName tries .gvfs walk-up first, then worktree fallback Managed hooks (Program.cs): - After TryGetGVFSEnlistmentRoot fails, try TryGetWorktreeInfo -> GetEnlistmentRoot() before exiting - Validate .gvfs exists at the resolved root Tests: - Unit tests for GetEnlistmentRoot with marker file, SharedGitDir fallback, and marker-preferred scenarios - Functional test creating worktree in temp directory, verifying git status from root and subdirectory, file projection, commits Assisted-by: Claude Opus 4.6 Signed-off-by: Ty Larrabee <tyrielv@gmail.com>
GVFS launches git.exe processes with CreateNoWindow=true (or CREATE_NO_WINDOW in native code). Despite the name, this flag tells Windows to create a new hidden console for each child process, which allocates a conhost.exe instance. For frequent, small git operations (e.g., during prefetch), the per-process conhost creation/teardown overhead is disproportionately large. Changes: - GitProcess.cs: Set CreateNoWindow=false. With UseShellExecute=false and stdout/stderr redirected to pipes, the child inherits the parent's console state. Since GVFS.Mount runs as a service with no console, the child gets no console and no conhost. Also remove the unused redirectStandardError parameter (all callers pass true). - ProcessHelper.cs: Set CreateNoWindow=false unconditionally. When redirectOutput is true, I/O goes through pipes so no console is needed. When redirectOutput is false, the child inherits the parent's console handles, which is correct for terminal contexts and harmless in service contexts (output was already going to an invisible hidden console). - GitHooksLoader.cpp: Use DETACHED_PROCESS instead of CREATE_NO_WINDOW in the no-console branch. Explicitly detaches from any console without allocating a new one. The console branch (user terminal) is unchanged. Verified with edge-case tests across terminal, hidden-console, and fully-detached (DETACHED_PROCESS) parent contexts. All git status, git fetch (prefetch hook), and gvfs health scenarios pass. Assisted-by: Tyrie Vella <tyrielv@gmail.com> Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Git's "Updating files" progress output on stderr is timing-dependent, causing ErrorsShouldMatch to fail when one repo emits progress and the other does not. Set GIT_PROGRESS_DELAY=3600 on all test git invocations so progress output is never emitted for commands completing under an hour. This eliminates the flake at the source without skipping error validation. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…iminate-conhost Eliminate unnecessary conhost.exe from git.exe and hooks process launches
GVFS's orphan-lock check (GVFSLock.LockHolder.GetExternalHolder) only
asked the OS "is there a process at this PID?" via IsProcessActive. If
the original lock-holder exited and Windows recycled its PID to an
unrelated process before the next git command arrived, GVFS saw a live
process at that PID and concluded the lock was still held. The git
command then either waited the full 300s timeout or printed a spurious
"Waiting for '<holder>' to release the lock" message.
This is a real product bug, not just a test issue. It surfaced as
flakiness in the OrphanedGVFSLockIsCleanedUp functional test on busy CI
agents (two failure modes: 300s timeout, and assertion failure with
non-empty stderr), but the same race can affect users when a PID is
quickly reassigned by the OS scheduler.
The fix is process-identity tracking. When the lock is granted to an
external requestor, capture that process's creation timestamp
(GetProcessTimes, an opaque 100-ns FILETIME value) alongside its PID.
On every orphan check, look up the current creation timestamp at that
PID and compare. If the process is gone the lookup fails; if a
different process is now at the PID the timestamps differ. Either way
the lock is correctly recognized as orphaned.
Implementation notes:
* New abstract GVFSPlatform.TryGetActiveProcessStartTime returns the
raw 64-bit creation time. It is documented as identity-only; the
value is never decoded as a date.
* The Windows implementation combines GetExitCodeProcess (STILL_ACTIVE
check) and GetProcessTimes. GetProcessTimes alone is not sufficient
because it still returns a creation time for terminated processes
whose kernel object is kept alive by an outstanding handle
elsewhere.
* Acquisition does not fail if start-time capture fails. The existing
IsProcessActiveImplementation has a Process.GetProcessById fallback
for cross-integrity callers that OpenProcess(QueryLimitedInformation)
cannot open, and we preserve the same compatibility surface here. A
HasStartTime flag is stored alongside the PID; when it is false the
orphan check falls back to the legacy IsProcessActive call. The
fallback is recorded in telemetry (StartTimeUnavailable=true on the
TryAcquireLockExternal event) so we can see if it ever becomes
common in the field.
* The ExternalLockHolderExited telemetry event now carries an
ExternalHolderTerminationReason field (ProcessNotActive | PidRecycled
| Unknown) to make future post-mortems straightforward.
* The OrphanedGVFSLockIsCleanedUp functional test is unchanged. It
polls Process.GetProcessById to detect when the lock holder has
exited, then runs git status. With this product fix, git status
succeeds cleanly even if the PID has already been recycled to an
unrelated process by the time mount processes the request, so the
test passes naturally without any test-side workaround.
* Two new unit tests cover the new identity-check path:
TryAcquireLockForExternalRequestor_WhenHolderPidRecycled (verifies
that an orphan is detected when start times differ at the same PID)
and TryAcquireLockForExternalRequestor_WhenHolderStartTimeMatches
(the inverse: a still-alive holder is correctly seen as active and
a new acquisition is denied).
Assisted-by: Claude Opus 4.7
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Fix PID-recycling race in orphan-lock detection
The official release pipeline has been failing since libgit2 was
statically linked via vcpkg, because the pool's build agents do not have
direct public-internet access. vcpkg was reaching out to sourceforge.net
(and other origins) for libgit2 transitive dependencies (pcre, zlib,
http_parser, ...) which the network layer blocked, causing
WinHttpSendRequest 12029 errors.
Route vcpkg asset downloads through the internal Terrapin cache (the
canonical CI asset-caching service).
This is opt-in. External contributors and the public GitHub Actions
PR-validation workflow (.github/workflows/build.yaml) continue to use
the standard vcpkg flow with direct downloads from public origins. No
internal feed authentication is required to build VFS for Git from a
fresh clone, and the repo source code intentionally contains no msbuild
SDK references that would pull internal packages into a default restore.
Changes:
Directory.Build.props
Add IsLocalBuild and UseTerrapinAssetCache properties, both
defaulted to false. IsLocalBuild controls Terrapin upload-on-fetch
(allowed on dev boxes only); UseTerrapinAssetCache opts the build
into the Terrapin asset-cache machinery (release pipeline only).
Directory.Build.targets
Build a --x-asset-sources string that combines the Terrapin
retrieval script with x-block-origin, gated on
UseTerrapinAssetCache=true, and append it to both vcpkg install
invocations. Add a guard <Error> that requires the pipeline to
supply TerrapinRetrievalToolPath up front when asset caching is
enabled. Also validate static git2.lib (was only checking dynamic
git2.dll).
.azure-pipelines/official-release-nuget.config (new)
Pipeline-only NuGet config that points at an internal feed. Used
solely by a one-off `nuget install` of Microsoft.Build.Vcpkg, not by
any csproj restore. Not consulted by external contributors or by
GitHub Actions.
.azure-pipelines/release.yml
Add NuGetAuthenticate@1 task. Add a NuGetCommand@2 task that
out-of-band installs the internal Microsoft.Build.Vcpkg package
(which ships TerrapinRetrievalTool.exe) into $(Agent.TempDirectory).
Add an explicit "Restore vcpkg native libraries (Terrapin cache)"
step that invokes _RestoreVcpkgDependencies on GVFS.Common.csproj
with -p:UseTerrapinAssetCache=true and TerrapinRetrievalToolPath
pointing at the extracted tool. Build.bat's own vcpkg install step
then skips because the libs are already present.
Assisted-by: Claude Opus 4.7
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
hooks: resolve enlistment root for worktrees outside enlistment tree
Fix flaky CreatePlaceholderTests stderr comparison
…g-fix Release pipeline: route vcpkg through Terrapin asset cache
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 : )