[TrimmableTypeMap] Build pipeline: targets, stubs, manifest integration, feature switches#11036
[TrimmableTypeMap] Build pipeline: targets, stubs, manifest integration, feature switches#11036simonrozsival wants to merge 11 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR wires the TrimmableTypeMap feature into the .NET for Android build pipeline, adding CoreCLR linker/store integration, generating required Java/native stubs, and expanding the typemap generator to emit manifest + merged acw-map outputs.
Changes:
- Reworks trimmable typemap MSBuild targets to generate typemap assemblies/JCWs post-compile, integrate with ILLink, and populate assembly-store inputs.
- Adds new build-time stubs and preserve config (LLVM IR typemap stubs, empty ApplicationRegistration for trimmable path, ILLink root descriptor).
- Extends the TrimmableTypeMap generator/scanner to support manifest generation inputs and updated JNI/override naming behavior, with corresponding test updates.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Scanner/OverrideDetectionTests.cs | Updates expected native callback name derivation from Connector. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapModelBuilderTests.cs | Adjusts expectations for generated wrapper members. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapAssemblyGeneratorTests.cs | Aligns proxy type wrapper expectations with generator output. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs | Adds direct unit tests for the core generator pipeline and incremental behavior. |
| tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/JcwJavaSourceGeneratorTests.cs | Updates JNI name conversion and native method naming assertions. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/GenerateTrimmableTypeMapTests.cs | Refactors MSBuild-task tests to match new outputs/behavior. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs | Converts MSBuild task into adapter delegating to TrimmableTypeMapGenerator, adds manifest/acw-map integration inputs/outputs. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs | Allows CoreCLR trimmable path to proceed when JNIEnvInit tokens are trimmed (token=0 fallback). |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateEmptyTypemapStub.cs | New task generating per-ABI empty LLVM IR typemap stubs. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Major build pipeline rewrite for trimmable typemap generation, stub copying, and native-stub prep. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.CoreCLR.targets | Adds ILLink and assembly-store integration for typemap assemblies under CoreCLR. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Disables TrimmableTypeMap runtime feature for NativeAOT. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.MonoVM.targets | Disables TrimmableTypeMap runtime feature for MonoVM. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/ApplicationRegistration.Trimmable.java | Adds trimmable-path ApplicationRegistration stub with empty registerApplications(). |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/PreserveLists/Trimmable.CoreCLR.xml | Adds ILLink root descriptor to preserve JNIEnvInit.Initialize. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapTypes.cs | Introduces result/config records for manifest + provider sources. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs | Implements end-to-end generator (scan → typemap → JCW → manifest/acw-map outputs). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/NullableExtensions.cs | Adds NRT-friendly string helpers for netstandard2.0. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestGenerator.cs | Adds Generate overload accepting pre-loaded manifest template and refactors default creation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestModel.cs | Removes old manifest model types (replaced by scanner records). |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/JniSignatureHelper.cs | Fixes nested-class JNI name conversion for Java source output. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerScanner.cs | Adds manifest scanning, component attribute capture, and correct native callback/declaring-type derivation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs | Updates component attribute documentation and usage. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyIndex.cs | Expands attribute decoding to capture component properties, intent-filters, metadata, and assembly-level manifest info. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/AssemblyManifestInfo.cs | Adds aggregated assembly-level manifest data model. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/ComponentInfo.cs | Adds component kind + data model for manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/IntentFilterInfo.cs | Adds intent-filter data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/MetaDataInfo.cs | Adds metadata data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionInfo.cs | Adds permission data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionGroupInfo.cs | Adds permission-group data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PermissionTreeInfo.cs | Adds permission-tree data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesPermissionInfo.cs | Adds uses-permission data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesFeatureInfo.cs | Adds uses-feature data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesLibraryInfo.cs | Adds uses-library data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/UsesConfigurationInfo.cs | Adds uses-configuration data model used by manifest generation. |
| src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/PropertyInfo.cs | Adds manifest property data model used by manifest generation. |
| } else if (attrName == "IntentFilterAttribute") { | ||
| attrInfo ??= new TypeAttributeInfo ("IntentFilterAttribute"); | ||
| attrInfo.IntentFilters.Add (ParseIntentFilterAttribute (ca)); | ||
| } else if (attrName == "MetaDataAttribute") { | ||
| attrInfo ??= new TypeAttributeInfo ("MetaDataAttribute"); | ||
| attrInfo.MetaData.Add (ParseMetaDataAttribute (ca)); |
There was a problem hiding this comment.
IntentFilterAttribute / MetaDataAttribute can initialize attrInfo with a TypeAttributeInfo whose AttributeName is not a component attribute. If those attributes appear before [Activity]/[Service]/…, later component parsing won’t create the correct TypeAttributeInfo subtype (e.g., ApplicationAttributeInfo), and ToComponentInfo() won’t recognize the component kind, so manifest generation may silently drop component data/intent-filters/metadata. Consider keeping component-kind separate from intent-filter/metadata collection (e.g., always create the component TypeAttributeInfo via CreateTypeAttributeInfo() when a component attribute is encountered, and store intent-filters/metadata on a separate structure or on a type-info object that doesn’t overwrite the component attribute identity).
| <Target Name="_PrepareNativeAssemblySources" | ||
| Condition=" '$(_AndroidRuntime)' != 'NativeAOT' " | ||
| Inputs="@(_BuildTargetAbis)" | ||
| Outputs="@(_BuildTargetAbis->'$(_NativeAssemblySourceDir)typemaps.%(Identity).ll')"> |
There was a problem hiding this comment.
_PrepareNativeAssemblySources declares Outputs as typemaps.%(Identity).ll, but GenerateEmptyTypemapStub writes files named typemap.{abi}.ll. This output mismatch breaks MSBuild incremental up-to-date checks (the target will look for non-existent outputs and rerun every build). Update the Outputs pattern (and/or the generated filename) so they match exactly.
| Outputs="@(_BuildTargetAbis->'$(_NativeAssemblySourceDir)typemaps.%(Identity).ll')"> | |
| Outputs="@(_BuildTargetAbis->'$(_NativeAssemblySourceDir)typemap.%(Identity).ll')"> |
| var generator = CreateGenerator (); | ||
| Assert.Throws<ArgumentNullException> (() => generator.Execute ( | ||
| null!, Path.Combine (testDir, "out"), Path.Combine (testDir, "java"), | ||
| new Version (11, 0), new HashSet<string> ())); |
There was a problem hiding this comment.
This test uses the null-forgiving operator (null!) to pass a null argument. The repo guidance is to avoid ! even in tests; prefer a nullable local (e.g., IReadOnlyList<string>? assemblyPaths = null) and pass that, or otherwise structure the test so it doesn’t require !.
db8fc87 to
6500aa5
Compare
- Update TrimmableTypeMapGenerator.Execute() with manifest generation, assembly manifest scanning, acw-map writing, and new optional parameters - Add ManifestConfig record to TrimmableTypeMapTypes.cs - Update TrimmableTypeMapResult with AdditionalProviderSources - Update GenerateTrimmableTypeMap MSBuild task with manifest/config properties - Create GenerateEmptyTypemapStub task for LLVM IR native typemap stubs - Create ApplicationRegistration.Trimmable.java (empty registerApplications) - Create Trimmable.CoreCLR.xml preserve list for JNIEnvInit.Initialize - Rewrite Trimmable.targets with full build pipeline (separate generation and _GenerateJavaStubs targets, native stub generation, manifest handling) - Rewrite Trimmable.CoreCLR.targets with ILLink integration and per-ABI assembly store support - Add TrimmableTypeMap=false feature switch to MonoVM and NativeAOT targets - Handle trimmed JNIEnvInit tokens in GenerateNativeApplicationConfigSources Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove redundant #nullable enable from 6 Generator files and task file - Convert string.IsNullOrEmpty to IsNullOrEmpty extension in task Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The target declared Outputs as typemaps.{abi}.ll but GenerateEmptyTypemapStub
writes typemap.{abi}.ll. The mismatch caused MSBuild to always rerun the target.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a049075 to
2a245bb
Compare
simonrozsival
left a comment
There was a problem hiding this comment.
🤖 AI Review Summary
Verdict: ❌ Reject
Found 2 issues (1 critical, 1 warning):
- ❌ Error handling:
manifestTemplatePath(string) passed whereXDocument?is expected — compilation error CS1503 (TrimmableTypeMapGenerator.cs:102) ⚠️ Error handling:Path.GetDirectoryName()can return null, passed directly toDirectory.CreateDirectory()(TrimmableTypeMapGenerator.cs:60)
👍 Good use of Files.CopyIfStringChanged in GenerateEmptyTypemapStub. Feature switches for MonoVM/NativeAOT are a clean way to gate the trimmable path. The _RemoveRegisterAttribute override-as-no-op with explanatory comment is well-documented.
Review generated by android-reviewer from review guidelines.
| ApplicationJavaClass = config.ApplicationJavaClass, | ||
| }; | ||
|
|
||
| return generator.Generate (manifestTemplatePath, allPeers, assemblyManifestInfo, mergedManifestOutputPath); |
There was a problem hiding this comment.
🤖 ❌ Error handling — manifestTemplatePath is a string? but ManifestGenerator.Generate() expects XDocument? as the first parameter. This causes compilation error CS1503 (confirmed failing in internal CI build 13700906).
Fix: load the manifest template before passing it:
XDocument? manifestTemplateDoc = null;
if (!manifestTemplatePath.IsNullOrEmpty () && File.Exists (manifestTemplatePath)) {
manifestTemplateDoc = XDocument.Load (manifestTemplatePath);
}
return generator.Generate (manifestTemplateDoc, allPeers, assemblyManifestInfo, mergedManifestOutputPath);Rule: Fail fast on critical ops (Postmortem #6)
|
|
||
| // Write merged acw-map.txt if requested | ||
| if (!acwMapOutputPath.IsNullOrEmpty ()) { | ||
| Directory.CreateDirectory (Path.GetDirectoryName (acwMapOutputPath)); |
There was a problem hiding this comment.
🤖 Path.GetDirectoryName(acwMapOutputPath) can return null (e.g., root path). Passing null to Directory.CreateDirectory() throws ArgumentNullException.
var acwDirectory = Path.GetDirectoryName (acwMapOutputPath);
if (!acwDirectory.IsNullOrEmpty ()) {
Directory.CreateDirectory (acwDirectory);
}Rule: Validate parameters (Postmortem #44)
… null - Load manifestTemplatePath into XDocument before passing to ManifestGenerator.Generate() which expects XDocument? (not string) - Guard Path.GetDirectoryName() result with IsNullOrEmpty check before passing to Directory.CreateDirectory() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous PR (#11034) established the pattern that TTMG has no IO operations. The build pipeline PR regressed this by adding acw-map writing, manifest template loading, and manifest file saving into the generator. Move all IO back to GenerateTrimmableTypeMap (MSBuild task): - Load manifest template XDocument from file path - Write merged manifest to disk via XDocument.Save() - Write merged acw-map.txt via Files.CopyIfStreamChanged() Refactor ManifestGenerator.Generate to return (XDocument, IList<string>) instead of writing to disk. Add GeneratedManifest record to carry the in-memory result. Update ManifestGeneratorTests to work entirely in-memory (no temp dirs, no IDisposable). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…strumentation types
Application and Instrumentation types skip the JCW static initializer
block (CannotRegisterInStaticCtor) because libmonodroid.so isn't loaded
when these classes are first instantiated by the Android framework.
The legacy path deferred their registration to the generated
ApplicationRegistration.registerApplications() method. The trimmable
path incorrectly used an empty static file, meaning these types would
never get their native methods registered → UnsatisfiedLinkError.
Fix: dynamically generate ApplicationRegistration.java in the
GenerateTrimmableTypeMap MSBuild task with
mono.android.Runtime.registerNatives(MyApp.class);
for each Application/Instrumentation type. This triggers the
trimmable TrimmableTypeMap.OnRegisterNatives handler which
dispatches to the UCO-generated IAndroidCallableWrapper.RegisterNatives.
Call graph:
MonoPackageManager.LoadApplication()
→ Runtime.initInternal()
→ ApplicationRegistration.registerApplications()
→ Runtime.registerNatives(MyApp.class) [generated]
→ OnRegisterNatives (JNI native) [TrimmableTypeMap.cs]
→ IAndroidCallableWrapper.RegisterNatives() [UCO IL]
→ Application.onCreate() → n_onCreate() [now registered]
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…mmableTypeMap.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… in TrimmableTypeMapTypes - Restore IsMonoRuntime=false and IsCoreClrRuntime=false RuntimeHostConfigurationOption items accidentally removed from NativeAOT.targets - Change ApplicationRegistrationTypes parameter from null! to nullable (null?) to comply with the no null-forgiving operator rule 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 2 issues (1 error, 1 warning), plus 1 suggestion:
- ❌ Incremental build:
Document.Save()always updates the manifest timestamp — useFiles.CopyIfStringChanged(GenerateTrimmableTypeMap.cs:125) ⚠️ Dead work:PerAssemblyAcwMapFiles[Output]property andGeneratePerAssemblyAcwMaps()are now unused — the only consumer (_MergeAcwMaps) was removed in this PR. The method still runs unconditionally, writes per-assembly files to disk that nothing reads, and those files are no longer in@(FileWrites)soIncrementalCleanwill delete them.AcwMapDirectoryremains[Required]but only backs these dead files. Consider removingPerAssemblyAcwMapFiles,GeneratePerAssemblyAcwMaps(), and relaxing/removingAcwMapDirectory. (GenerateTrimmableTypeMap.cs:57,:117,:248)- 💡 Style:
System.Text.StringBuilderis fully-qualified; addusing System.Text;at top. (GenerateTrimmableTypeMap.cs:282)
👍 Positive callouts:
GenerateEmptyTypemapStubcorrectly usesFiles.CopyIfStringChanged— good for incremental.ManifestGeneratorno longer does file I/O; returning(XDocument, IList<string>)is a clean separation of concerns.RuntimeProviderJavaNamerefactor is clean — MSBuild decides the class name, the generator just uses it._RemoveRegisterAttributeoverride-as-no-op is well-documented._TrimmableRuntimeProviderJavaNamedefaults set correctly per-runtime in CoreCLR/NativeAOT targets._AddTrimmableTypeMapAssembliesToStorebatching withOutputs="%(_BuildTargetAbis.Identity)"is the right MSBuild pattern for per-ABI iteration.
using System.Xml.Linq (fixed in latest commit). New build is pending.
Review generated by android-reviewer from review guidelines.
src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs
Outdated
Show resolved
Hide resolved
src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs
Outdated
Show resolved
Hide resolved
- Use Files.CopyIfStringChanged instead of XDocument.Save() to preserve manifest file timestamp when content is unchanged (fixes incremental builds) - Remove dead PerAssemblyAcwMapFiles [Output] property, GeneratePerAssemblyAcwMaps() method, and AcwMapDirectory [Required] property — the only consumer (_MergeAcwMaps) was removed in this PR; nothing reads the per-assembly acw-map files anymore - Add 'using System.Text' and use short StringBuilder form Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
XDocument.ToString() drops the <?xml version="1.0" encoding="utf-8"?> declaration. Use XDocument.Save(Stream) + Files.CopyIfStreamChanged to preserve both the XML declaration and incremental-build safety. 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: ✅ LGTM
Found 1 issue — already self-corrected before posting this review:
- ❌ Formatting/Incremental build (
GenerateTrimmableTypeMap.cs):XDocument.ToString()drops the<?xml version="1.0" encoding="utf-8"?>declaration. Fixed in the latest commit by usingXDocument.Save(MemoryStream)+Files.CopyIfStreamChanged.
👍 Good use of Files.CopyIfStreamChanged throughout. The ManifestGenerator → RuntimeProviderJavaName refactor is clean. Nullable fixes and dead-code removal (PerAssemblyAcwMapFiles) are correct.
Review generated by android-reviewer from review guidelines.
|
/azp run Xamarin.Android-PR |
|
No pipelines are associated with this pull request. |
Summary
Build pipeline changes for the trimmable typemap feature. This PR stacks on PRs #11032, #11033, #11034, and #11035 — it includes their changes via a merge commit and adds the build infrastructure on top.
PR 4 commit changes (11 files, +492/-77):
New files
GenerateEmptyTypemapStub.cs— MSBuild task generating empty LLVM IR typemap stubs (typemap.{abi}.ll) per ABI. The native toolchain compiles these into the symbolslibmonodroid.soexpects.ApplicationRegistration.Trimmable.java— EmptyregisterApplications()for the trimmable path (Application/Instrumentation types are activated viaRuntime.registerNatives()+ UCO wrappers).Trimmable.CoreCLR.xml— ILLink preserve descriptor ensuringJNIEnvInit.Initializesurvives trimming (native entry point fromhost.cc).Rewritten targets
Trimmable.targets— Major rewrite:_GenerateTrimmableTypeMaptarget runs afterCoreCompile(uses@(ReferencePath))_GenerateJavaStubsbecomes the orchestration target (copies JCW files, manifest, ApplicationRegistration.java, acw-map.txt)_PrepareNativeAssemblySourcesgenerates empty typemap LLVM stubs viaGenerateEmptyTypemapStub_RemoveRegisterAttributeoverridden as no-op (trimmable path needs[Register]at runtime)_CollectPerAssemblyAcwMapsand_MergeAcwMaps(merged acw-map now written directly by the task)Trimmable.CoreCLR.targets— Major rewrite:_AddTrimmableTypeMapToLinkeradds TypeMap DLLs to ILLink_ConfigureTrimmableTypeMapForLinkerpasses--typemap-entry-assembly_AddTrimmableTypeMapAssembliesToStoreadds per-ABI TypeMap DLLs to assembly store (tries linked/ first, falls back to untrimmed)Feature switches
MonoVM.targets/NativeAOT.targets— AddMicrosoft.Android.Runtime.RuntimeFeature.TrimmableTypeMap=falsefeature switch (trimmable path is CoreCLR-only for now)Modified tasks
GenerateTrimmableTypeMap.cs— Added 14 manifest/config properties,ManifestConfigconstruction, mergedacw-map.txtoutput,AdditionalProviderSourcesoutputTrimmableTypeMapGenerator.cs— Added manifest generation, assembly manifest scanning, acw-map writingTrimmableTypeMapTypes.cs— AddedManifestConfigrecord,AdditionalProviderSourcesto resultGenerateNativeApplicationConfigSources.cs— Gracefully handle trimmed JNIEnvInit tokens whenTargetsCLR(use token 0 for missing tokens)Dependencies
Depends on (merge first):