From 5cfb8dfe3751b1cc6a4e6e2e9953f40855a22ad7 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Thu, 9 Jul 2026 14:36:39 -0700 Subject: [PATCH] Stream large -z git output instead of buffering it against the cap Building on the git-output bounding change, this adds an opt-in path that removes the truncation exposure for the two commands whose full result is correctness-critical by streaming their output instead of buffering it. DiffCachedNameStatus (diff --cached --name-status -z) feeds every staged path into ModifiedPaths; StatusPorcelain (status --porcelain -z) drives the sparse dirty check. Both use -z (NUL-delimited), so line-based streaming cannot chunk them - git delivers the whole blob as a single line. Add a NUL-delimited streaming mode to InvokeGitImpl: a new parseStdOutToken callback reads stdout synchronously and splits on NUL, invoking the callback once per record as it arrives. stderr stays async (BeginErrorReadLine), so the synchronous stdout read cannot deadlock. Only one record is held in memory, so an arbitrarily large result streams without buffering, truncation, or OOM. Both commands expose a streaming overload and a buffered overload. The callers choose at runtime from the gvfs.stream-git-status-output config key, which defaults to false (off) per the feature-flag convention: by default they use the bounded-buffer path and its OutputTruncated fail-safes (the proven behavior), and streaming is enabled only when the rollout infrastructure turns the flag on. Add an optional streaming watchdog gated by gvfs.git-status-stream-timeout-seconds (default -1 = infinite/disabled): when set, a timer kills the git process tree if the synchronous read does not finish in time and the result reports a timeout. The default is infinite so a legitimately long status on a very large working tree is never killed. The watchdog disarms under processLock once the read completes, so a late callback cannot report a false timeout or kill a reused process. Hardening from self-review: the streaming read kills the git child if a callback throws (no orphaned process); AddStagedFilesToModifiedPaths fails on an unpaired trailing status token rather than acting on an incomplete list; MockGitProcess feeds output through the production tokenizer so the test double cannot drift. Tests: - ReadStdOutTokens: NUL splitting, empty input, embedded empty records, a lone NUL, a trailing record without a NUL, and a record spanning the 8KB buffer. - Streaming and buffered overloads of DiffCachedNameStatus/StatusPorcelain. - GetNextGitPath (buffered fallback) parsing; PathCoveredBySparseFolders unchanged. Assisted-by: Claude Opus 4.8 Signed-off-by: Tyrie Vella Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- GVFS/GVFS.Common/GVFSConstants.cs | 14 + GVFS/GVFS.Common/Git/GitProcess.cs | 292 ++++++++++++++++-- GVFS/GVFS.UnitTests/Git/GitProcessTests.cs | 132 ++++++++ .../GVFS.UnitTests/Mock/Git/MockGitProcess.cs | 19 +- .../FileSystemCallbacks.cs | 138 ++++++--- GVFS/GVFS/CommandLine/SparseVerb.cs | 85 ++++- 6 files changed, 604 insertions(+), 76 deletions(-) diff --git a/GVFS/GVFS.Common/GVFSConstants.cs b/GVFS/GVFS.Common/GVFSConstants.cs index 3dd946bff5..652dfadbd2 100644 --- a/GVFS/GVFS.Common/GVFSConstants.cs +++ b/GVFS/GVFS.Common/GVFSConstants.cs @@ -56,6 +56,20 @@ public static class GitConfig public const string MountProgress = GVFSPrefix + "mount-progress"; public const bool MountProgressDefault = false; + /* Opt-in switch for NUL-delimited streaming of large "-z" git status/diff output. + * Default false: use the bounded-buffer path with its truncation fail-safes (the proven + * behavior). Set true to stream instead, which processes an arbitrarily large result + * without buffering it. Off by default per the feature-flag convention so the rollout + * infrastructure can enable streaming gradually. */ + public const string StreamGitStatusOutput = GVFSPrefix + "stream-git-status-output"; + public const bool StreamGitStatusOutputDefault = false; + + /* Optional watchdog for the streaming status/diff read: kill the git process if it does + * not finish within this many seconds. Default -1 (infinite / disabled) so a legitimately + * long status on a very large working tree is never killed; operators can opt in. */ + public const string GitStatusStreamTimeoutSeconds = GVFSPrefix + "git-status-stream-timeout-seconds"; + public const int GitStatusStreamTimeoutSecondsDefault = -1; + public const string MaxHttpConnectionsConfig = GVFSPrefix + "max-http-connections"; public const string PrefetchUseIdx = GVFSPrefix + "prefetch-use-idx"; diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs index 03cc27b417..859b647380 100644 --- a/GVFS/GVFS.Common/Git/GitProcess.cs +++ b/GVFS/GVFS.Common/Git/GitProcess.cs @@ -525,6 +525,55 @@ public bool TryGetFromConfig(string settingName, bool forceOutsideEnlistment, ou return false; } + /// + /// Reads a boolean git-config value, returning when the setting is + /// unset or unreadable. Uses git's boolean semantics (true/yes/on/1 => true; false/no/off/0/empty + /// => false). + /// + public virtual bool GetConfigBoolOrDefault(string settingName, bool defaultValue) + { + if (this.TryGetFromConfig(settingName, forceOutsideEnlistment: false, out string value) && value != null) + { + switch (value.Trim().ToLowerInvariant()) + { + case "true": + case "yes": + case "on": + case "1": + return true; + case "false": + case "no": + case "off": + case "0": + return false; + } + } + + // Unset, unreadable, or an unrecognized value falls back to the (safe) default. + return defaultValue; + } + + /// + /// Reads an integer git-config value, returning when the setting is + /// unset or unparseable. + /// + public virtual int GetConfigIntOrDefault(string settingName, int defaultValue) + { + try + { + ConfigResult result = this.GetFromConfig(settingName, forceOutsideEnlistment: false); + if (result.TryParseAsInt(defaultValue, int.MinValue, out int value, out string _)) + { + return value; + } + } + catch + { + } + + return defaultValue; + } + public ConfigResult GetOriginUrl() { /* Disable precommand hook because this config call is used during mounting process @@ -568,16 +617,39 @@ public Result Status(bool allowObjectDownloads, bool useStatusCache, bool showUn return this.InvokeGitInWorkingDirectoryRoot(command, useReadObjectHook: allowObjectDownloads); } + /// + /// Buffers the entire "git status" porcelain -z output and returns it on . + /// Bounded by the stdout capture cap; check before acting on + /// the result. This is the fallback used when streaming is disabled via + /// . + /// public Result StatusPorcelain() { - string command = "status -uall --porcelain -z"; - return this.InvokeGitInWorkingDirectoryRoot(command, useReadObjectHook: false); + return this.InvokeGitInWorkingDirectoryRoot(StatusPorcelainCommand, useReadObjectHook: false); } /// - /// Returns staged file changes (index vs HEAD) as null-separated pairs of - /// status and path: "A\0path1\0M\0path2\0D\0path3\0". - /// Status codes: A=added, M=modified, D=deleted, R=renamed, C=copied. + /// Streams "git status" output in porcelain -z form, delivering each NUL-terminated record to + /// as it is read. This avoids buffering the entire status + /// output, which can be large in a big working tree. + /// + /// Receives each NUL-terminated record as it is read. + /// + /// Watchdog timeout in milliseconds, or -1 () for + /// no bound. If positive and the read does not finish in time, the git process is killed and the + /// result reports failure. + /// + public Result StatusPorcelain(Action parseStdOutToken, int timeoutMs = -1) + { + return this.InvokeGitInWorkingDirectoryRoot(StatusPorcelainCommand, useReadObjectHook: false, parseStdOutToken: parseStdOutToken, timeoutMs: timeoutMs); + } + + /// + /// Buffers staged file changes (index vs HEAD) as NUL-separated records and returns them on + /// in the form "A\0path1\0M\0path2\0...". Bounded by the stdout + /// capture cap; check before acting on the result. This is + /// the fallback used when streaming is disabled via + /// . /// /// Inline pathspecs to scope the diff, or null for all. /// @@ -589,6 +661,40 @@ public Result StatusPorcelain() /// separated by NUL instead of newline (--pathspec-file-nul). /// public Result DiffCachedNameStatus(string[] pathspecs = null, string pathspecFromFile = null, bool pathspecFileNul = false) + { + string command = DiffCachedNameStatusCommand(pathspecs, pathspecFromFile, pathspecFileNul); + return this.InvokeGitInWorkingDirectoryRoot(command, useReadObjectHook: false); + } + + /// + /// Streams staged file changes (index vs HEAD) as NUL-separated records: each change is emitted + /// as two records, a status token ("A", "M", "D", ...) followed by a path token. The records are + /// delivered to as they are read, so an arbitrarily large + /// staged set is processed without buffering the whole list. + /// + /// Receives each NUL-terminated record (status, path, status, path, ...). + /// Inline pathspecs to scope the diff, or null for all. + /// + /// Path to a file containing additional pathspecs (one per line), forwarded + /// as --pathspec-from-file to git. Null if not used. + /// + /// + /// When true and pathspecFromFile is set, pathspec entries in the file are + /// separated by NUL instead of newline (--pathspec-file-nul). + /// + /// + /// Watchdog timeout in milliseconds, or -1 () for + /// no bound. + /// + public Result DiffCachedNameStatus(Action parseStdOutToken, string[] pathspecs = null, string pathspecFromFile = null, bool pathspecFileNul = false, int timeoutMs = -1) + { + string command = DiffCachedNameStatusCommand(pathspecs, pathspecFromFile, pathspecFileNul); + return this.InvokeGitInWorkingDirectoryRoot(command, useReadObjectHook: false, parseStdOutToken: parseStdOutToken, timeoutMs: timeoutMs); + } + + private const string StatusPorcelainCommand = "status -uall --porcelain -z"; + + private static string DiffCachedNameStatusCommand(string[] pathspecs, string pathspecFromFile, bool pathspecFileNul) { string command = "diff --cached --name-status -z --no-renames"; @@ -606,7 +712,7 @@ public Result DiffCachedNameStatus(string[] pathspecs = null, string pathspecFro command += " -- " + string.Join(" ", pathspecs.Select(p => QuoteGitPath(p))); } - return this.InvokeGitInWorkingDirectoryRoot(command, useReadObjectHook: false); + return command; } /// @@ -975,13 +1081,22 @@ protected virtual Result InvokeGitImpl( Action parseStdOutLine, int timeoutMs, string gitObjectsDirectory = null, - bool usePreCommandHook = true) + bool usePreCommandHook = true, + Action parseStdOutToken = null) { if (failedToSetEncoding && writeStdIn != null) { return new Result(string.Empty, "Attempting to use to stdin, but the process does not have the right input encodings set.", Result.GenericFailureCode); } + // NUL-delimited streaming reads stdout synchronously on this thread, so it cannot be combined + // with line streaming. A finite timeout is honored via a watchdog (see the streaming branch + // below) rather than the WaitForExit(timeoutMs) path used for buffered reads. + if (parseStdOutToken != null && parseStdOutLine != null) + { + throw new InvalidOperationException($"{nameof(parseStdOutToken)} cannot be combined with {nameof(parseStdOutLine)}."); + } + try { // From https://msdn.microsoft.com/en-us/library/system.diagnostics.process.standardoutput.aspx @@ -1004,20 +1119,26 @@ protected virtual Result InvokeGitImpl( errors.AppendLine(args.Data); } }; - this.executingProcess.OutputDataReceived += (sender, args) => + + // In NUL-delimited streaming mode we read stdout ourselves (below) rather than using + // the line-based async reader, so we do not subscribe OutputDataReceived. + if (parseStdOutToken == null) { - if (args.Data != null) + this.executingProcess.OutputDataReceived += (sender, args) => { - if (parseStdOutLine != null) + if (args.Data != null) { - parseStdOutLine(args.Data); - } - else - { - output.AppendLine(args.Data); + if (parseStdOutLine != null) + { + parseStdOutLine(args.Data); + } + else + { + output.AppendLine(args.Data); + } } - } - }; + }; + } lock (this.executionLock) { @@ -1046,14 +1167,100 @@ protected virtual Result InvokeGitImpl( writeStdIn?.Invoke(this.executingProcess.StandardInput); this.executingProcess.StandardInput.Close(); - this.executingProcess.BeginOutputReadLine(); + // Always drain stderr asynchronously so the child can never block writing to it. this.executingProcess.BeginErrorReadLine(); - if (!this.executingProcess.WaitForExit(timeoutMs)) + if (parseStdOutToken != null) + { + // Read stdout synchronously, splitting on NUL and handing each record to the + // callback as it arrives. Because stderr is drained asynchronously above, a + // synchronous stdout read cannot deadlock. Only a single record is held in + // memory at a time, so an arbitrarily large result (e.g. every staged file in + // a monorepo) is processed without buffering the whole thing. + // + // Optional watchdog: the synchronous read would otherwise block forever on a + // git that never closes stdout. When a finite timeout is configured, arm a + // timer that kills the process tree; the blocked Read then returns EOF and we + // surface a timeout. Default (timeoutMs == Timeout.Infinite) leaves streaming + // unbounded, matching the buffered path. + bool killedByTimeout = false; + bool readCompleted = false; + Timer watchdog = null; + if (timeoutMs != Timeout.Infinite) + { + watchdog = new Timer( + _ => + { + lock (this.processLock) + { + // Only kill if the read is still in progress. Guarding on + // readCompleted (set under the same lock once the read returns) + // prevents a late callback from reporting a false timeout or + // killing a subsequent process reused on this instance. + if (!readCompleted && this.executingProcess != null) + { + killedByTimeout = true; + GVFSPlatform.Instance.TryKillProcessTree(this.executingProcess.Id, out int _, out string _); + } + } + }, + state: null, + dueTime: timeoutMs, + period: Timeout.Infinite); + } + + try + { + ReadStdOutTokens(this.executingProcess.StandardOutput, parseStdOutToken); + } + catch + { + // The stdout read or a streaming callback threw. The child git process is + // still running; disposing the Process wrapper (the using block below) would + // not end the child, leaking it. Kill the process tree before letting the + // exception propagate. Do not set 'stopping' here (unlike + // TryKillRunningProcess): this instance may be reused for later git calls. + lock (this.processLock) + { + if (this.executingProcess != null) + { + GVFSPlatform.Instance.TryKillProcessTree(this.executingProcess.Id, out int _, out string _); + } + } + + throw; + } + finally + { + // Disarm the watchdog under the lock so an in-flight callback either ran + // before this or becomes a no-op, then dispose the timer. + lock (this.processLock) + { + readCompleted = true; + } + + watchdog?.Dispose(); + } + + // stdout is at EOF; block until the process fully exits so the async stderr + // reads complete before we read ExitCode/Errors. + this.executingProcess.WaitForExit(); + + if (killedByTimeout) + { + return new Result(string.Empty, "Operation timed out: " + errors.ToString(), Result.GenericFailureCode, outputTruncated: false, errorsTruncated: errors.Truncated); + } + } + else { - this.executingProcess.Kill(); + this.executingProcess.BeginOutputReadLine(); + + if (!this.executingProcess.WaitForExit(timeoutMs)) + { + this.executingProcess.Kill(); - return new Result(output.ToString(), "Operation timed out: " + errors.ToString(), Result.GenericFailureCode, output.Truncated, errors.Truncated); + return new Result(output.ToString(), "Operation timed out: " + errors.ToString(), Result.GenericFailureCode, output.Truncated, errors.Truncated); + } } } @@ -1075,6 +1282,42 @@ private static string GenerateCredentialVerbCommand(string verb) return $"-c {GitConfigSetting.CredentialUseHttpPath}=true credential {verb}"; } + /// + /// Reads a redirected stdout stream that is NUL-delimited (git's "-z" machine-readable format), + /// invoking once per NUL-terminated record as it is read. + /// Only a single record is accumulated at a time, so an arbitrarily large result is processed + /// without buffering the entire stream. + /// + internal static void ReadStdOutTokens(StreamReader reader, Action parseStdOutToken) + { + StringBuilder token = new StringBuilder(); + char[] buffer = new char[8192]; + int read; + + while ((read = reader.Read(buffer, 0, buffer.Length)) > 0) + { + for (int i = 0; i < read; i++) + { + if (buffer[i] == '\0') + { + parseStdOutToken(token.ToString()); + token.Clear(); + } + else + { + token.Append(buffer[i]); + } + } + } + + // git's -z output always terminates the final record with a NUL, so there should be nothing + // left here. Flush any trailing partial record defensively rather than dropping it. + if (token.Length > 0) + { + parseStdOutToken(token.ToString()); + } + } + private static string ParseValue(string contents, string prefix) { int startIndex = contents.IndexOf(prefix) + prefix.Length; @@ -1128,7 +1371,9 @@ private Result InvokeGitInWorkingDirectoryRoot( string command, bool useReadObjectHook, Action writeStdIn = null, - Action parseStdOutLine = null) + Action parseStdOutLine = null, + Action parseStdOutToken = null, + int timeoutMs = -1) { return this.InvokeGitImpl( command, @@ -1137,7 +1382,8 @@ private Result InvokeGitInWorkingDirectoryRoot( useReadObjectHook: useReadObjectHook, writeStdIn: writeStdIn, parseStdOutLine: parseStdOutLine, - timeoutMs: -1); + timeoutMs: timeoutMs, + parseStdOutToken: parseStdOutToken); } /// diff --git a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs index 8182f8dfb5..9136bcfaad 100644 --- a/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs +++ b/GVFS/GVFS.UnitTests/Git/GitProcessTests.cs @@ -1,14 +1,146 @@ using GVFS.Common.Git; using GVFS.Tests.Should; using GVFS.UnitTests.Mock.Common; +using GVFS.UnitTests.Mock.Git; using NUnit.Framework; +using System.Collections.Generic; using System.Diagnostics; +using System.IO; +using System.Text; namespace GVFS.UnitTests.Git { [TestFixture] public class GitProcessTests { + [TestCase] + public void ReadStdOutTokens_SplitsOnNul() + { + List tokens = ReadTokens("a.txt\0d/b.txt\0d/c.txt\0"); + tokens.ShouldMatchInOrder("a.txt", "d/b.txt", "d/c.txt"); + } + + [TestCase] + public void ReadStdOutTokens_EmptyInputYieldsNoTokens() + { + ReadTokens(string.Empty).Count.ShouldEqual(0); + } + + [TestCase] + public void ReadStdOutTokens_SingleNulYieldsOneEmptyRecord() + { + // A lone NUL is one zero-length record, not "no records". A caller pairing status/path + // records relies on this so its state machine does not silently swallow the separator. + List tokens = ReadTokens("\0"); + tokens.Count.ShouldEqual(1); + tokens[0].ShouldEqual(string.Empty); + } + + [TestCase] + public void ReadStdOutTokens_PreservesEmptyRecords() + { + // diff --name-status -z emits status and path as separate records; an empty record must + // still be delivered so a caller's status/path state machine stays aligned. + List tokens = ReadTokens("A\0path\0\0after-empty\0"); + tokens.ShouldMatchInOrder("A", "path", string.Empty, "after-empty"); + } + + [TestCase] + public void ReadStdOutTokens_FlushesTrailingRecordWithoutNul() + { + List tokens = ReadTokens("a.txt\0trailing"); + tokens.ShouldMatchInOrder("a.txt", "trailing"); + } + + [TestCase] + public void ReadStdOutTokens_ReassemblesRecordSpanningReadBoundary() + { + // A single record longer than the internal 8192-char read buffer must be reassembled across + // multiple reads rather than split. + string longPath = new string('x', 20000); + List tokens = ReadTokens("short\0" + longPath + "\0"); + + tokens.Count.ShouldEqual(2); + tokens[0].ShouldEqual("short"); + tokens[1].ShouldEqual(longPath); + } + + [TestCase] + public void DiffCachedNameStatus_StreamsRecordsAsTokens() + { + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + "diff --cached --name-status -z --no-renames", + () => new GitProcess.Result("A\0added.txt\0M\0modified.txt\0", string.Empty, GitProcess.Result.SuccessCode)); + + List tokens = new List(); + GitProcess.Result result = git.DiffCachedNameStatus(t => tokens.Add(t)); + + result.ExitCodeIsSuccess.ShouldBeTrue(); + tokens.ShouldMatchInOrder("A", "added.txt", "M", "modified.txt"); + + // Streaming mode delivers all data through the callback; Output is empty (production never + // subscribes OutputDataReceived when streaming), so callers cannot depend on Output here. + result.Output.ShouldEqual(string.Empty); + } + + [TestCase] + public void StatusPorcelain_StreamsRecordsAsTokens() + { + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + "status -uall --porcelain -z", + () => new GitProcess.Result("A added.txt\0 M modified.txt\0", string.Empty, GitProcess.Result.SuccessCode)); + + List tokens = new List(); + GitProcess.Result result = git.StatusPorcelain(t => tokens.Add(t)); + + result.ExitCodeIsSuccess.ShouldBeTrue(); + tokens.ShouldMatchInOrder("A added.txt", " M modified.txt"); + } + + [TestCase] + public void DiffCachedNameStatus_BufferedFallbackReturnsOutput() + { + // With streaming disabled (gvfs.stream-git-status-output=false) callers use the buffered + // overload, which returns the whole -z blob on Result.Output for the caller to split. + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + "diff --cached --name-status -z --no-renames", + () => new GitProcess.Result("A\0added.txt\0M\0modified.txt\0", string.Empty, GitProcess.Result.SuccessCode)); + + GitProcess.Result result = git.DiffCachedNameStatus(); + + result.ExitCodeIsSuccess.ShouldBeTrue(); + result.Output.ShouldEqual("A\0added.txt\0M\0modified.txt\0"); + } + + [TestCase] + public void StatusPorcelain_BufferedFallbackReturnsOutput() + { + MockGitProcess git = new MockGitProcess(); + git.SetExpectedCommandResult( + "status -uall --porcelain -z", + () => new GitProcess.Result("A added.txt\0 M modified.txt\0", string.Empty, GitProcess.Result.SuccessCode)); + + GitProcess.Result result = git.StatusPorcelain(); + + result.ExitCodeIsSuccess.ShouldBeTrue(); + result.Output.ShouldEqual("A added.txt\0 M modified.txt\0"); + } + + private static List ReadTokens(string content) + { + List tokens = new List(); + using (MemoryStream stream = new MemoryStream(Encoding.UTF8.GetBytes(content))) + using (StreamReader reader = new StreamReader(stream, Encoding.UTF8)) + { + GitProcess.ReadStdOutTokens(reader, token => tokens.Add(token)); + } + + return tokens; + } + [TestCase] public void BoundedGitOutputBuffer_KeepsShortOutput() { diff --git a/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs b/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs index c9095cc2c8..cba158ec29 100644 --- a/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs +++ b/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs @@ -84,7 +84,8 @@ protected override Result InvokeGitImpl( Action parseStdOutLine, int timeoutMs, string gitObjectsDirectory = null, - bool usePrecommandHook = true) + bool usePrecommandHook = true, + Action parseStdOutToken = null) { this.CommandsRun.Add(command); @@ -122,6 +123,22 @@ protected override Result InvokeGitImpl( } /* Future: result.Output should be set to null in this case */ } + + if (parseStdOutToken != null && !string.IsNullOrEmpty(result.Output)) + { + // Feed the mock output through the real production tokenizer so the test double cannot + // drift from ReadStdOutTokens' actual semantics (empty records, trailing-fragment flush). + using (MemoryStream stream = new MemoryStream(Encoding.UTF8.GetBytes(result.Output))) + using (StreamReader reader = new StreamReader(stream, Encoding.UTF8)) + { + GitProcess.ReadStdOutTokens(reader, parseStdOutToken); + } + + // In streaming mode production never subscribes OutputDataReceived, so Result.Output is + // empty; mirror that here so callers cannot rely on Output being populated after streaming. + result = new Result(string.Empty, result.Errors, result.ExitCode, result.OutputTruncated, result.ErrorsTruncated); + } + return result; } diff --git a/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs b/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs index 91e627dd23..cade665da3 100644 --- a/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs +++ b/GVFS/GVFS.Virtualization/FileSystemCallbacks.cs @@ -435,54 +435,118 @@ public bool AddStagedFilesToModifiedPaths(string messageBody, out int addedCount } } - // Query all staged files in one call using --name-status -z. - // Output format: "A\0path1\0M\0path2\0D\0path3\0" - GitProcess.Result result = gitProcess.DiffCachedNameStatus(pathspecs, pathspecFromFile, pathspecFileNul); - - if (result.OutputTruncated) + // Query all staged files in one call using --name-status -z. Records arrive in pairs: a + // status token ("A", "M", "D", ...) followed by a path token. By default we stream the + // records so we never buffer the entire (potentially huge) staged file list in memory; the + // gvfs.stream-git-status-output=false kill switch restores the bounded-capture path with its + // truncation fail-safe. + List addedFilePaths = new List(); + int added = 0; + + // Record a single (status, path) pair into ModifiedPaths / the hydration list. Shared by the + // streaming and buffered paths so both behave identically per record. + Action handleRecord = (status, gitPath) => { - // The staged-file list exceeded the capture buffer. Acting on a partial list would leave - // some staged files out of ModifiedPaths (skip-worktree not cleared, stale placeholders), - // which is worse than failing. Fail safe and let the caller retry. - EventMetadata metadata = new EventMetadata(); - metadata.Add("ExitCode", result.ExitCode); - this.context.Tracer.RelatedError( - metadata, - nameof(this.AddStagedFilesToModifiedPaths) + ": git diff --cached output was truncated; refusing to update ModifiedPaths from a partial staged-file list"); - return false; - } + if (string.IsNullOrEmpty(gitPath)) + { + return; + } - if (result.ExitCodeIsSuccess && !string.IsNullOrEmpty(result.Output)) - { - string[] parts = result.Output.Split(new[] { '\0' }, StringSplitOptions.RemoveEmptyEntries); - List addedFilePaths = new List(); + string platformPath = gitPath.Replace(GVFSConstants.GitPathSeparator, Path.DirectorySeparatorChar); + if (this.modifiedPaths.TryAdd(platformPath, isFolder: false, isRetryable: out _)) + { + added++; + } - // Parts alternate: status, path, status, path, ... - for (int i = 0; i + 1 < parts.Length; i += 2) + // Added files (in index but not in HEAD) are ProjFS placeholders that + // would vanish when the projection reverts to HEAD. Collect them for + // hydration below. + if (status.StartsWith("A")) { - string status = parts[i]; - string gitPath = parts[i + 1]; + addedFilePaths.Add(gitPath); + } + }; - if (string.IsNullOrEmpty(gitPath)) - { - continue; - } + bool streamOutput = gitProcess.GetConfigBoolOrDefault( + GVFSConstants.GitConfig.StreamGitStatusOutput, + GVFSConstants.GitConfig.StreamGitStatusOutputDefault); - string platformPath = gitPath.Replace(GVFSConstants.GitPathSeparator, Path.DirectorySeparatorChar); - if (this.modifiedPaths.TryAdd(platformPath, isFolder: false, isRetryable: out _)) + GitProcess.Result result; + if (streamOutput) + { + int seconds = gitProcess.GetConfigIntOrDefault( + GVFSConstants.GitConfig.GitStatusStreamTimeoutSeconds, + GVFSConstants.GitConfig.GitStatusStreamTimeoutSecondsDefault); + int timeoutMs = (seconds > 0 && seconds <= int.MaxValue / 1000) ? seconds * 1000 : -1; + + string pendingStatus = null; + result = gitProcess.DiffCachedNameStatus( + token => { - addedCount++; - } + if (pendingStatus == null) + { + pendingStatus = token; + return; + } + + string status = pendingStatus; + pendingStatus = null; + handleRecord(status, token); + }, + pathspecs, + pathspecFromFile, + pathspecFileNul, + timeoutMs); + + addedCount = added; + + if (result.ExitCodeIsSuccess && pendingStatus != null) + { + // The -z stream ended on a status token with no matching path, so the staged-file + // list is incomplete (e.g. git was killed mid-write). Acting on a partial list would + // leave staged files out of ModifiedPaths, so fail and let the caller retry rather + // than silently dropping the last entry. + EventMetadata incompleteMetadata = new EventMetadata(); + incompleteMetadata.Add("ExitCode", result.ExitCode); + this.context.Tracer.RelatedError( + incompleteMetadata, + nameof(this.AddStagedFilesToModifiedPaths) + ": git diff --cached output ended on an unpaired status token; refusing to act on an incomplete staged-file list"); + return false; + } + } + else + { + result = gitProcess.DiffCachedNameStatus(pathspecs, pathspecFromFile, pathspecFileNul); - // Added files (in index but not in HEAD) are ProjFS placeholders that - // would vanish when the projection reverts to HEAD. Collect them for - // hydration below. - if (status.StartsWith("A")) + if (result.OutputTruncated) + { + // The staged-file list exceeded the capture buffer. Acting on a partial list would + // leave some staged files out of ModifiedPaths (skip-worktree not cleared, stale + // placeholders), which is worse than failing. Fail safe and let the caller retry. + EventMetadata truncatedMetadata = new EventMetadata(); + truncatedMetadata.Add("ExitCode", result.ExitCode); + this.context.Tracer.RelatedError( + truncatedMetadata, + nameof(this.AddStagedFilesToModifiedPaths) + ": git diff --cached output was truncated; refusing to update ModifiedPaths from a partial staged-file list"); + return false; + } + + if (result.ExitCodeIsSuccess && !string.IsNullOrEmpty(result.Output)) + { + string[] parts = result.Output.Split(new[] { '\0' }, StringSplitOptions.RemoveEmptyEntries); + + // Parts alternate: status, path, status, path, ... + for (int i = 0; i + 1 < parts.Length; i += 2) { - addedFilePaths.Add(gitPath); + handleRecord(parts[i], parts[i + 1]); } } + addedCount = added; + } + + if (result.ExitCodeIsSuccess) + { // Write added files from the git object store to disk as full files // so they persist across projection changes. Batched into as few git // process invocations as possible. @@ -494,7 +558,7 @@ public bool AddStagedFilesToModifiedPaths(string messageBody, out int addedCount } } } - else if (!result.ExitCodeIsSuccess) + else { EventMetadata metadata = new EventMetadata(); metadata.Add("ExitCode", result.ExitCode); diff --git a/GVFS/GVFS/CommandLine/SparseVerb.cs b/GVFS/GVFS/CommandLine/SparseVerb.cs index 334e4d1ca4..d1cc76de9b 100644 --- a/GVFS/GVFS/CommandLine/SparseVerb.cs +++ b/GVFS/GVFS/CommandLine/SparseVerb.cs @@ -18,8 +18,8 @@ public class SparseVerb : GVFSVerb.ForExistingEnlistment { private const string SparseVerbName = "sparse"; private const string FolderListSeparator = ";"; - private const char StatusPathSeparatorToken = '\0'; private const char StatusRenameToken = 'R'; + private const char StatusPathSeparatorToken = '\0'; private const string PruneOptionName = "prune"; private enum SetDirectoryTimeResult @@ -628,29 +628,84 @@ private void ForceProjectionChange(ITracer tracer, GVFSEnlistment enlistment) private void CheckGitStatus(ITracer tracer, GVFSEnlistment enlistment, HashSet sparseFolders) { GitProcess.Result statusResult = null; - HashSet dirtyPathsNotInSparseSet = null; + HashSet dirtyPathsNotInSparseSet = new HashSet(); if (!this.ShowStatusWhileRunning( () => { + dirtyPathsNotInSparseSet.Clear(); GitProcess git = new GitProcess(enlistment); - statusResult = git.StatusPorcelain(); - if (statusResult.ExitCodeIsFailure) + + bool streamOutput = git.GetConfigBoolOrDefault( + GVFSConstants.GitConfig.StreamGitStatusOutput, + GVFSConstants.GitConfig.StreamGitStatusOutputDefault); + + if (streamOutput) { - return false; + int seconds = git.GetConfigIntOrDefault( + GVFSConstants.GitConfig.GitStatusStreamTimeoutSeconds, + GVFSConstants.GitConfig.GitStatusStreamTimeoutSecondsDefault); + int timeoutMs = (seconds > 0 && seconds <= int.MaxValue / 1000) ? seconds * 1000 : -1; + + // Stream porcelain -z records so we never buffer the whole status output. Each entry + // is a primary "XY " token; a rename adds a second token for the original path. + bool expectingRenameOrigin = false; + statusResult = git.StatusPorcelain( + token => + { + string gitPath; + if (expectingRenameOrigin) + { + expectingRenameOrigin = false; + gitPath = token; + } + else + { + if (token.Length < 3) + { + return; + } + + // Two status chars (XY) then a space, then the path. + expectingRenameOrigin = token[0] == StatusRenameToken || token[1] == StatusRenameToken; + gitPath = token.Substring(3); + } + + if (!PathCoveredBySparseFolders(gitPath, sparseFolders)) + { + dirtyPathsNotInSparseSet.Add(gitPath); + } + }, + timeoutMs); + + if (statusResult.ExitCodeIsFailure) + { + return false; + } } - - if (statusResult.OutputTruncated) + else { - // git status output exceeded the capture buffer. A partial status could omit - // dirty paths and let sparse proceed over uncommitted changes (data loss), so - // treat truncation as "cannot verify clean" and abort. - tracer.RelatedError( - new EventMetadata(), - "git status output was truncated; aborting sparse to avoid acting on an incomplete status"); - return false; + // Buffered fallback (gvfs.stream-git-status-output=false): capture the whole status + // output, refuse to act on a truncated result, then parse it. + statusResult = git.StatusPorcelain(); + if (statusResult.ExitCodeIsFailure) + { + return false; + } + + if (statusResult.OutputTruncated) + { + // git status output exceeded the capture buffer. A partial status could omit + // dirty paths and let sparse proceed over uncommitted changes (data loss), so + // treat truncation as "cannot verify clean" and abort. + tracer.RelatedError( + new EventMetadata(), + "git status output was truncated; aborting sparse to avoid acting on an incomplete status"); + return false; + } + + dirtyPathsNotInSparseSet.UnionWith(this.GetPathsNotCoveredBySparseFolders(statusResult.Output, sparseFolders)); } - dirtyPathsNotInSparseSet = this.GetPathsNotCoveredBySparseFolders(statusResult.Output, sparseFolders); return dirtyPathsNotInSparseSet.Count == 0; }, "Running git status",