Repository navigation
Expand optimized publish to conservative trimmed app scenarios - #56484
marcpopMSFT wants to merge 2 commits into
Conversation
Generate managed publish runtime configuration independently of Build, preserve build path and Clean contracts, and add rollout documentation and regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 2 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. |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The high-impact MSBuild publish-graph change spans incremental build and Clean semantics and still requires cross-platform CI validation.
Review effort: Balanced
Findings: None
What changed in this PR
Extends optimized publish to conservative self-contained trimmed app scenarios while preserving existing build, AOT, no-build, and clean behavior.
Changes:
- Adds eligibility and managed optimized-publish target flow.
- Adds integration coverage for outputs, routing, configuration, and cleaning.
- Documents eligibility, compatibility constraints, and opt-out guidance.
| File | Description |
|---|---|
test/Microsoft.NET.ILLink.Tests/GivenThatWeWantToPublishTrimmedAppsOptimally.cs |
Adds trimmed optimized-publish tests. |
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets |
Extracts shared runtime-configuration preparation. |
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets |
Implements eligibility, metadata generation, and cleanup tracking. |
documentation/general/publish-build-optimization.md |
Documents the expanded rollout and limitations. |
|
|
||
| </Target> | ||
|
|
||
| <Target Name="_PrepareRuntimeConfigurationFiles"> |
There was a problem hiding this comment.
copilot pulled this into a separate target as both build and optimized publish use it.
| $([MSBuild]::VersionGreaterThanOrEquals('$(_TargetFrameworkVersionWithoutV)', '11.0')) and | ||
| '$(Language)' == 'C#' and '$(_IsExecutable)' == 'true' and '$(SelfContained)' == 'true' and | ||
| '$(TargetPlatformIdentifier)' == '' and | ||
| ('$(RuntimeIdentifier)' == 'win-x64' or '$(RuntimeIdentifier)' == 'win-arm64' or |
There was a problem hiding this comment.
My copilot is super nervous about making this change have broader impact. It tried to eliminate any config it wasn't sure would work but that makes this conditional pretty complicated. It also might make things confusing for the user. I'm not sure we need to be this risk adverse.
Here's the breakdown of the conditional it made for me. I think I'm going to have it split it out so it's easier to read:
Preserve the rollout policy while separating publish mode, project shape, runtime, SDK exclusions, and integration exclusions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| <Output TaskParameter="Filtered" ItemName="_OptimizedPublishFileWrites" /> | ||
| </RemoveDuplicates> | ||
| <WriteLinesToFile File="$(IntermediateOutputPath)$(CleanFile)" | ||
| Lines="@(_OptimizedPublishFileWrites)" |
There was a problem hiding this comment.
I think this doesn't include intermediates produced by the build, so this breaks dotnet clean after an optimized publish (build outputs are left over).
| '$(RuntimeIdentifier)' == 'win-x64' or '$(RuntimeIdentifier)' == 'win-arm64' or | ||
| '$(RuntimeIdentifier)' == 'linux-x64' or '$(RuntimeIdentifier)' == 'linux-arm64' or | ||
| '$(RuntimeIdentifier)' == 'linux-musl-x64' or '$(RuntimeIdentifier)' == 'linux-musl-arm64' or | ||
| '$(RuntimeIdentifier)' == 'osx-x64' or '$(RuntimeIdentifier)' == 'osx-arm64'">true</_IsOptimizedTrimmedPublishRuntime> |
There was a problem hiding this comment.
Why do we need to filter eligibility so specifically? Should it be .NET11+ or .NET12+?
Overall I'd prefer to make the optimized publish the default more broadly and only exclude specific scenarios where it's known to be problematic. It's hard to tell from reading this which conditions are actually necessary to avoid an incorrect publish.
There was a problem hiding this comment.
Ah sorry, I missed your comment #56484 (comment). I agree, not sure we need to be so risk-averse.
| </PropertyGroup> | ||
| </Target> | ||
|
|
||
| <Target Name="_GenerateOptimizedPublishRuntimeConfigurationFile" |
There was a problem hiding this comment.
Why do we define a new target for this instead of scheduling the Build target that writes it on the unoptimized publish path? Is there a way to do this by reusing an existing Build target instead, perhaps with some small adjustments to that target?
🤖 Extend
UseOptimizedPublishbeyond Native AOT so eligible trimmed applications avoid a redundant root-project build deployment while still compiling. This is the first conservative .NET 12 rollout, not a general replacement for Build during publish.Approach
truecannot force unsupported scenarios.UseOptimizedPublish=false, the separate NoBuild route, explicit Build, and normal project-reference builds. Register the intermediate runtime configuration for Clean without priming Build's incremental-clean snapshot or enrolling the published deployment in its ownership.The TFM policy deliberately includes net11 projects using SDK 12; it is not net12-only. Build-hook integrations that need runnable
binoutput must opt out until adapted. The documentation describes this behavior change and OpenAPI/XML serializer coordination limitations.Validation
Related to #55911; this implements only the first rollout step.