Repository navigation
Merge master into vnext - #2134
Merged
Tyrie Vella (tyrielv) merged 20 commits intoOct 8, 2026
Merged
Tyrie Vella (tyrielv) merged 20 commits into
Tyrie Vella (tyrielv) merged 20 commits into
Conversation
libgit2 treats every string it receives and returns as UTF-8. The LibGit2Repo P/Invoke declarations left CharSet unset, so a bare [DllImport] string parameter defaulted to CharSet.Ansi and encoded through the Windows ANSI code page. On a non-UTF-8 code page an unmappable character (for example a repository path under a non-English Windows user name, such as "C:\Users\...\café_日本語") was dropped to '?', so git_repository_open resolved the wrong path and the constructor threw InvalidDataException. Annotate every string parameter with [MarshalAs(UnmanagedType.LPUTF8Str)]. This covers the repository path (git_repository_open), the revspec (git_revparse_single), the object id string (git_oid_fromstr), and the config keys (git_config_get_string, git_config_get_bool, git_config_get_multivar_foreach). Read the borrowed git_error.message pointer as an IntPtr and copy it with Marshal.PtrToStringUTF8, matching the existing GitConfigEntry pattern, so the interop marshaller never frees libgit2's heap memory. The config value output path already used Marshal.PtrToStringUTF8, so it is unchanged. Add LibGit2NonAsciiPathTests, which opens a real repository at a non-ASCII path through git2.dll and reads a non-ASCII config value. Before the fix both cases fail with "No mapping for the Unicode character exists in the target multi-byte code page"; after the fix they pass. The fixture ignores itself when the active ANSI code page is already UTF-8 (CP65001), where the bug cannot reproduce. Document the UTF-8 marshalling contract on the LibGit2Repo class and in AGENTS.md so future libgit2 bindings do not regress. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…e-v2.55.0.vfs.0.10 Update default Microsoft Git version to v2.55.0.vfs.0.10
…arshalling Marshal libgit2 P/Invoke strings as UTF-8, not ANSI
Document payload publishing, named runner arguments, and local service path resolution. Warn about stale outputs and shared functional-test resources. Assisted-by: GPT-5.6 Sol Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
docs(agents): clarify functional test dev workflow
PR validation builds on the arm64 runner intermittently fail with `C3859: Failed to create virtual memory for PCH` while building one of the native hook projects (e.g. GVFS.PostIndexChangedHook). This is a transient cl.exe failure caused by memory pressure on the build host, not a problem with the code being built, and the same build reliably succeeds on a manual re-run. Build.bat now retries each native vcxproj build (GitHooksLoader, GVFS.NativeTests, GVFS.PostIndexChangedHook, GVFS.ReadObjectHook, GVFS.VirtualFileSystemHook) up to 3 times, with a short delay between attempts, before failing the build. This removes the need for a human to notice the failure and manually re-run the pipeline. Assisted-by: GitHub Copilot Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
The previous commit retried a native vcxproj build on any MSBuild failure, which would also retry (and mask, and waste time on) a real compile error introduced by a code change. Capture each attempt's output and only retry when the log actually contains the transient C3859 "Failed to create virtual memory for PCH" error; any other failure now fails immediately on the first attempt. Assisted-by: GitHub Copilot Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…ch-build Retry native C++ project builds on transient MSBuild PCH failure
In a GVFS enlistment, a hydrated placeholder is on disk but is not in ModifiedPaths, so it still has skip-worktree. A mixed reset does not update the working tree. If git leaves skip-worktree set on a hydrated placeholder whose index entry the reset changes, reset and status do not report the file as modified. The git side of this behavior requires microsoft/git v2.55.0.vfs.0.3 or later. Add two tests: - CorruptionReproTests.ReproResetMixedSkipWorktree is a black-box test. It runs the command sequence that exposed the problem (blame, then reset HEAD~1) and compares the results with the control repo. - ResetMixedTests.ResetMixedClearsSkipWorktreeOnHydratedPlaceholder is a white-box test. It asserts each precondition before the reset: the index entry changes, the file is hydrated, the file is not in ModifiedPaths, and skip-worktree is set. After the reset, it asserts that skip-worktree is cleared and that GVFS adds the file to ModifiedPaths. Also document on GitHelpers.ValidateGitCommand that it compares status output after each command other than status. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…d-skipworktree Add functional test for reset --mixed skip-worktree bug (depends on microsoft/git#935)
Expose whether outgoing gvfs.exe requests are sent without an Authorization header via the periodic VFS.Heartbeat event, sourced from the enlistment's existing public GitAuthentication.IsAnonymous property. Application Insights shows a fleet failure mode where mounts get stuck unable to download blob sizes or objects (SizesUnavailableException), affecting roughly 6% of active machines with no repro in hand. The suspected cause is an anonymous-auth latch: when the initial anonymous config probe comes back indeterminate, GitAuthentication never clears IsAnonymous, so HttpRequestor permanently omits the Authorization header. Today no heartbeat or telemetry field exposes this state, so the mechanism cannot be confirmed or sized in the field. This change is intentionally scoped away from GitAuthentication.cs and HttpRequestor.cs, which are being reworked by unrelated in-flight fixes for the same bug. The new field is named for the observable effect on the wire (no Authorization header sent) rather than for the internal property, and reads the same way regardless of which build emits it, even though IsAnonymous's default value before the first auth probe completes differs between the current code and the in-flight fix. Assisted-by: Claude Sonnet 5 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Background GVFS processes such as GVFS.Mount.exe run with a hidden console window. When git launches the Git Credential Manager from such a process, the credential manager parents its interactive sign-in prompt to the hidden console. The prompt then has no foreground right and sinks behind other windows, so the user never sees it. Launch git detached from the console (CreateNoWindow=true) for commands that may require authentication when the current process has no visible console window. With no console, git starts the credential manager detached, so the credential manager parents its prompt to a topmost stub window instead of the hidden console. A process with a visible console window (e.g. gvfs.exe run from a classic console) keeps the current behavior, so the credential manager parents the prompt to that window. Detection keys on the console window, not an arbitrary GUI window. A process whose console is hidden, or a non-displayed pseudo-console such as a foreground gvfs.exe run from Windows Terminal, is treated as having no visible window and also detaches git. For the Git Credential Manager this is equivalent or better, because the detached prompt is topmost instead of owned by a hidden host window. Add GVFSPlatform.CurrentProcessHasVisibleConsoleWindow. The Windows implementation checks GetAncestor(GetConsoleWindow(), GA_ROOTOWNER) and IsWindowVisible. Add a mayRequireAuth option to GitProcess.GetGitProcess, which detaches git from a hidden console only when set. Only credential fill can trigger an interactive prompt, so only the url and certificate fills set mayRequireAuth. Credential approve and reject store or erase an already-obtained credential and never prompt, so they keep the default. Every other git command GVFS runs is local and keeps the default, so the per-child conhost cost stays avoided for every non-authenticating call. Log hasVisibleConsoleWindow from TryGetCredential so a credential fill that does not surface a prompt stays diagnosable from telemetry. Guard the native console probe so a binding failure keeps the previous behavior instead of aborting the fill. Add a certificate-fill test for the mayRequireAuth wiring. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 7 to 7. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v7...043fb46) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…tions/actions/upload-artifact-7 Bump actions/upload-artifact from 7 to 7
…artbeat Add IsSendingAnonymousRequests to VFS.Heartbeat
…og it on failure Address review feedback on the credential-prompt console-detach change. - Probe CurrentProcessHasVisibleConsoleWindow once per credential-fill call and thread the value through InvokeGitAgainstDotGitFolder, InvokeGitImpl, and GetGitProcess so the logged value and the CreateNoWindow decision are the same probe. GetGitProcess treats a supplied value as authoritative and only probes when none is passed. - Log hasVisibleConsoleWindow on the credential-fill failure path (including the timeout a hidden, unanswered prompt produces), not only on success. The success-path metadata and the certificate-fill success and failure paths log the same value. - Remove the misnamed intermediate local in GetGitProcess; compute CreateNoWindow directly so the && short-circuits the probe when auth cannot be required. - Restore the original pre-command-hook comment wording. - MockPlatform gains ThrowOnConsoleProbe to simulate a native probe that cannot run; add tests for the safe fallback, the caller-supplied authoritative value, and console-visibility telemetry on the URL and certificate credential-fill failure paths. The telemetry tests assert the logged value, not just the key. - MockGitProcess records each git invocation (command, mayRequireAuth, and hasVisibleConsoleWindow) instead of keeping only the last value per command, so the wiring assertions cannot be fooled if a command runs twice with different flags. The failure-telemetry tests also assert the probed console-visibility value threads down to the git invocation. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
…indow Fix regression causing credential prompts to hide behind other windows more often
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
Tyrie Vella (tyrielv)
enabled auto-merge
October 8, 2026 17:18
Nathan Baird (ShiningMassXAcc)
approved these changes
Oct 8, 2026
Nathan Baird (ShiningMassXAcc)
left a comment
Nathan Baird (ShiningMassXAcc)
left a comment
Member
There was a problem hiding this comment.
Reviewed this for merge fidelity rather than re-reviewing the eight upstream PRs.
Worth flagging that this one is not a pure auto-merge: three files conflicted and carry hand-written resolution — AGENTS.md, GVFS.Common/Git/GitProcess.cs, and GVFS.UnitTests/Mock/Git/MockGitProcess.cs. I re-ran the merge with git merge-tree and diffed its output against the commit: those three files are the only ones that differ, so no cleanly auto-merged file was hand-touched.
The resolution looks correct:
- The interesting collision is master's
mayRequireAuth/hasVisibleConsoleWindowplumbing (#2127) against vnext'sInvokeGitAgainstDotGitFolderOrOutsideEnlistment. Both survived intact — the new parameters are threaded through every overload and actually reachprocessInfo.CreateNoWindowinGetGitProcess, and the credential paths still passmayRequireAuth: true. No parameters accepted and then silently dropped, which is the usual way this kind of resolution goes wrong. MockGitProcess.InvocationsRunwas folded intoInitialize(), so it is non-null on both constructor paths, including vnext's(gitBinPath, workingDirectoryRoot)overload.- The
AGENTS.mdresolution keeps vnext's fuller stale-payload warning and still absorbs master's "publish all four managed payload projects" line; the prerequisite block reads coherently. - No conflict markers anywhere in the merged tree.
- CI green including functional tests on x64 and arm64.
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
3 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:
actions/upload-artifact(Bump actions/upload-artifact from 7 to 7 #2128)Merge upstream/master into vnext with a merge commit. This includes master tip #2127.