Fix XALNS7015: run FixAbstractMethodsStep before R2R in non-trimmed CoreCLR builds by simonrozsival · Pull Request #11027 · dotnet/android

simonrozsival · 2026-03-26T16:08:17Z

Summary

Rebased on top of #10891 + #11058 (branch: dev/sbomer/merge-pipelines).

In non-trimmed CoreCLR Release builds, PublishReadyToRun=true causes the inner build to compile IL assemblies to R2R images (via crossgen2) before_LinkAssembliesNoShrink runs. R2R assemblies have the ILONLY PE flag cleared, so when FixAbstractMethodsStep detects a missing abstract method and marks the assembly modified, SaveChangedAssemblyStep calls assembly.Write() via Mono.Cecil, which throws:

NotSupportedException: Writing mixed-mode assemblies is not supported

Changes

Fix: move `_LinkAssembliesNoShrink` to run before R2R compilation in the inner build

The previous _LinkAssembliesNoShrink target ran in the outer build using @(ResolvedAssemblies) and copied modified assemblies to an intermediate directory. This happened too late — the inner build had already produced R2R images by that point.

The fix restructures _LinkAssembliesNoShrink to run inside the inner build (BeforeTargets="_PrepareForReadyToRunCompilation"):

Xamarin.Android.Common.targets: Rewrite _LinkAssembliesNoShrink to run in the inner build when trimming is disabled. It operates on @(ResolvedFileToPublish) (available in the inner build) instead of @(ResolvedAssemblies) (outer build only). Assemblies are copied to an intermediate directory (linked-noshrink/) rather than modified in-place, since ResolvedFileToPublish items may point to shared locations (NuGet cache, runtime packs). After the task runs, ResolvedFileToPublish is updated so R2R and publish consume the modified copies.
Framework/user assembly classification: Uses the same known-name filter as the trimmed path (Mono.Android, Java.Interop, etc.) instead of treating all DLLs as user assemblies.
Sidecar XML handling: Extends _CopySidecarXmlToAssemblyPaths to handle both trimmed (linked/) and non-trimmed (android/linked-noshrink/) sidecar XML files using a $(_SidecarSubDir) property.
Microsoft.Android.Sdk.AssemblyResolution.targets: In _PrepareAssemblies, populate _ResolvedAssemblies etc. unconditionally from @(ResolvedAssemblies) (no longer redirecting through the intermediate directory for non-trimmed builds).

Tests

Remove Assert.Ignore workarounds in BuildTest.SimilarAndroidXAssemblyNames and LinkerTests.AndroidAddKeepAlives for the CoreCLR Release non-trimmed case — these scenarios now pass.

Depends on

PR Run AssemblyModifierPipeline before R2R #10891 — moves _AfterILLinkAdditionalSteps (trimmed path) to the inner build
PR Merge post-trim and assembly modification pipelines #11058 — merges PostTrimmingPipeline into AssemblyModifierPipeline

Copilot

Pull request overview

Fixes the CoreCLR Release (non-trimmed) PublishReadyToRun=true build failure where FixAbstractMethodsStep tries to rewrite an already-R2R’d assembly and Mono.Cecil throws NotSupportedException for mixed-mode output.

Changes:

Run PostTrimmingPipeline before crossgen2/ReadyToRun for both trimmed and non-trimmed PublishReadyToRun builds, enabling FixAbstractMethods only in the non-trimmed path.
Update PostTrimmingPipeline to optionally include FixAbstractMethodsStep and mark processed assemblies as Android/user assemblies in StepContext.
Re-enable previously ignored CoreCLR test cases and add a unit regression test for FixAbstractMethods behavior via PostTrimmingPipeline.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs	Adds a regression unit test validating `PostTrimmingPipeline` can inject a missing abstract-method stub.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs	Removes the CoreCLR non-trimmed ignore workaround now that the pipeline should run pre-R2R.
src/Xamarin.Android.Build.Tasks/Tasks/PostTrimmingPipeline.cs	Adds `FixAbstractMethods` support and updates `StepContext` flags so `FixAbstractMethodsStep` can run.
src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets	Extends `_PostTrimmingPipeline` to also run for non-trimmed R2R builds and wires `FixAbstractMethods` into the task call.

simonrozsival · 2026-03-31T09:19:36Z

Replying to the Copilot review comments — the implementation was reworked in commits 3-4, so all four comments are now outdated. Resolving them below.

