Open
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.
sbomer
requested review from
grendello,
jonathanpeppers and
simonrozsival
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Moves post-ILLink assembly modification steps into the inner (per-RID) build so assemblies are modified after trimming but before R2R/AOT native compilation, and ensures generated sidecar XML artifacts are available next to the final assembly locations used by outer-build tasks.
Changes:
- Run
_AfterILLinkAdditionalStepsAfterTargets="ILLink"in the inner build and feedAssemblyModifierPipelinethe trimmed assemblies from$(IntermediateLinkDir). - Add
_CopySidecarXmlToAssemblyPathsto copy/create.jlo.xml/.typemap.xmlnext to final assembly outputs (e.g. R2R/publish locations) for outer-build consumers. - NativeAOT: re-add the user assembly into
@(ManagedAssemblyToLink)for ILLink + adjust ILC inputs; update an incremental-build warning expectation inPackagingTest.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Reorders post-link assembly modification into inner build and adds a target to propagate sidecar XML files to final assembly paths. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs | Updates incremental rebuild expectation: second build should not emit IL3053 warnings when only Strings.xml changes. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Ensures NativeAOT + ILLink includes the user assembly in ManagedAssemblyToLink and avoids duplicating it in ILC references. |
Member
jonathanpeppers
left a comment
jonathanpeppers
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict: ✅ LGTM
Found 1 suggestion — no blocking issues.
- 💡 Incremental builds: Consider adding
@(FileWrites)for sidecar XML files in_CopySidecarXmlToAssemblyPaths(Xamarin.Android.Common.targets:1668)
👍 What's great about this PR:
- Exceptionally thorough inline documentation at every decision point — the XML comments explain the "why" for each item group operation, condition, and edge case
- Correct framework assembly classification verified against
MonoAndroidHelper.IsFrameworkAssembly(the 4 names match exactly: Mono.Android, Mono.Android.Export, Mono.Android.Runtime, Java.Interop) - Comprehensive edge case handling: NativeAOT duplicate assemblies via
RemoveDuplicates, R2R composites with empty sidecar files, multi-RID vs single-RID path differences, assemblies not inManagedAssemblyToLink - Sound incrementality design: using
$(_LinkSemaphore)from ILLink as the Input ensures_AfterILLinkAdditionalStepsis correctly skipped when ILLink is skipped on no-change rebuilds - Clean test update that removes the NativeAOT-specific IL3053 workaround — the root cause (ILLink re-running unnecessarily) is fixed
_AndroidFixManagedAssemblyToLinkForILLinkneatly fixes two NativeAOT problems at once (ILLink processing the user assembly, and incremental build tracking via@(ManagedAssemblyToLink)in_RunILLinkInputs)
Review generated by android-reviewer from review guidelines.
| <Copy | ||
| SourceFiles="@(_SidecarXmlCopySource)" | ||
| DestinationFiles="@(_SidecarXmlCopyDestination)" | ||
| SkipUnchangedFiles="true" /> |
Member
There was a problem hiding this comment.
🤖 💡 Incremental builds — The sidecar XML files created/copied by this target are not added to @(FileWrites). While the target runs every build (no Inputs/Outputs), IncrementalClean could delete these intermediate files between builds. Consider adding:
<ItemGroup>
<FileWrites Include="@(_SidecarXmlCopyDestination)" />
</ItemGroup>after the Copy task. The Touch-created source files in linked/ may similarly benefit from being tracked in the inner build's @(FileWrites) (from _AfterILLinkAdditionalSteps).
Rule: FileWrites for intermediate files
Member
There was a problem hiding this comment.
I think this comment is valid and the other AI reviewer wasn't as accurate.
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.
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).