Merge post-trim and assembly modification pipelines#11058
Open
Merge post-trim and assembly modification pipelines#11058
Conversation
Move _AfterILLinkAdditionalSteps from the outer build into the inner (per-RID) build using AfterTargets="ILLink". This ensures AssemblyModifierPipeline runs on trimmed IL assemblies BEFORE CreateReadyToRunImages/IlcCompile compiles them to native code, preventing assembly modifications from overwriting R2R/AOT native code with pure IL. Add _CopySidecarXmlToAssemblyPaths target to copy .jlo.xml and .typemap.xml sidecar files from linked/ to wherever assemblies end up after R2R/AOT (e.g. R2R/, publish/), so outer-build consumers (_GenerateJavaStubs, GenerateTypeMappings) can find them. Handles: NativeAOT duplicate assemblies (KeepDuplicates="false"), R2R composite assemblies (empty sidecar files), assemblies not in ManagedAssemblyToLink (Touch AlwaysCreate), single-RID vs multi-RID path differences, and framework vs user assembly classification without NuGetPackageId (filter by known framework assembly names).
In NativeAOT builds, the project's own assembly is not in @(ManagedAssemblyToLink) — ILLink passes it as a TrimmerRootAssembly. This caused AssemblyModifierPipeline to skip it, producing no JCW for MainActivity and failing with XA0103. Add the root assembly explicitly to _AfterILLinkAssemblies using Exclude (not KeepDuplicates) to avoid duplicates. KeepDuplicates compares items including metadata, so a bare Include would be considered distinct from an existing item with rich metadata from @(ManagedAssemblyToLink), causing GetPerArchAssemblies() to throw a duplicate key error in CoreCLR builds. Exclude compares by ItemSpec only, correctly deduplicating in both scenarios. Also set HasMonoAndroidReference=true on the root assembly so IsAndroidAssembly() returns true and FindJavaObjectsStep scans it.
KeepDuplicates="false" compares items INCLUDING metadata, so when NativeAOT builds produce duplicate @(ManagedAssemblyToLink) entries for the same assembly (e.g. Java.Interop.dll) with different metadata, the transformed items survive deduplication and cause GetPerArchAssemblies() to throw "duplicate key" errors. Replace KeepDuplicates with the RemoveDuplicates task, which deduplicates by ItemSpec only, ignoring metadata differences.
… late When RuntimeIdentifier is set after path evaluation (e.g. via MockPrimaryCpuAbi.targets), IntermediateOutputPath does not contain the RID. The target now detects this and appends the RID explicitly to find sidecar XML files in the correct linked/ directory.
…nputs
Two related incrementality fixes for trimmed Android builds:
1. Add Inputs/Outputs to _AfterILLinkAdditionalSteps so it skips on no-change
rebuilds. Without this, SaveChangedAssemblyStep always updates assembly
timestamps, cascading through _GenerateJavaStubs -> _CompileJava ->
_CompileToDalvik -> _BuildApkEmbed -> _Sign even when nothing changed.
2. Add _AndroidFixManagedAssemblyToLinkForILLink target in NativeAOT.targets
to restore the user assembly to @(ManagedAssemblyToLink) after the standard
NativeAOT SDK strips it. Android re-enables ILLink (RunILLink=true) and
needs the user assembly in @(ManagedAssemblyToLink) so that:
- ILLink processes it via AssemblyPaths (not just as a root name)
- _RunILLink's Inputs correctly detect user assembly changes, ensuring
ILLink re-runs and $(_LinkSemaphore) updates on incremental builds
Also exclude the user assembly from _AndroidILLinkAssemblies -> IlcReference
to prevent double-counting (it's already in IlcCompileInput).
Fixes: CheckTimestamps, CheckAppBundle, BasicApplicationRepetitiveReleaseBuild,
JavacTaskDoesNotRunOnSecondBuild, GenerateJavaStubsAndAssembly,
BuildAotApplication, BuildIncrementingClassName(NativeAOT)
When switching RuntimeIdentifier between builds without cleaning (e.g. ChangeSupportedAbis test switches from android-arm64 to android-x64), the inner build may run for the old RID while the outer build expects the new RID's linked/ directory. The Touch task fails with MSB3371 because it cannot create files in a non-existent directory. Add MakeDir before Touch to ensure the linked/ directory exists. Fixes: ChangeSupportedAbis(NativeAOT)
The second build in CheckSignApk only changes Strings.xml (an Android resource), so assemblies are unchanged and ILLink correctly skips. With the _AfterILLinkAdditionalSteps incrementality fix, IL3053 warnings no longer appear on no-code-change rebuilds. Update the test to expect no warnings for all runtimes on the second build.
Filter _PostTrimmingAssembly by PostprocessAssembly=true metadata instead of all .dll files from @(ResolvedFileToPublish). The unfiltered list included NuGet satellite assemblies (e.g. Microsoft.Maui.Controls.resources.dll) that point to shared paths in the NuGet cache. When parallel inner builds for multiple RIDs both opened these shared files with ReadWrite access, it caused an IOException file-locking conflict on Windows (XAPTP7000 wrapping XA0009). PostprocessAssembly=true is the same metadata gate that ILLink uses to select which assemblies to process, so this gives us exactly the trimmed assemblies from $(IntermediateLinkDir) without coupling to ILLink internals.
The %(PostprocessAssembly) batching condition on the ItemGroup Include triggers MSB4096 when any item in @(ResolvedFileToPublish) lacks the PostprocessAssembly metadata (e.g. .runtimeconfig.json). Switch to the WithMetadataValue() item function which safely skips items without the metadata.
…itionalSteps Both targets previously used AfterTargets="ILLink", making their relative execution order depend on import order. Change _AfterILLinkAdditionalSteps to AfterTargets="_PostTrimmingPipeline" so the post-trimming pipeline (strip embedded resources, add keep-alives) always runs first. This prevents the assembly modifier pipeline from reading stale MVIDs.
…' into mergePipelines
Eliminate the separate PostTrimmingPipeline MSBuild task by moving all its steps (CheckForObsoletePreserveAttribute, StripEmbeddedLibraries, PostTrimmingAddKeepAlives, RemoveResourceDesigner) into AssemblyModifierPipeline. This gives trimmed builds a single unified pipeline after ILLink instead of two sequential ones. Introduce a virtual BuildAssemblyModificationSteps method that separates the variable steps (trimmed vs non-trimmed) from the common steps (FindJavaObjects, SaveChangedAssembly, FindTypeMapObjects). LinkAssembliesNoShrink now overrides BuildAssemblyModificationSteps instead of BuildPipeline.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR unifies the post-ILLink assembly modification flow for trimmed builds by folding the former PostTrimmingPipeline work into AssemblyModifierPipeline, aiming to ensure all assembly mutations happen once (and before R2R/AOT compilation) while keeping non-trimmed builds on LinkAssembliesNoShrink.
Changes:
- Run
_AfterILLinkAdditionalStepsin the inner (per-RID) build afterILLink, and execute all post-trim + common assembly modification steps viaAssemblyModifierPipeline. - Remove the standalone
PostTrimmingPipelinetask and its MSBuild wiring. - Add an outer-build target to copy
.jlo.xml/.typemap.xmlsidecars fromlinked/to the final assembly locations (e.g.,R2R/,publish/), and adjust tests accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Moves post-ILLink pipeline to inner build; adds sidecar-copying target for outer build |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj | Updates linker-step source includes/comments to reflect the new unified pipeline |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs | Updates incremental-build warning expectations after pipeline changes |
| src/Xamarin.Android.Build.Tasks/Tasks/PostTrimmingPipeline.cs | Removes the now-obsolete MSBuild task |
| src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs | Switches override to the new BuildAssemblyModificationSteps hook |
| src/Xamarin.Android.Build.Tasks/Tasks/AssemblyModifierPipeline.cs | Adds post-trimming steps + new virtual split between variable and common steps |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets | Removes PostTrimmingPipeline UsingTask/target wiring |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Ensures user assembly remains in @(ManagedAssemblyToLink) for Android’s ILLink flow |
| src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/CheckForObsoletePreserveAttributeStep.cs | Updates documentation comment to reflect the new pipeline name |
Comment on lines
+1626
to
+1629
| <!-- Create empty sidecar files for R2R composite assemblies (zero-length = WasScanned=false) --> | ||
| <Touch | ||
| Condition=" '@(_R2RCompositeAssemblies)' != '' " | ||
| Files="@(_R2RCompositeAssemblies->'%(RootDir)%(Directory)%(Filename).jlo.xml');@(_R2RCompositeAssemblies->'%(RootDir)%(Directory)%(Filename).typemap.xml')" |
There was a problem hiding this comment.
_CopySidecarXmlToAssemblyPaths uses Touch AlwaysCreate="true" for the R2R composite sidecar files. Touch updates timestamps even when the files already exist, which will force downstream targets that consume these sidecars to re-run on every build and break incremental builds. Consider only creating these files when they don't already exist (e.g., build an item group of missing sidecars and touch only those), or add appropriate Inputs/Outputs so the target can be skipped when nothing changed.
Suggested change
| <!-- Create empty sidecar files for R2R composite assemblies (zero-length = WasScanned=false) --> | |
| <Touch | |
| Condition=" '@(_R2RCompositeAssemblies)' != '' " | |
| Files="@(_R2RCompositeAssemblies->'%(RootDir)%(Directory)%(Filename).jlo.xml');@(_R2RCompositeAssemblies->'%(RootDir)%(Directory)%(Filename).typemap.xml')" | |
| <!-- Compute sidecar file paths for R2R composite assemblies --> | |
| <ItemGroup Condition=" '@(_R2RCompositeAssemblies)' != '' "> | |
| <_R2RCompositeSidecarFiles Include="@(_R2RCompositeAssemblies->'%(RootDir)%(Directory)%(Filename).jlo.xml')" /> | |
| <_R2RCompositeSidecarFiles Include="@(_R2RCompositeAssemblies->'%(RootDir)%(Directory)%(Filename).typemap.xml')" /> | |
| </ItemGroup> | |
| <!-- Filter to sidecar files that do not yet exist --> | |
| <ItemGroup Condition=" '@(_R2RCompositeSidecarFiles)' != '' "> | |
| <_MissingR2RCompositeSidecarFiles Include="@(_R2RCompositeSidecarFiles)" Condition=" !Exists('%(FullPath)') " /> | |
| </ItemGroup> | |
| <!-- Create empty sidecar files for missing R2R composite assemblies (zero-length = WasScanned=false) --> | |
| <Touch | |
| Condition=" '@(_MissingR2RCompositeSidecarFiles)' != '' " | |
| Files="@(_MissingR2RCompositeSidecarFiles)" |
Comment on lines
+1662
to
+1668
| <MakeDir | ||
| Directories="@(_SidecarXmlCopySource->'%(RootDir)%(Directory)')" | ||
| Condition=" '@(_SidecarXmlCopySource)' != '' " /> | ||
| <Touch | ||
| Files="@(_SidecarXmlCopySource)" | ||
| AlwaysCreate="true" | ||
| Condition=" '@(_SidecarXmlCopySource)' != '' " /> |
There was a problem hiding this comment.
_CopySidecarXmlToAssemblyPaths touches all computed source sidecar XML paths with AlwaysCreate="true". This will update the timestamps in linked/ on every outer build, making Copy SkipUnchangedFiles="true" ineffective (sources always look newer) and causing repeated copies + downstream rebuilds. Instead, only create/touch the subset of sidecar files that are missing (e.g., include only Condition="!Exists('%(Identity)')" items) so existing timestamps remain stable and incrementality is preserved.
| Condition="'$(PublishTrimmed)' == 'true'" | ||
| Inputs="$(_AndroidLinkFlag)" | ||
| AfterTargets="ILLink" | ||
| Condition=" '$(PublishTrimmed)' == 'true' and '$(_ComputeFilesToPublishForRuntimeIdentifiers)' == 'true' " |
There was a problem hiding this comment.
_AfterILLinkAdditionalSteps now uses Inputs="$(_LinkSemaphore)" but does not guard against configurations where ILLink is disabled or the semaphore file is absent. If $(_LinkSemaphore) doesn't exist, MSBuild will treat the input as missing and this target can run every build (and/or run when it shouldn't). Consider adding Exists('$(_LinkSemaphore)') (and possibly '$(RunILLink)' != 'false') to the target Condition, similar to _TouchAndroidLinkFlag in Microsoft.Android.Sdk.TypeMap.LlvmIr.targets.
Suggested change
| Condition=" '$(PublishTrimmed)' == 'true' and '$(_ComputeFilesToPublishForRuntimeIdentifiers)' == 'true' " | |
| Condition=" '$(PublishTrimmed)' == 'true' and '$(_ComputeFilesToPublishForRuntimeIdentifiers)' == 'true' and '$(RunILLink)' != 'false' and Exists('$(_LinkSemaphore)') " |
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.
Merge PostTrimmingPipeline into AssemblyModifierPipeline
Eliminate the separate PostTrimmingPipeline MSBuild task by moving all its
steps (CheckForObsoletePreserveAttribute, StripEmbeddedLibraries,
PostTrimmingAddKeepAlives, RemoveResourceDesigner) into AssemblyModifierPipeline.
This gives trimmed builds a single unified pipeline after ILLink instead of two
sequential ones.
Introduce a virtual BuildAssemblyModificationSteps method that separates the
variable steps (trimmed vs non-trimmed) from the common steps (FindJavaObjects,
SaveChangedAssembly, FindTypeMapObjects). LinkAssembliesNoShrink now overrides
BuildAssemblyModificationSteps instead of BuildPipeline.
Requires #10891