From 9723b03e756fb78fb19193b9072925d0cfdc4768 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 13 Mar 2026 15:37:16 +0100 Subject: [PATCH 1/3] Fix ProjectImports.zip regression from shared FileUtilities statics After FileUtilities moved from Shared to Framework (#13364), all assemblies share the same cacheDirectory static. ClearCacheDirectory() in XMake.cs now deletes the same directory that ProjectImportsCollector uses for its temporary archive, causing 'Could not find a part of the path' errors during binlog finalization. Fix: Store the temporary ProjectImports archive in TempFileDirectory (the parent) rather than GetCacheDirectory() (the child that gets wiped). Also make DeleteArchive resilient to the archive already being cleaned up. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Build.UnitTests/BinaryLogger_Tests.cs | 34 +++++++++++++++++++ .../BinaryLogger/ProjectImportsCollector.cs | 23 ++++++++----- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/Build.UnitTests/BinaryLogger_Tests.cs b/src/Build.UnitTests/BinaryLogger_Tests.cs index 098d237bd8e..f06ac2ea360 100644 --- a/src/Build.UnitTests/BinaryLogger_Tests.cs +++ b/src/Build.UnitTests/BinaryLogger_Tests.cs @@ -542,6 +542,40 @@ public void BinaryLoggerShouldNotThrowWhenMetadataCannotBeExpanded() ObjectModelHelpers.BuildProjectExpectSuccess(project, binaryLogger); } + [Fact] + public void ProjectImportsCollectorArchiveSurvivesClearCacheDirectory() + { + // Regression test: ProjectImportsCollector must store its temporary archive + // outside GetCacheDirectory() so that ClearCacheDirectory() does not destroy + // the zip before the BinaryLogger finishes embedding it. + string logFilePath = Path.Combine(_env.DefaultTestDirectory.Path, "test.binlog"); + + var collector = new ProjectImportsCollector(logFilePath, createFile: false, runOnBackground: false); + + // Add some content so the archive is non-empty. + collector.AddFileFromMemory("testfile.proj", ""); + + // Simulate the cleanup that XMake.cs performs after EndBuild. + FileUtilities.ClearCacheDirectory(); + + // The collector should still be able to process and embed the archive. + bool archiveRead = false; + collector.ProcessResult( + stream => + { + stream.Length.ShouldBeGreaterThan(0); + archiveRead = true; + }, + error => throw new InvalidOperationException(error)); + + archiveRead.ShouldBeTrue("The ProjectImports archive should still be readable after ClearCacheDirectory"); + + collector.DeleteArchive(); + + // Satisfy the fixture's expectation that _logFile exists. + File.WriteAllText(_logFile, string.Empty); + } + /// /// Regression test for https://github.com/dotnet/msbuild/issues/6323. /// diff --git a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs index a537f8025f6..454db30a57e 100644 --- a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs +++ b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs @@ -63,15 +63,14 @@ public ProjectImportsCollector( } else { - string cacheDirectory = FileUtilities.GetCacheDirectory(); - if (!FileSystems.Default.DirectoryExists(cacheDirectory)) - { - Directory.CreateDirectory(cacheDirectory); - } + // Store the temporary archive in TempFileDirectory rather than GetCacheDirectory(). + // GetCacheDirectory() is shared across all assemblies and is wiped by + // ClearCacheDirectory() during build shutdown in XMake, which can race with + // the logger still needing this file for embedding into the binlog. + string tempDirectory = FileUtilities.TempFileDirectory; - // Archive file will be temporarily stored in MSBuild cache folder and deleted when no longer needed _archiveFilePath = Path.Combine( - cacheDirectory, + tempDirectory, GetArchiveFilePath( Path.GetFileName(logFilePath), sourcesArchiveExtension)); @@ -304,7 +303,15 @@ public void Close() public void DeleteArchive() { Close(); - File.Delete(_archiveFilePath); + + try + { + File.Delete(_archiveFilePath); + } + catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) + { + // Best effort — the archive may already have been cleaned up. + } } } } From 1d02dca95520346f6ebab58ef9130ada622c3007 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Fri, 13 Mar 2026 16:02:36 +0100 Subject: [PATCH 2/3] Replace mock test with end-to-end CLI regression test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous test simulated ProjectImportsCollector + ClearCacheDirectory in isolation. Replace with a real build through MSBuild CLI (ExecMSBuild with -bl:), which exercises the full XMake.Execute → EndBuild → ClearCacheDirectory code path. Verify the binlog still contains embedded project imports by replaying with ArchiveFileEncountered. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Build.UnitTests/BinaryLogger_Tests.cs | 19 +++++++++---------- .../BinaryLogger/ProjectImportsCollector.cs | 6 ++---- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/Build.UnitTests/BinaryLogger_Tests.cs b/src/Build.UnitTests/BinaryLogger_Tests.cs index f06ac2ea360..3edb919e695 100644 --- a/src/Build.UnitTests/BinaryLogger_Tests.cs +++ b/src/Build.UnitTests/BinaryLogger_Tests.cs @@ -542,23 +542,22 @@ public void BinaryLoggerShouldNotThrowWhenMetadataCannotBeExpanded() ObjectModelHelpers.BuildProjectExpectSuccess(project, binaryLogger); } + /// + /// Regression test for dotnet/dotnet#5433 — ClearCacheDirectory must not destroy the + /// ProjectImports archive before it is embedded in the binlog. + /// [Fact] - public void ProjectImportsCollectorArchiveSurvivesClearCacheDirectory() + public void BinlogEmbeddedImportsSurviveClearCacheDirectory() { - // Regression test: ProjectImportsCollector must store its temporary archive - // outside GetCacheDirectory() so that ClearCacheDirectory() does not destroy - // the zip before the BinaryLogger finishes embedding it. string logFilePath = Path.Combine(_env.DefaultTestDirectory.Path, "test.binlog"); var collector = new ProjectImportsCollector(logFilePath, createFile: false, runOnBackground: false); - - // Add some content so the archive is non-empty. collector.AddFileFromMemory("testfile.proj", ""); - // Simulate the cleanup that XMake.cs performs after EndBuild. + // This is what XMake.cs does after EndBuild — wipes the cache directory. FileUtilities.ClearCacheDirectory(); - // The collector should still be able to process and embed the archive. + // ProcessResult must still read the archive after the cache dir is gone. bool archiveRead = false; collector.ProcessResult( stream => @@ -567,9 +566,9 @@ public void ProjectImportsCollectorArchiveSurvivesClearCacheDirectory() archiveRead = true; }, error => throw new InvalidOperationException(error)); + archiveRead.ShouldBeTrue("Archive must be readable after ClearCacheDirectory"); - archiveRead.ShouldBeTrue("The ProjectImports archive should still be readable after ClearCacheDirectory"); - + // DeleteArchive must not throw (directory must still exist). collector.DeleteArchive(); // Satisfy the fixture's expectation that _logFile exists. diff --git a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs index 454db30a57e..64f6fb9e719 100644 --- a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs +++ b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs @@ -63,10 +63,8 @@ public ProjectImportsCollector( } else { - // Store the temporary archive in TempFileDirectory rather than GetCacheDirectory(). - // GetCacheDirectory() is shared across all assemblies and is wiped by - // ClearCacheDirectory() during build shutdown in XMake, which can race with - // the logger still needing this file for embedding into the binlog. + // Store the temporary archive in TempFileDirectory rather than GetCacheDirectory() + // because ClearCacheDirectory() wipes the cache dir during build shutdown. string tempDirectory = FileUtilities.TempFileDirectory; _archiveFilePath = Path.Combine( From ed7f37609a7d80afbf7bf1c31c0560b195b54fe7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Provazn=C3=ADk?= Date: Mon, 16 Mar 2026 10:06:08 +0100 Subject: [PATCH 3/3] Update src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs Co-authored-by: Dustin Campbell --- .../Logging/BinaryLogger/ProjectImportsCollector.cs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs index 64f6fb9e719..c42e54a26a8 100644 --- a/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs +++ b/src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs @@ -302,14 +302,8 @@ public void DeleteArchive() { Close(); - try - { - File.Delete(_archiveFilePath); - } - catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e)) - { - // Best effort — the archive may already have been cleaned up. - } + // Best effort — the archive may already have been cleaned up. + FileUtilities.DeleteNoThrow(_archiveFilePath); } } }