[TrimmableTypeMap] Root manifest-referenced types as unconditional#11037
[TrimmableTypeMap] Root manifest-referenced types as unconditional#11037simonrozsival wants to merge 21 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the TrimmableTypeMap pipeline so Android components referenced only from a user-provided AndroidManifest.xml template are treated as unconditionally preserved, preventing them from being trimmed when there are no direct managed references.
Changes:
- Parse the manifest template for component declarations and mark matching scanned
JavaPeerInfoentries asIsUnconditional. - Integrate trimmable typemap generation more deeply into the MSBuild pipeline (targets + runtime host config), including generation of native typemap stub
.llsources. - Expand scanning/generator models for manifest-related metadata and update tests for the updated JNI/native-callback naming behavior.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/OverrideDetectionTests.cs | Updates expected derived native callback name for override detection. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapModelBuilderTests.cs | Adjusts expectations for generated wrapper surface. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs | Adjusts expectations for generated wrapper surface. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Adds direct generator tests, including manifest rooting scenarios. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/JcwJavaSourceGeneratorTests.cs | Updates JNI name conversion and generated native method naming expectations. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateTrimmableTypeMapTests.cs | Refactors MSBuild task tests to the new adapter behavior and outputs. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Makes the task a thin adapter over TrimmableTypeMapGenerator and passes manifest/acw-map parameters. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs | Allows missing JNIEnvInit tokens in the CoreCLR/trimmable path by using token 0. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateEmptyTypemapStub.cs | New task to generate empty LLVM IR typemap stubs per ABI. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Wires typemap generation target, JCW copying, manifest/acw-map handling, and native stub generation. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.CoreCLR.targets | Hooks generated TypeMap assemblies into ILLink and assembly store for CoreCLR builds. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Adds runtime feature flag defaulting trimmable typemap off for NativeAOT. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.MonoVM.targets | Adds runtime feature flag defaulting trimmable typemap off for MonoVM. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/ApplicationRegistration.Trimmable.java | Adds a trimmable-path ApplicationRegistration stub with empty registration. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/PreserveLists/Trimmable.CoreCLR.xml | Adds an ILLink descriptor to preserve Android.Runtime.JNIEnvInit.Initialize. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapTypes.cs | Adds shared result/config records (TrimmableTypeMapResult, ManifestConfig). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Implements manifest-rooting + orchestrates scan → typemap → JCW → manifest/acw-map. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesPermissionInfo.cs | New manifest model record for assembly-level uses-permission. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesLibraryInfo.cs | New manifest model record for assembly-level uses-library. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesFeatureInfo.cs | New manifest model record for assembly-level uses-feature. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesConfigurationInfo.cs | New manifest model record for assembly-level uses-configuration. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PropertyInfo.cs | New manifest model record for assembly-level property entries. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionTreeInfo.cs | New manifest model record for permission-tree. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionInfo.cs | New manifest model record for permission. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionGroupInfo.cs | New manifest model record for permission-group. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/MetaDataInfo.cs | New/updated model for metadata entries (type + assembly level). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Captures component attribute data + improves native callback name derivation and declaring type parsing. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs | Makes IsUnconditional mutable post-scan; documents component attribute storage. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/IntentFilterInfo.cs | New model for intent-filter data. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/ComponentInfo.cs | New model for component attributes and related intent filters/metadata. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyManifestInfo.cs | New model to aggregate assembly-level manifest attributes across assemblies. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyIndex.cs | Adds decoding/parsing for assembly-level manifest attributes + richer type attribute capture. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/NullableExtensions.cs | Adds NRT-friendly string helper extensions for netstandard2.0. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestModel.cs | Removes the old manifest model types (superseded by Scanner/ record types). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestGenerator.cs | Refactors manifest generation to accept a pre-loaded template document. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/JniSignatureHelper.cs | Fixes JNI name → Java name conversion for nested types ($ → .) in Java source contexts. |
src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs
Outdated
Show resolved
Hide resolved
...id.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets
Outdated
Show resolved
Hide resolved
...id.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets
Outdated
Show resolved
Hide resolved
db8fc87 to
6500aa5
Compare
e1819e1 to
c504fa1
Compare
a049075 to
2a245bb
Compare
c504fa1 to
5926858
Compare
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict:
Found 2 issues (1 warning, 1 formatting):
⚠️ Formatting: Broken indentation inacwDirectorynull-check block (TrimmableTypeMapGenerator.cs:66-68)- 💡 API design:
IsUnconditionalchanged frominittoset— consider documenting the mutation contract
👍 Excellent test coverage — 6 new tests covering rooting, warnings, already-unconditional, empty manifest, relative names, and nested types. The ResolveManifestClassName logic correctly handles all three Android manifest name conventions (fully-qualified, dot-relative, simple).
Review generated by android-reviewer from review guidelines.
src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs
Outdated
Show resolved
Hide resolved
f7cefab to
a36a8a0
Compare
Parse the user's AndroidManifest.xml template for activity, service, receiver, and provider elements with android:name attributes. Mark matching scanned Java peer types as IsUnconditional = true so the ILLink TypeMap step preserves them even if no managed code references them directly. Changes: - JavaPeerInfo.IsUnconditional: init → set (must be mutated after scanning) - TrimmableTypeMapGenerator: add warn callback, RootManifestReferencedTypes() called between scanning and typemap generation - GenerateTrimmableTypeMap task: pass Log.LogWarning as warn callback - 4 new xUnit tests covering rooting, unresolved warnings, already-unconditional skip, and empty manifest Replaces #11016 (closed — depended on old PR shape with TaskLoggingHelper). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix RootManifestReferencedTypes to resolve relative android:name values (.MyActivity, MyActivity) using manifest package attribute - Keep $ separator in peer lookup keys so nested types (Outer$Inner) match correctly against manifest class names - Guard Path.GetDirectoryName against null return for acw-map path - Fix pre-existing compilation error: load XDocument from template path before passing to ManifestGenerator.Generate - Add tests for relative name resolution and nested type matching Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove file-path overload (IO belongs in MSBuild task, not generator) - Accept XDocument? directly, handle null with pattern match - Add ResolveManifestClassName to resolve dot-relative (.MyActivity) and simple (MyService) names against the manifest package attribute - Fix '$' handling in peer lookup: manifests use '$' for nested types, don't replace it with '.' in the lookup key Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
266703c to
431f59e
Compare
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict:
Found 1 issue:
- ❌ Bugs & correctness:
RootManifestReferencedTypes()still skips<application android:name>and<instrumentation android:name>, so manifest-only entry points can still be trimmed away (src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs:160).
The added coverage for relative and nested names is solid. Public CI is still pending, so I’m holding off on a ✅ until that and the manifest gap are addressed.
Review generated by android-reviewer from review guidelines.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict:
Found 1 issue:
- ❌ MSBuild tasks: Bare
Log.LogWarningwithout an XA code (GenerateTrimmableTypeMap.cs:99)
CI: The public dotnet-android check failure is an infrastructure agent-assignment abandonment, not caused by this PR's code. macOS build passed. Internal Xamarin.Android-PR is still running.
Positive callouts: The RootManifestReferencedTypes design is clean — keeping the lookup dictionary local to the method avoids any cross-call state pollution. The early-out on componentNames.Count == 0 is good. The ResolveManifestClassName logic correctly handles dot-prefix, bare (no-dot) names, and $-separated nested types. Mutation of IsUnconditional from init to set is safe: JavaPeerInfo is only ever stored as a list/dictionary value, never as a key, so hash-code mutation under a collection key cannot occur. The seven unit tests cover the happy path, relative names, nested types, already-unconditional types, empty manifest, unresolved types, and application/instrumentation types — solid coverage.
Review generated by android-reviewer from review guidelines.
src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict:
I didn’t find any further code blockers after the latest follow-up fixes. The manifest-rooting, placeholder-resolution, and deferred-registration changes all look consistent, and the standalone typemap test suite is passing locally.
The remaining blocker is CI: the dotnet-android checks are still in progress, so this PR is not mergeable yet.
Found 1 suggestion:
- 💡 Testing — consider mirroring the new placeholder-rooting regression at the host-side MSBuild task layer as well, so the end-to-end
GenerateTrimmableTypeMapwiring stays covered.
Review generated by android-reviewer from review guidelines.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs
Show resolved
Hide resolved
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Parses the user's
AndroidManifest.xmltemplate for componentandroid:nameentries and marks matching scanned Java peer types asIsUnconditional = true, so the trimmable TypeMap / ILLink path preserves them even when no managed code references them directly.This now covers:
application,instrumentation,activity,service,receiver, andprovider.MyActivityMyServiceOuter$InnerReplaces #11016 (closed — depended on the old PR shape).
Changes
JavaPeerInfo.IsUnconditional:init→set, with the mutation contract documented as one-way (false→true) for post-scan rootingJavaPeerInfo.CannotRegisterInStaticConstructor: now also supports manifest-rootedapplication/instrumentationtypes so they still go through deferredregisterNatives()TrimmableTypeMapGeneratorJavaNameandCompatJniName$and Java-source nested-type naming formsITrimmableTypeMapLoggerinterface for warnings + progress/info messagesGenerateTrimmableTypeMaptaskXA4250for unresolved manifest-referenced typesXA4250message documentation entryapplication/instrumentationentriesJavaName != CompatJniNameStatus
This PR has been rebased after #11036 merged and now stands directly on
main.