Fix ProjectImports.zip regression from shared FileUtilities statics#13382
Merged
JanProvaznik merged 3 commits intodotnet:mainfrom Mar 16, 2026
Merged
Conversation
After FileUtilities moved from Shared to Framework (dotnet#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>
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>
JanProvaznik
force-pushed
the
fix/project-imports-zip-regression
branch
from
4e0c79d to
1d02dca
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a regression where ClearCacheDirectory() could delete the temporary *.ProjectImports.zip archive before the binary logger embeds it, due to FileUtilities statics becoming shared after #13364.
Changes:
- Store temporary ProjectImports archives under
FileUtilities.TempFileDirectoryinstead ofFileUtilities.GetCacheDirectory(), avoiding deletion during shutdown cache cleanup. - Make
ProjectImportsCollector.DeleteArchive()best-effort by swallowing IO-related exceptions. - Add a regression unit test ensuring
ClearCacheDirectory()does not break reading/embedding the imports archive.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Build/Logging/BinaryLogger/ProjectImportsCollector.cs | Moves temp archive location out of the cache directory and makes archive deletion resilient to IO failures. |
| src/Build.UnitTests/BinaryLogger_Tests.cs | Adds regression coverage to ensure ProjectImports archives survive ClearCacheDirectory(). |
You can also share your feedback on Copilot code review. Take the survey.
DustinCampbell
approved these changes
Mar 13, 2026
Member
DustinCampbell
left a comment
DustinCampbell
left a comment
There was a problem hiding this comment.
This looks good to me!
FWIW, this issue was pretty subtle and is a good example of why it's important to avoid static state in files compiled across binaries. For the future, it'd be good to audit the cache/temp path lifetimes.
(Note: Please wait for more legitimate MSBuild reviewers before taking my ✅.)
rainersigwald
approved these changes
Mar 13, 2026
Co-authored-by: Dustin Campbell <dustin@teamcampbell.org>
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
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.
Summary
Fixes the
ProjectImports.zipregression introduced by #13364.Problem
After #13364, all 9 VMR scenario tests fail across all platforms with:
Reported in: dotnet/dotnet#5433 (comment)
Root Cause
#13364 moved
FileUtilitiesfromsrc/Shared/(compiled into each assembly with independent statics) tosrc/Framework/(single shared assembly). Before,ClearCacheDirectory()in XMake.cs used MSBuild.exe's owncacheDirectorystatic — a different directory than the oneProjectImportsCollectorused. After, they share the same static, soClearCacheDirectory()destroys theProjectImports.ziparchive.Fix
ProjectImportsarchive inTempFileDirectory(parent, not wiped) instead ofGetCacheDirectory()(child, wiped byClearCacheDirectory()).DeleteArchive()resilient to IO errors.Open Question
Before #13364,
ClearCacheDirectory()at XMake.cs:1735 was a no-op for the real build cache. Now it actually cleans result cache files. This is new behavior that may need its own investigation.