…builds Move _LinkAssembliesNoShrink from the outer build into the inner (per-RID) build using BeforeTargets="_PrepareForReadyToRunCompilation". This ensures assembly modifications (FixAbstractMethods, AddKeepAlives, FindJavaObjects, etc.) operate on pure IL assemblies BEFORE crossgen2 creates R2R images, preventing Mono.Cecil from attempting to write mixed-mode assemblies. Changes: - Rewrite _LinkAssembliesNoShrink to run in the inner build, consuming @(ResolvedFileToPublish) instead of @(ResolvedAssemblies), writing to an intermediate linked-noshrink/ directory - Properly classify framework vs user assemblies by known names (matching the trimmed path in _AfterILLinkAdditionalSteps) - Update ResolvedFileToPublish so R2R and publish consume modified copies - Simplify _PrepareAssemblies: populate _ResolvedAssemblies unconditionally from @(ResolvedAssemblies) (no longer redirect through intermediate dir) - Extend _CopySidecarXmlToAssemblyPaths to handle both trimmed (linked/) and non-trimmed (android/linked-noshrink/) sidecar XML files - Remove _LinkAssembliesNoShrink from _LinkAssemblies DependsOnTargets - Remove Assert.Ignore workarounds in BuildTest.SimilarAndroidXAssemblyNames and LinkerTests.AndroidAddKeepAlives for CoreCLR non-trimmed Release builds Fixes: #11025 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

simonrozsival

🤖 AI Review Summary

Verdict: ⚠️ Needs Changes

Found 2 issues: 1 warning, 1 suggestion — no blocking bugs.

⚠️MSBuild targets: Missing Inputs/Outputs on _LinkAssembliesNoShrink — the target runs every inner build even when assemblies haven't changed (Xamarin.Android.Common.targets:1439)
💡 MSBuild targets: Duplicate FileWrites glob — the wildcard glob $(_LinkAssembliesNoShrinkDir)** already covers the per-file FileWrites on subsequent lines (Xamarin.Android.Common.targets:1501)

👍 What's great about this PR:

Correct root cause fix: running assembly modifications before R2R in the inner build prevents the Mono.Cecil NotSupportedException on mixed-mode assemblies
Proper framework/user assembly classification matching the trimmed path (4 known framework assembly names)
HasMonoAndroidReference=true is correctly scoped to only the root assembly (%(Filename) == $(TargetName)) instead of blanket-set on all DLLs
Clean generalization of _CopySidecarXmlToAssemblyPaths with $(_SidecarSubDir) to handle both trimmed (linked/) and non-trimmed (android/linked-noshrink/) paths
Good simplification of _PrepareAssemblies — unconditional population of _ResolvedAssemblies removes the confusing MonoAndroidIntermediateAssemblyDir redirect
Test cleanup: removing Assert.Ignore workarounds confirms the fix addresses the root cause

Review generated by android-reviewer from review guidelines.

simonrozsival · 2026-04-02T16:35:57Z

+-->
+<Target Name="_LinkAssembliesNoShrink"
+ BeforeTargets="_PrepareForReadyToRunCompilation"
+ Condition=" '$(PublishTrimmed)' != 'true' and '$(_ComputeFilesToPublishForRuntimeIdentifiers)' == 'true' ">


🤖 ⚠️MSBuild targets — This target has no Inputs/Outputs, so it runs on every inner build even when assemblies haven't changed. The trimmed counterpart (_AfterILLinkAdditionalSteps) uses Inputs="$(_LinkSemaphore)" / Outputs="$(_AdditionalPostLinkerStepsFlag)" for incrementality.
For the non-trimmed path, consider using Inputs="$(_AndroidBuildPropertiesCache)" or a similar stamp, and Outputs="$(_LinkAssembliesNoShrinkDir)noshrink.flag" (touched at the end). Without this, SaveChangedAssemblyStep will re-stamp assembly timestamps on every build, cascading through _GenerateJavaStubs → _CompileJava → _CompileToDalvik → _BuildApkEmbed → _Sign.
Rule: Incremental builds (Inputs/Outputs)

simonrozsival · 2026-04-02T16:35:57Z

