From 4b2a462a7ff30bc5cbad7206fdf7d3b1fc6fe0ac Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 20:52:13 +0000 Subject: [PATCH 1/4] Initial plan From 333438e27196d384e4d15c40abe2d38b776d7087 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 21:01:04 +0000 Subject: [PATCH 2/4] Improve diagnostics for flaky BinaryLoggerShouldEmbedFilesViaTaskOutput Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/06996b62-1cc3-478e-b15d-b8a74804fa3f Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com> --- src/Build.UnitTests/BinaryLogger_Tests.cs | 51 ++++++++++++++++++++--- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/src/Build.UnitTests/BinaryLogger_Tests.cs b/src/Build.UnitTests/BinaryLogger_Tests.cs index e47fb19b1e2..aa451d50564 100644 --- a/src/Build.UnitTests/BinaryLogger_Tests.cs +++ b/src/Build.UnitTests/BinaryLogger_Tests.cs @@ -74,10 +74,12 @@ public class BinaryLoggerTests : IDisposable "; private readonly TestEnvironment _env; + private readonly ITestOutputHelper _output; private string _logFile; public BinaryLoggerTests(ITestOutputHelper output) { + _output = output; _env = TestEnvironment.Create(output); // this is needed to ensure the binary logger does not pollute the environment @@ -437,30 +439,67 @@ private void AssemblyLoadsDuringTaskRun(string additionalEventText) [Fact] public void BinaryLoggerShouldEmbedFilesViaTaskOutput() { + // Pin the process current directory to a stable, isolated transient folder so + // both the WriteLinesToFile task (which resolves the relative file path against + // the process CWD) and the binary logger's file-existence/embed code observe + // the same directory. Historically this test has been flaky (#9412, #13762) + // because relative ItemSpecs on the EmbedInBinlog item were resolved against + // a different CWD on the binary logger's background embedding thread. + TransientTestFolder workingFolder = _env.CreateFolder(createFolder: true); + _env.SetCurrentDirectory(workingFolder.Path); + + const string OutputFileName = "testtaskoutputfile.txt"; + string expectedOutputFilePath = Path.Combine(workingFolder.Path, OutputFileName); + using var buildManager = new BuildManager(); var binaryLogger = new BinaryLogger() { Parameters = $"LogFile={_logFile}", CollectProjectImports = BinaryLogger.ProjectImportsCollectionMode.ZipFile, }; - var testProject = @" + + // Capture every event the binary logger sees so we can include it in the + // failure message if the assertion below fires intermittently. + var mockLogger = new MockLogger(_output); + + var testProject = $@" - - + + "; - ObjectModelHelpers.BuildProjectExpectSuccess(testProject, binaryLogger); + ObjectModelHelpers.BuildProjectExpectSuccess(testProject, binaryLogger, mockLogger); + + // Sanity-check the output file produced by WriteLinesToFile actually exists + // on disk after the build. If it does not, the diagnostic information will + // make the failure mode obvious instead of just reporting an empty zip. + bool outputFileExists = File.Exists(expectedOutputFilePath); + var projectImportsZipPath = Path.ChangeExtension(_logFile, ".ProjectImports.zip"); using var fileStream = new FileStream(projectImportsZipPath, FileMode.Open); using var zipArchive = new ZipArchive(fileStream, ZipArchiveMode.Read); + // Snapshot the zip entries up-front so the assertion message can describe + // every entry (FullName + Length), not just whichever one tripped Shouldly. + var entries = zipArchive.Entries + .Select(e => $" Name='{e.Name}' FullName='{e.FullName}' Length={e.Length}") + .ToList(); + + string diagnostics = + $"Working directory: {workingFolder.Path}{Environment.NewLine}" + + $"Process CWD at assert time: {Directory.GetCurrentDirectory()}{Environment.NewLine}" + + $"Expected output file: {expectedOutputFilePath} (Exists={outputFileExists}){Environment.NewLine}" + + $"Binlog path: {_logFile}{Environment.NewLine}" + + $"ProjectImports.zip path: {projectImportsZipPath}{Environment.NewLine}" + + $"Embedded files ({entries.Count}):{Environment.NewLine}{string.Join(Environment.NewLine, entries)}{Environment.NewLine}" + + $"=== Build log ==={Environment.NewLine}{mockLogger.FullLog}"; + // Can't just compare `Name` because `ZipArchive` does not handle unix directory separators well // thus producing garbled fully qualified paths in the actual .ProjectImports.zip entries - zipArchive.Entries.ShouldContain(zE => zE.Name.EndsWith("testtaskoutputfile.txt"), - $"Embedded files: {string.Join(",", zipArchive.Entries)}"); + zipArchive.Entries.ShouldContain(zE => zE.Name.EndsWith(OutputFileName), diagnostics); } [RequiresSymbolicLinksFact] From 1f6f65d24d7ed095d1febbca0ba4c5fe086c309d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 13 May 2026 21:05:46 +0000 Subject: [PATCH 3/4] Resolve relative EmbedInBinlog paths on the calling thread to fix flake Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/06996b62-1cc3-478e-b15d-b8a74804fa3f Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com> --- .../BinaryLogger/ProjectImportsCollector.cs | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs index c42e54a26a8..54e081a5838 100644 --- a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs +++ b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs @@ -91,7 +91,7 @@ public ProjectImportsCollector( public void AddFile(string? filePath) { - AddFileHelper(filePath, AddFileCore); + AddFileHelper(filePath, AddFileCore, makePathAbsolute: true); } public void AddFileFromMemory( @@ -101,7 +101,7 @@ public void AddFileFromMemory( bool makePathAbsolute = true) { AddFileHelper(filePath, path => - AddFileFromMemoryCore(path, data, makePathAbsolute, entryCreationStamp)); + AddFileFromMemoryCore(path, data, makePathAbsolute, entryCreationStamp), makePathAbsolute); } public void AddFileFromMemory( @@ -110,15 +110,41 @@ public void AddFileFromMemory( DateTimeOffset? entryCreationStamp = null, bool makePathAbsolute = true) { - AddFileHelper(filePath, path => AddFileFromMemoryCore(path, data, makePathAbsolute, entryCreationStamp)); + AddFileHelper(filePath, path => AddFileFromMemoryCore(path, data, makePathAbsolute, entryCreationStamp), makePathAbsolute); } private void AddFileHelper( string? filePath, - Action addFileWorker) + Action addFileWorker, + bool makePathAbsolute) { if (filePath != null && _fileStream != null) { + // When the caller wants the path to be absolute, resolve it here on the + // calling thread. Otherwise, when _runOnBackground is true the file + // existence check / GetFullPath in the worker would run on a TPL background + // thread later, where the process current directory is no longer guaranteed + // to match the project directory in use when this event was raised. MSBuild + // restores the original CWD after the build completes (see + // BuildRequestConfiguration / InProcNode), which made relative ItemSpecs + // from `EmbedInBinlog` items silently fail to embed. + // + // The replay path intentionally passes makePathAbsolute=false because the + // input "path" is actually an archive entry name (with ':' stripped) that + // must not be resolved against the current directory. + if (makePathAbsolute && !Path.IsPathRooted(filePath)) + { + try + { + filePath = Path.GetFullPath(filePath); + } + catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) + { + // If we can't even canonicalize the path, fall through and let the + // worker report the error via the standard FileIOException path. + } + } + lock (_fileStream) { if (_runOnBackground) From 1e2d02b27ba7ffcd77919b3bdfcab7e043e068f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 15:20:02 +0000 Subject: [PATCH 4/4] Resolve EmbedInBinlog item paths to absolute on the engine side Agent-Logs-Url: https://github.com/dotnet/msbuild/sessions/95e19c9c-02f0-4a3f-be86-7840eef28d4f Co-authored-by: rainersigwald <3347530+rainersigwald@users.noreply.github.com> --- .../Components/Logging/LoggingContext.cs | 14 +++ .../Logging/ProjectLoggingContext.cs | 5 ++ .../Logging/TargetLoggingContext.cs | 3 + .../Components/Logging/TaskLoggingContext.cs | 3 + .../IntrinsicTasks/ItemGroupLoggingHelper.cs | 90 +++++++++++++++++++ .../BinaryLogger/ProjectImportsCollector.cs | 34 +------ 6 files changed, 119 insertions(+), 30 deletions(-) diff --git a/src/Build/BackEnd/Components/Logging/LoggingContext.cs b/src/Build/BackEnd/Components/Logging/LoggingContext.cs index f08d8845008..b7a372c973d 100644 --- a/src/Build/BackEnd/Components/Logging/LoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/LoggingContext.cs @@ -70,6 +70,20 @@ public LoggingContext(LoggingContext baseContext, BuildEventContext newEventCont /// internal IBuildEngineDataConsumer BuildEngineDataConsumer => this; + /// + /// The full path of the project this context is associated with, if any. Returns + /// for contexts that are not project-scoped (e.g. + /// ). + /// + /// + /// Used for resolving relative paths to absolute on the engine side before they + /// flow out to loggers, where neither the process current directory nor the + /// background thread CWD is a reliable base directory (especially in + /// multithreaded MSBuild execution; see + /// documentation/specs/multithreading/multithreaded-msbuild.md). + /// + internal virtual string? ProjectFullPath => null; + /// /// Retrieves the logging service /// diff --git a/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs b/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs index 6f467e25d9b..589e4e6a576 100644 --- a/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/ProjectLoggingContext.cs @@ -153,6 +153,11 @@ private ProjectLoggingContext( this.IsValid = true; } + /// + /// The full path of the project this context was created for. + /// + internal override string ProjectFullPath => _projectFullPath; + private static BuildEventContext CreateInitialContext( NodeLoggingContext nodeLoggingContext, int submissionId, diff --git a/src/Build/BackEnd/Components/Logging/TargetLoggingContext.cs b/src/Build/BackEnd/Components/Logging/TargetLoggingContext.cs index 1f44ab275de..120352aa197 100644 --- a/src/Build/BackEnd/Components/Logging/TargetLoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/TargetLoggingContext.cs @@ -69,6 +69,9 @@ internal ProjectLoggingContext ProjectLoggingContext } } + /// + internal override string ProjectFullPath => _projectLoggingContext?.ProjectFullPath; + /// /// Retrieves the target. /// diff --git a/src/Build/BackEnd/Components/Logging/TaskLoggingContext.cs b/src/Build/BackEnd/Components/Logging/TaskLoggingContext.cs index 43d668805e4..a2b09eb4566 100644 --- a/src/Build/BackEnd/Components/Logging/TaskLoggingContext.cs +++ b/src/Build/BackEnd/Components/Logging/TaskLoggingContext.cs @@ -101,6 +101,9 @@ internal TargetLoggingContext TargetLoggingContext } } + /// + internal override string ProjectFullPath => _targetLoggingContext?.ProjectFullPath; + /// /// Retrieves the task node. /// diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs index 23f3d96c1f6..00308dd50de 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupLoggingHelper.cs @@ -5,6 +5,7 @@ using System.Collections; using System.Collections.Generic; using System.Globalization; +using System.IO; #if FEATURE_APPDOMAIN using System.Runtime.Remoting; #endif @@ -263,6 +264,20 @@ internal static void LogTaskParameter( bool logItemMetadata, IElementLocation location = null) { + // For items destined for the binary logger via the EmbedInBinlog hint, resolve + // any relative item specs against the project's directory before the event + // leaves the engine. The logger consumes events on background threads where + // the process current directory is not a reliable base directory; this is + // especially true under multithreaded MSBuild execution where multiple + // projects may be running concurrently in the same process and the + // process-level CWD is not a per-project concept at all (see + // documentation/specs/multithreading/multithreaded-msbuild.md). + if (items != null + && string.Equals(itemType, ItemTypeNames.EmbedInBinlog, StringComparison.OrdinalIgnoreCase)) + { + items = MakeEmbedInBinlogItemSpecsAbsolute(items, loggingContext.ProjectFullPath); + } + var args = CreateTaskParameterEventArgs( loggingContext.BuildEventContext, messageKind, @@ -278,6 +293,81 @@ internal static void LogTaskParameter( loggingContext.LogBuildEvent(args); } + /// + /// Returns an item list equivalent to in which every relative + /// (or relative bare-string spec) has been resolved to + /// an absolute path against 's directory. The original + /// items are not mutated; a snapshot is allocated only when at least one rewrite is + /// required. + /// + private static IList MakeEmbedInBinlogItemSpecsAbsolute(IList items, string projectFullPath) + { + if (string.IsNullOrEmpty(projectFullPath)) + { + return items; + } + + string projectDirectory = Path.GetDirectoryName(projectFullPath); + if (string.IsNullOrEmpty(projectDirectory)) + { + return items; + } + + int count = items.Count; + object[] cloned = null; + + for (int i = 0; i < count; i++) + { + object item = items[i]; + string originalSpec = item switch + { + ITaskItem taskItem => taskItem.ItemSpec, + string s => s, + _ => null, + }; + + if (string.IsNullOrEmpty(originalSpec) || Path.IsPathRooted(originalSpec)) + { + if (cloned != null) + { + cloned[i] = item; + } + + continue; + } + + // Path.GetFullPath of an already-rooted path does not consult the process + // current directory, so this is safe in any execution mode. + string absoluteSpec = Path.GetFullPath(Path.Combine(projectDirectory, originalSpec)); + + if (cloned == null) + { + cloned = new object[count]; + for (int j = 0; j < i; j++) + { + cloned[j] = items[j]; + } + } + + cloned[i] = item is ITaskItem origTaskItem + ? new TaskItemData(absoluteSpec, CopyMetadata(origTaskItem)) + : (object)absoluteSpec; + } + + return cloned ?? items; + } + + private static IDictionary CopyMetadata(ITaskItem item) + { + var dictionary = new Dictionary(); + foreach (KeyValuePair kvp in item.EnumerateMetadata()) + { + dictionary[kvp.Key] = kvp.Value; + } + + return dictionary; + } + internal static TaskParameterEventArgs CreateTaskParameterEventArgs( BuildEventContext buildEventContext, TaskParameterMessageKind messageKind, diff --git a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs index 54e081a5838..c42e54a26a8 100644 --- a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs +++ b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs @@ -91,7 +91,7 @@ public ProjectImportsCollector( public void AddFile(string? filePath) { - AddFileHelper(filePath, AddFileCore, makePathAbsolute: true); + AddFileHelper(filePath, AddFileCore); } public void AddFileFromMemory( @@ -101,7 +101,7 @@ public void AddFileFromMemory( bool makePathAbsolute = true) { AddFileHelper(filePath, path => - AddFileFromMemoryCore(path, data, makePathAbsolute, entryCreationStamp), makePathAbsolute); + AddFileFromMemoryCore(path, data, makePathAbsolute, entryCreationStamp)); } public void AddFileFromMemory( @@ -110,41 +110,15 @@ public void AddFileFromMemory( DateTimeOffset? entryCreationStamp = null, bool makePathAbsolute = true) { - AddFileHelper(filePath, path => AddFileFromMemoryCore(path, data, makePathAbsolute, entryCreationStamp), makePathAbsolute); + AddFileHelper(filePath, path => AddFileFromMemoryCore(path, data, makePathAbsolute, entryCreationStamp)); } private void AddFileHelper( string? filePath, - Action addFileWorker, - bool makePathAbsolute) + Action addFileWorker) { if (filePath != null && _fileStream != null) { - // When the caller wants the path to be absolute, resolve it here on the - // calling thread. Otherwise, when _runOnBackground is true the file - // existence check / GetFullPath in the worker would run on a TPL background - // thread later, where the process current directory is no longer guaranteed - // to match the project directory in use when this event was raised. MSBuild - // restores the original CWD after the build completes (see - // BuildRequestConfiguration / InProcNode), which made relative ItemSpecs - // from `EmbedInBinlog` items silently fail to embed. - // - // The replay path intentionally passes makePathAbsolute=false because the - // input "path" is actually an archive entry name (with ':' stripped) that - // must not be resolved against the current directory. - if (makePathAbsolute && !Path.IsPathRooted(filePath)) - { - try - { - filePath = Path.GetFullPath(filePath); - } - catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) - { - // If we can't even canonicalize the path, fall through and let the - // worker report the error via the standard FileIOException path. - } - } - lock (_fileStream) { if (_runOnBackground)