Fix RecursiveDir metadata loss in -mt builds (TaskHost boundary only)#13318
Open
JanProvaznik wants to merge 5 commits intodotnet:mainfrom
Open
Fix RecursiveDir metadata loss in -mt builds (TaskHost boundary only)#13318JanProvaznik wants to merge 5 commits intodotnet:mainfrom
JanProvaznik wants to merge 5 commits intodotnet:mainfrom
Conversation
When using MSBuild's -mt mode (/maxcpucount), tasks without [MSBuildMultiThreadableTask] are routed to out-of-process TaskHost sidecars. Items passed to these tasks are serialized via TaskParameterTaskItem, which calls CloneCustomMetadataEscaped() — copying only custom metadata. RecursiveDir is a built-in metadata that is non-derivable (requires _includeBeforeWildcardExpansionEscaped), so it's lost during this serialization, returning empty string on the TaskHost side. This causes dotnet pack /mt to flatten directory structures in NuGet packages (e.g., build/wix/bundle/bundle.wxs becomes build/bundle.wxs). Fix: Explicitly copy RecursiveDir to custom metadata in the TaskParameterTaskItem constructor before serialization, so it survives the cross-process boundary. Unlike the previous attempt (PR dotnet#13142, reverted in PR dotnet#13245), this fix ONLY touches the TaskHost serialization boundary (TaskParameterTaskItem). It does NOT change GatherTaskItemOutputs or ProjectItemInstance, which was what caused the NuGet/sfxproj double-append regression. Items that lose RecursiveDir at the MSBuild task callback boundary (e.g., sfxproj targets returning items via TargetOutputs) are unaffected — they already have RecursiveDir=empty before reaching TaskParameterTaskItem. Fixes dotnet#13140 Related: dotnet#3121 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
|
the pipeline succeeded packing the arcade project but then failed on #13188 |
Member
Author
|
I'd say ready to review, but rerunning pipeline to verify that something does not break down the line. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes loss of %(RecursiveDir) when items are serialized across the TaskHost process boundary in multithreaded (-mt//maxcpucount) builds, preventing directory flattening issues (e.g., dotnet pack /mt producing NU5118 duplicates).
Changes:
- Preserve
RecursiveDirby explicitly copying it intoTaskParameterTaskItem’s custom escaped metadata during TaskHost-bound serialization. - Add a unit test intended to validate
RecursiveDirround-trip behavior throughTaskParameterserialization.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Shared/TaskParameter.cs | Copies built-in RecursiveDir into the serialized custom metadata bag so it survives TaskHost cross-process translation in -mt builds. |
| src/Shared/UnitTests/TaskParameter_Tests.cs | Adds a test for RecursiveDir surviving TaskParameter serialization/deserialization. |
The previous test used Microsoft.Build.Utilities.TaskItem.SetMetadata which stores RecursiveDir as custom metadata (in the _metadata dictionary). Since CloneCustomMetadataEscaped() already copies all custom metadata, the test passed trivially without exercising the actual fix. The new test constructs a ProjectItemInstance.TaskItem with a wildcard pattern (includeBeforeWildcardExpansionEscaped != includeEscaped) so RecursiveDir is computed as built-in metadata via BuiltInMetadata.GetRecursiveDirValue(). This correctly exercises the bug: CloneCustomMetadataEscaped() does NOT include built-in metadata, so without the fix RecursiveDir is lost during TaskHost serialization. Verified: test FAILs when the fix is commented out, PASSes with the fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JanProvaznik
force-pushed
the
fix-recursivedir-mt-only
branch
from
963d813 to
e79960b
Compare
Remove arcade, runtime, aspnetcore, sdk, efcore, and razor from the MSBuildMTExcludedReposOverride list. These repos were excluded because NuGet PackTask loses RecursiveDir metadata when routed through TaskHost in -mt mode. The TaskParameterTaskItem fix preserves RecursiveDir, unblocking these repos. Repos without known RecursiveDir pack issues (roslyn, winforms, wpf, source-build-reference-packages) remain excluded until individually validated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
JanProvaznik
force-pushed
the
fix-recursivedir-mt-only
branch
from
0d83c27 to
4651dad
Compare
The previous commit removed too many repos from the -mt exclusion list. Only arcade has been validated to work with -mt in the RecursiveDir fix. Other repos remain excluded until validated in CI source-build Stage 2. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
When using MSBuild's
-mtmode (/maxcpucount),%(RecursiveDir)metadata is lost when items cross the TaskHost process boundary. This causesdotnet pack /mtto flatten directory structures in NuGet packages.Example:
build/wix/bundle/bundle.wxsbecomesbuild/bundle.wxs, causing NU5118 duplicate file errors.Fixes #13140
Related: #3121
Root Cause
In
-mtmode, tasks without[MSBuildMultiThreadableTask](like NuGet'sPackTask) are routed to out-of-process TaskHost sidecars viaTaskRouter.NeedsTaskHostInMultiThreadedMode(). Items passed to these tasks are serialized throughTaskParameterTaskItem, which callsCloneCustomMetadataEscaped()— copying only custom metadata.RecursiveDiris a built-in metadata that is explicitly non-derivable (IsDerivableItemSpecModifier("RecursiveDir")returnsfalse). Unlike other built-in metadata (FullPath,FileName, etc.), it cannot be recomputed from the item spec alone — it requires the original wildcard pattern (_includeBeforeWildcardExpansionEscaped). On the TaskHost side,GetMetadata("RecursiveDir")falls through to the custom metadata dictionary lookup, finds nothing, and returns empty string.Without
-mt, items are passed as rawProjectItemInstance.TaskItemobjects (no serialization), soGetMetadata("RecursiveDir")computes the value on-demand from_includeBeforeWildcardExpansionEscapedviaBuiltInMetadata.GetRecursiveDirValue().Fix
Explicitly copy
RecursiveDirto the custom metadata dictionary inTaskParameterTaskItem's constructor (afterCloneCustomMetadataEscaped), so it survives cross-process serialization. This aligns out-of-proc (-mt) behavior with in-proc behavior.Why this is narrower than the reverted PR #13142
PR #13142 was reverted (#13245) because it fixed two separate serialization boundaries:
TaskParameterTaskItem)GatherTaskItemOutputs)<MSBuild>taskTargetOutputsincludeBeforeWildcardExpansionEscapedin newProjectItemInstanceThe Boundary 2 change caused NuGet pack to double-append RecursiveDir to PackagePath for items like sfxproj analyzers where
RecursiveDirwas already baked intoTargetPathduring evaluation (e.g.,TargetPath="analyzers/%(RecursiveDir)").This PR only fixes Boundary 1 (
TaskParameter.cs). Boundary 2 is untouched, so sfxproj items that lose RecursiveDir at the MSBuild callback boundary still haveRecursiveDir=""by the time they reachTaskParameterTaskItem— no doubling occurs.Safety matrix
buildbuild/wix/bundle/✅""(lost in TaskHost)buildbuild/❌ flattenedwix/bundle/(in custom metadata)buildbuild/wix/bundle/✅""(lost at callback)analyzers/dotnet/cs/analyzers/dotnet/cs/✅""(lost at callback then TaskHost)analyzers/dotnet/cs/analyzers/dotnet/cs/✅""(lost at callback before TaskHost)analyzers/dotnet/cs/analyzers/dotnet/cs/✅Testing
RecursiveDirSurvivesTaskParameterSerializationunit test verifying round-trip serialization preserves RecursiveDirTaskParameter_Testspassdotnet pack /mton arcade'sMicrosoft.DotNet.Build.Tasks.Installerssucceeds with correct directory structure using bootstrapped msbuildFuture work
The underlying bug #3121 (RecursiveDir lost through MSBuild task callback boundary /
GatherTaskItemOutputs) remains unfixed. Fixing that requires a coordinated change with NuGet to add an idempotency check inPackTaskLogic.cs(don't append RecursiveDir if PackagePath already ends with it), since consumers like sfxproj bake RecursiveDir into TargetPath as a workaround.