Repository navigation
Restore Arcade's Build.proj and remove Tools.proj - #7491
Merged
Merged
Conversation
ViktorHofer
added a commit
to dotnet/msbuild
that referenced
this pull request
Jun 30, 2026
…metaproject MSB4057 in parallel builds (#14223) Unblocks dotnet/dotnet#7491 ### Summary Fixes a bug where building a solution that requests a non-default target (e.g. `Pack`) fails with `MSB4057: The target "Pack" does not exist in the project` in **parallel / multithreaded** builds, while the same build succeeds serially. This was observed building the `dotnet/dotnet` VMR (Arcade builds the solution in two phases with different `__BuildPhase` global properties; the `SolutionBuild` phase requests `Build;Pack`). ### Root cause `BuildRequestConfiguration.RequestedTargets` was a get-only auto-property that was **never serialized** in `Translate`. In parallel/MT builds the configuration is round-tripped through translation (even when the work appears to run in-proc on node 1), so the deserialized configuration reset `RequestedTargets` to an empty collection. `SolutionProjectGenerator` only emits user-requested targets (such as `Pack`) into the generated `.slnx.metaproj` when they are present in `config.RequestedTargets`. With an empty set, `Pack` was omitted from the metaproject and the build failed with `MSB4057`. Serial builds never serialized the configuration, so they were unaffected. ### Fix Back `RequestedTargets` with a `List<string>` field and translate it in `BuildRequestConfiguration.Translate`. ### Validation Reproduced end-to-end against the `dotnet/dotnet` VMR (`symreader`). A/B with patched MSBuild deployed into the VMR SDK: | Serialization | targets reaching `SolutionBuild` generation | Result | | --- | --- | --- | | disabled (before) | `[]` | metaproject lacks `Pack` -> **MSB4057** | | enabled (this fix) | `[Build;Pack]` | metaproject contains `Pack` -> **build succeeds** | Added regression test `TestTranslationPreservesRequestedTargets`; full `*TestTranslation*` suite passes (56/56, net10.0 + net472). --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ViktorHofer
force-pushed
the
RestoreToolsetBuildProj
branch
2 times, most recently
from
October 27, 2026 09:27
9591696 to
c9016bb
Compare
Pass the /restore flag to the msbuild process invocation so that the toolset Build.proj gets restored. With that, the separate Tools.proj restore invocation isn't necessary and with that, the toolset restore happens immediately.
ViktorHofer
force-pushed
the
RestoreToolsetBuildProj
branch
from
October 27, 2026 09:43
c9016bb to
5a4aade
Compare
ViktorHofer
marked this pull request as ready for review
October 27, 2026 09:44
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The shared helpers introduce redundant restores for standalone SDK tasks on both platforms.
1 open issue
What changed in this PR
This PR moves Arcade toolset restoration into the initial MSBuild invocation, replacing the separate Tools.proj restore step used by the shared build infrastructure.
Changes:
- Add
/restoreto the shell and PowerShell MSBuild helpers when restore is enabled. - Move tool restoration and repository extension imports into Build.proj, and delete Tools.proj.
- Remove an obsolete comment referencing Tools.proj.
| File | Description |
|---|---|
| src/arcade/src/Microsoft.DotNet.Arcade.Sdk/toolset/VisualStudio.BuildIbcTrainingSettings.proj | Removes an outdated restore comment. |
| src/arcade/src/Microsoft.DotNet.Arcade.Sdk/toolset/Tools.proj | Deletes the separate tool restore project. |
| src/arcade/src/Microsoft.DotNet.Arcade.Sdk/toolset/Build.proj | Incorporates tool restoration and prepares inputs during evaluation. |
| src/arcade/eng/common/tools.sh | Adds conditional /restore to shell MSBuild invocations. |
| src/arcade/eng/common/tools.ps1 | Adds conditional /restore to PowerShell MSBuild invocations. |
Review effort: Balanced
The shared MSBuild helper now passes /restore when restore is enabled, so the separate Build 'Restore' invocation restored the task project twice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c9618a59-702c-46d4-aba3-18db69e722b6
…olset restore aspnetcore's eng/Tools.props relied on Tools.proj being invoked with a global ExcludeRestorePackageImports=true so that RemoveProperties on the second MSBuild call produced a distinct global property set and forced a re-evaluation of GenerateFiles.csproj after its restore. Neither MSBuild's /restore nor NuGet sets that property on the project being restored (NuGet only adds it on its inner restore graph walk), so use an explicit _GenerateFilesPass discriminator instead. Also update two comments that referenced the now-deleted Tools.proj. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05660a81-cc90-44af-892c-f8b0651ce9f9
…ore Tools.props
BaseIntermediateOutputPath is only set as a global property by
eng/common/sdk-task.{ps1,sh}, and only on the toolset <taskName>.proj files
(Publish.proj, Sign.proj, ...), none of which import eng/Tools.props.
eng/common/build.{ps1,sh}, which invokes the toolset Build.proj that does import
it, never passes the property, so RemoveProperties had nothing to remove.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 05660a81-cc90-44af-892c-f8b0651ce9f9
GenerateDirectoryBuildFiles runs AfterTargets=Restore, which is now inside MSBuild's /restore submission. That submission carries MSBuildIsRestoring=true as a global property, so Microsoft.Common.props defaults ImportProjectExtensionProps to false for both MSBuild task calls and never imports obj/GenerateFiles.csproj.nuget.g.props. The GenerateFileFromTemplate UsingTask was therefore unresolved (MSB4036 on Windows, missing generated Directory.Build.props downstream on Linux). Set ImportProjectExtensionProps explicitly on both calls, as Microsoft.Common.props instructs. The differing values also keep the two global property sets distinct, which is what forces the second call to re-evaluate the project. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05660a81-cc90-44af-892c-f8b0651ce9f9
ImportProjectExtensionProps is a general-purpose knob; removing MSBuildIsRestoring from the second invocation states the intent directly - that pass is not part of the restore - and lets Microsoft.Common.props apply its normal defaults. Verified on a clean obj directory both with MSBuild's /restore switch and with a bare /t:Restore (pre-PR arcade flow): the second invocation imports the generated obj/GenerateFiles.csproj.nuget.g.props in both modes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05660a81-cc90-44af-892c-f8b0651ce9f9
'restore=$_tmp_restore=' left $restore as 'true=' instead of 'true'. This was harmless until the shared MSBuild helper in eng/common/tools.sh started adding MSBuild's /restore switch based on $restore. With the typo the comparison never matched, so aspnetcore's Linux builds ran without /restore: the Restore target on the toolset Build.proj never ran, eng/Tools.props' GenerateDirectoryBuildFiles target (AfterTargets=Restore) never fired, and every project failed to evaluate on the missing artifacts/bin/GenerateFiles/Directory.Build.props. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05660a81-cc90-44af-892c-f8b0651ce9f9
Which global property disables the package imports for the second GenerateFiles invocation depends on the Arcade version: - With this PR's toolset, MSBuild's /restore switch sets MSBuildIsRestoring, which makes Microsoft.Common.props default ImportProjectExtensionProps to false and skip importing obj\GenerateFiles.csproj.nuget.g.props entirely. - With the pinned toolset that standalone aspnetcore still uses, Build.proj restores Tools.proj with ExcludeRestorePackageImports=true. NuGet conditions every group in nuget.g.props on that property, so the ImportGroup pulling in Microsoft.DotNet.Build.Tasks.Templating.props is dropped. Either way GenerateFileFromTemplate can't be resolved, so remove both. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05660a81-cc90-44af-892c-f8b0651ce9f9
…dFiles This reverts fd99185. The property is never set when this target runs: - In the VMR every repo except arcade and source-build-assets builds against the live src/arcade, whose toolset no longer sets ExcludeRestorePackageImports anywhere (Tools.proj and _RestoreToolsProps are gone with this PR). - MSBuild's /restore switch deliberately does not inject it either, see the note in XMake.cs ExecuteRestore and dotnet/msbuild#14274. - Standalone aspnetcore can't see a mismatched state: the backflow PR carries this eng/Tools.props change and the new Arcade SDK from the same VMR build. MSBuildIsRestoring is the only global property that suppresses the package imports here, so removing just that one is sufficient. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05660a81-cc90-44af-892c-f8b0651ce9f9
BootstrapArcadeRepos (arcade and source-build-assets) build with the root eng/common scripts instead of src/arcade/eng/common. In the SB_CentOSStream10_Offline_CurrentSourceBuiltSdk leg those repos consume the Arcade SDK produced by the preceding source build, i.e. the one from this PR, while still running the unmodified root scripts. That pairing is broken: the new Build.proj only declares the toolset projects as ProjectReference when MSBuildRestoreSessionId is set, which only happens under MSBuild's /restore switch. Without the switch AfterSolutionBuild.proj was never restored, so Microsoft.DotNet.SourceBuild.Tasks' props were not imported and TrackPrebuiltUsage.targets failed with MSB4036 on WritePackageUsageData. The four files are byte-identical to src/arcade/eng/common on main, so this is a straight copy of the same change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 05660a81-cc90-44af-892c-f8b0651ce9f9
akoeplinger
approved these changes
Oct 28, 2026
This was referenced Oct 29, 2026
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.
Pass the /restore flag to the msbuild process invocation so that the toolset Build.proj gets restored. With that, the separate Tools.proj restore invocation isn't necessary and with that, the toolset restore happens immediately.
For reviewers: All the changes in Build.proj are due to the Tools.proj content being moved there.