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

[simonrozsival]

Summary

Fixes #11025

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]

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

[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]

🤖 ⚠️ 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]

🤖 💡 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

[jonathanpeppers]

/azp run Xamarin.Android-PR

[azure-pipelines]

No pipelines are associated with this pull request.