[Xamarin.Android.Build.Tasks] Stop legacy assembly scans - #12968
Closed
simonrozsival wants to merge 1 commit into
Closed
simonrozsival wants to merge 1 commit into
simonrozsival wants to merge 1 commit into
Conversation
Untrimmed builds call <LinkAssembliesNoShrink/>. The shared Xamarin.Android.Tasks.AssemblyModifierPipeline registers legacy JCW and typemap scanners even though trimmable is the only supported backend. The IList export coverage added in #12931 exposes this path after #12887 made trimmable the default, causing XALNS7003 in Debug. Remove both scanner registrations instead of retaining an LLVM selector or fallback. Preserve compatibility fixups, keep-alives and assembly saving, and leave modern generator timing unchanged. Add task and app/library regression coverage for legacy-scan absence, trimmable-only exports, final staged assembly identity and MVID resolution, and unchanged/body/export-changed incremental builds. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
An existing regression test still requires the removed .jlo.xml output and will fail.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Removes legacy Cecil scanner registration from assembly modification pipelines while preserving assembly saving and compatibility fixups.
Changes:
- Removes legacy Java-object and type-map scanners.
- Adds pipeline-ordering and untrimmed trimmable-build coverage.
- Verifies modern output generation and incrementality.
| File | Description |
|---|---|
AssemblyModifierPipeline.cs |
Retains only assembly saving. |
LinkerTests.cs |
Tests scanner removal and step ordering. |
TrimmableTypeMapBuildTests.cs |
Adds end-to-end regression coverage. |
| pipeline.Steps.Add (findJavaObjectsStep); | ||
|
|
||
| // SaveChangedAssemblyStep | ||
| // Java peer scanning belongs to the trimmable typemap generator. |
Member
Author
|
superseded by #12976 |
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.
Why
Untrimmed builds still enter the
_LinkAssembliesNoShrinktarget when$(PublishTrimmed)is nottrue.<LinkAssembliesNoShrink/>must stage assemblies and preserve opted-in compatibility fixes, but its sharedXamarin.Android.Tasks.AssemblyModifierPipeline.BuildPipeline()also unconditionally registeredFindJavaObjectsStepandFindTypeMapObjectsStep.That runs the legacy Cecil JCW/type-map scanners alongside the modern trimmable pipeline. The interaction between #12887 (trimmable by default) and #12931 (expanded export coverage) exposes the regression:
TrimmableExportReferencePeer.MakeList()exportsSystem.Collections.IList, which the modern generator supports but the legacy importer does not. BuildingMono.Android.NET-Testsin untrimmed Debug fails withXALNS7003and the misleading legacyArgumentNullExceptionnamingconnector, before any device tests run.Change
Remove both legacy scanner registrations outright. Trimmable is the only supported product backend, so there is no LLVM selector, legacy default, fallback, or new task input. Direct callers of
<AssemblyModifierPipeline/>now get the assembly-save step only; they no longer perform Java peer scanning.Keep
SaveChangedAssemblyStepand the existing<LinkAssembliesNoShrink/>compatibility/resource-designer/keep-alive steps and ordering unchanged. Leave the modern generator timing and trimmed CoreCLR post-link pipeline unchanged. Modern type maps reference their owners symbolically by assembly/type/member identity, not by the owner's original MVID or metadata tokens.Add focused coverage proving:
IListexport shape as the real fixture, and produces modern type-map and Java outputs without either legacy XML sidecar.This is a standalone regression fix against
main. The original export fixtures remain unmodified. There are no native, LLVM-removal, assembly-store/ELF, R8, localization, or bootstrap changes, and no prerequisite PR.Validation
Built an independent SDK from
mainat976e5528dac9a9a4a18c21cc74b431160ae1d74fusing the pinned .NET SDK12.0.100-alpha.1.26477.101,make prepare, andmake leeroy(including extra API bindings and local workload setup). No source/framework/runtime pin overrides were used.The original unmodified Debug fixture first reproduced the exact
XALNS7003failure against the main-built task binaries. With the final scanner-removal implementation:Mono.Android.NET-Tests, Debug,PublishTrimmed=false, android-arm64JniReferenceLeaksflavorPublishTrimmed=trueAll four original-fixture builds use the same single android-arm64 RID and retain the export tests. Validation is host-side/build-only: no install target, physical device, or emulator was used. Local reruns disable build-server/node reuse after replacing task assemblies to avoid testing a previously loaded DLL; that is only a validation prerequisite, not a product workaround.