+ <ItemGroup>
+ <ResolvedFileToPublish Remove="@(_LinkNoShrinkAllAssemblies)" />
+ <ResolvedFileToPublish Include="@(_LinkNoShrinkAllAssemblies->'$(_LinkAssembliesNoShrinkDir)%(RelativePath)')" />
+ <FileWrites Include="$(_LinkAssembliesNoShrinkDir)**" />


🤖 💡 MSBuild targets — The glob $(_LinkAssembliesNoShrinkDir)** already covers all files in the directory, including the DLLs, .jlo.xml, and .typemap.xml files listed in the subsequent <FileWrites> block (lines 1505-1507). The per-file entries are redundant. Consider keeping only the glob, or only the per-file entries (which are more explicit and don't risk over-matching).
Rule: Don't duplicate item group transforms

…ileWrites - Add Inputs/Outputs to _LinkAssembliesNoShrink using @(IntermediateAssembly) and $(_AndroidBuildPropertiesCache) as inputs, and a stamp file as output. This prevents SaveChangedAssemblyStep from re-stamping assembly timestamps on every build, which would cascade through downstream targets. - Touch stamp file after successful completion. - Remove duplicate FileWrites entries — the glob $(_LinkAssembliesNoShrinkDir)** already covers all files in the directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

jonathanpeppers · 2026-04-02T21:16:41Z

/azp run Xamarin.Android-PR

azure-pipelines · 2026-04-02T21:16:48Z

No pipelines are associated with this pull request.

simonrozsival · 2026-05-14T07:07:30Z

We should revisit this at some point, but this PR is already stale.

simonrozsival requested a review from jonathanpeppers as a code owner March 26, 2026 16:08

CopilotAI review requested due to automatic review settings March 26, 2026 16:08

Copilotstarted reviewing on behalf of simonrozsival March 26, 2026 16:09 View session

simonrozsival marked this pull request as draft March 26, 2026 16:10

CopilotAI reviewed Mar 26, 2026
View reviewed changes

simonrozsival force-pushed the fix/11025-r2r-abstract-methods branch from 5e65810 to 6469db8Compare March 31, 2026 11:50

simonrozsival force-pushed the fix/11025-r2r-abstract-methods branch from 6469db8 to ebcf2bdCompare April 2, 2026 15:31

simonrozsival marked this pull request as ready for review April 2, 2026 15:31

simonrozsival changed the base branch from main to dev/sbomer/merge-pipelinesApril 2, 2026 16:04

simonrozsival commented Apr 2, 2026
View reviewed changes

simonrozsival closed this May 14, 2026

jonathanpeppers mentioned this pull request Jun 9, 2026
[CoreCLR] Build R2R images for SDK assemblies in Debug builds #10760
Open

github-actionsBot locked and limited conversation to collaborators Jun 13, 2026

Navigation Menu

Uh oh!

Fix XALNS7015: run FixAbstractMethodsStep before R2R in non-trimmed CoreCLR builds - #11027

Fix XALNS7015: run FixAbstractMethodsStep before R2R in non-trimmed CoreCLR builds#11027
simonrozsival wants to merge 2 commits into
dev/sbomer/merge-pipelinesfrom
fix/11025-r2r-abstract-methods

simonrozsival commented Mar 26, 2026•
edited
Loading

Uh oh!

CopilotAI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Mar 31, 2026

Uh oh!

simonrozsival left a comment

Uh oh!

simonrozsivalApr 2, 2026

Uh oh!

simonrozsivalApr 2, 2026

Uh oh!

jonathanpeppers commented Apr 2, 2026

Uh oh!

azure-pipelinesBot commented Apr 2, 2026

Uh oh!

simonrozsival commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

simonrozsival commented Mar 26, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Fix: move _LinkAssembliesNoShrink to run before R2R compilation in the inner build

Tests

Depends on

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Mar 31, 2026

Uh oh!

simonrozsival left a comment

Choose a reason for hiding this comment

🤖 AI Review Summary

Uh oh!

simonrozsivalApr 2, 2026

Choose a reason for hiding this comment

Uh oh!

simonrozsivalApr 2, 2026

Choose a reason for hiding this comment

Uh oh!

jonathanpeppers commented Apr 2, 2026

Uh oh!

azure-pipelinesBot commented Apr 2, 2026

Uh oh!

simonrozsival commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simonrozsival commented Mar 26, 2026•
edited
Loading

Fix: move `_LinkAssembliesNoShrink` to run before R2R compilation in the inner build