[TrimmableTypeMap] Build pipeline: targets, scanner, task, assembly store#10980
[TrimmableTypeMap] Build pipeline: targets, scanner, task, assembly store#10980simonrozsival wants to merge 31 commits intomainfrom
Conversation
...id.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets
Show resolved
Hide resolved
....Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.CoreCLR.targets
Outdated
Show resolved
Hide resolved
....Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.CoreCLR.targets
Outdated
Show resolved
Hide resolved
e2214d6 to
483dc03
Compare
12a1c2b to
4d55775
Compare
76a490e to
aad2ddb
Compare
af78ed0 to
5a4a1f3
Compare
5a4a1f3 to
2bdc637
Compare
…t providers - Fix Categories parsing: remove TryGetNamedArgument<string> guard that always returns false for string[] properties. Directly iterate named arguments with IReadOnlyCollection<> cast. - Delete empty ManifestModel.cs placeholder (all types in JavaPeerInfo.cs). - Document ApplicationRegistration.java: registerApplications() is intentionally empty in the trimmable path (types activated via registerNatives + UCO wrappers, not Runtime.register). - Add TODO for multi-process per-provider .java generation. 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>
Enables ILLink to trim TrimmableTypeMap code paths when ILLink runs (e.g. with PublishTrimmed=true or AOT). Without this, the feature switch is unknown to the trimmer and both branches are preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
get_TargetType is no longer emitted on proxy types — it's inherited from the generic base class JavaPeerProxy<T>. Update test assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use $(IntermediateOutputPath) instead of manually constructing the path from $(BaseIntermediateOutputPath)$(Configuration)/$(TargetFramework)/. The manual construction breaks when AppendTargetFrameworkToOutputPath is false, as in the test infrastructure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1. Add $(_AndroidManifestAbs) to _GenerateTrimmableTypeMap Inputs so manifest changes trigger re-generation. 2. Use @(ReferencePath->Count()) instead of '@(ReferencePath)' != '' to avoid string-joining hundreds of assembly paths. 3. Add XA4212 and XA4217 error codes for duplicate/conflicting [Application] attributes (replacing bare Log.LogError calls). 4. Add copied Java files, ApplicationRegistration.java, AndroidManifest, and acw-map.txt to @(FileWrites) so IncrementalClean won't delete them. 5. Add Inputs/Outputs to _PrepareNativeAssemblySources so MSBuild can skip it on incremental builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pass AcwMapOutputFile to GenerateTrimmableTypeMap so AcwMapWriter produces real managed→Java mappings for _ConvertCustomView and layout fixup. The <Touch> in _GenerateJavaStubs now only creates an empty placeholder when _GenerateTrimmableTypeMap was skipped. The generated acw-map.txt is tracked in @(FileWrites) from both targets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move business logic into a core class in the TrimmableTypeMap project. The MSBuild task becomes a thin adapter. Tests instantiate the generator directly — no MSBuild ceremony needed for unit testing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6f42b4d to
384a5a8
Compare
src/Microsoft.Android.Sdk.TrimmableTypeMap/Microsoft.Android.Sdk.TrimmableTypeMap.csproj
Outdated
Show resolved
Hide resolved
...in.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets
Show resolved
Hide resolved
- Remove commented-out lines in TypeMapAssemblyGeneratorTests and TypeMapModelBuilderTests - Replace WriteLinesToFile inline Java with a static ApplicationRegistration.Trimmable.java file - Remove UseMonoRuntime from HelloWorld.DotNet.csproj (not needed) - Move MSBuildPackageReferenceVersion to eng/Versions.props (centralize with other package versions) - Fix TrimmableTypeMap feature switch in NativeAOT.targets: Value=true (NativeAOT uses trimmable typemap) 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 5 issues: 2 errors, 2 warnings, 1 suggestion.
- ❌ Localization — Error messages hardcoded as strings instead of
Properties.Resources.XA####(TrimmableTypeMapGenerator.cs:247,261,263) - ❌ Platform-specific — Backslash path separators in cross-platform target will break the Mac/Linux build (
Microsoft.Android.Sdk.TypeMap.Trimmable.targets:103,105,108) ⚠️ Code organization — ~13 new public types added toJavaPeerInfo.cs; one type per file (JavaPeerInfo.cs:322+)⚠️ Nullable —string.IsNullOrEmpty(x)should use the.IsNullOrEmpty()extension (multiple files)- 💡 Patterns —
TrimmableTypeMapResultrecord exposesList<string>in public API; preferIReadOnlyList<string>
👍 Great use of sealed record types for data carriers, proper #nullable enable, thorough XML docs, and clean Inputs/Outputs for incremental builds. The GetNativeCallbackName/ParseConnectorDeclaringType refactoring is well-reasoned.
Review generated by android-reviewer from review guidelines.
src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs
Outdated
Show resolved
Hide resolved
...id.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets
Outdated
Show resolved
Hide resolved
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/JavaPeerInfo.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestGenerator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs
Outdated
Show resolved
Hide resolved
- Use Properties.Resources.XA#### for error messages (XA4212, XA4213, XA4217)
- Add Properties/Resources.resx + Resources.Designer.cs to TrimmableTypeMap project
- Remove XA4212/XA4217 from Xamarin.Android.Build.Tasks resources (they are
only used in TrimmableTypeMapGenerator, not in any Build.Tasks code path)
- Fix cross-platform path separator bugs in Trimmable.targets
- Replace \**\*.java glob with /**/*.java
- Replace android\src\ destination paths with android/src/
- Replace ..\ TrimmerRootDescriptor path with ../
- Split 13 new types out of JavaPeerInfo.cs (one type per file)
- ComponentInfo.cs (ComponentKind + ComponentInfo)
- IntentFilterInfo.cs
- MetaDataInfo.cs
- AssemblyManifestInfo.cs
- ManifestAttributeInfo.cs (Permission*, UsesPermission, UsesFeature,
UsesLibrary, UsesConfiguration, PropertyInfo)
- TrimmableTypeMapTypes.cs (ManifestConfig + TrimmableTypeMapResult)
- Replace string.IsNullOrEmpty() static calls with .IsNullOrEmpty() extension
- Add NullableExtensions.cs to TrimmableTypeMap project
- ManifestGenerator.cs: 6 occurrences
- TrimmableTypeMapGenerator.cs: 3 occurrences
- Change List<string> to IReadOnlyList<string> in TrimmableTypeMapResult
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 4 issues: 1 error + 2 warnings + 1 suggestion.
- ❌ MSBuild targets:
AcwMapOutputFilepassed to task but no such property exists → MSB4064 build error (Microsoft.Android.Sdk.TypeMap.Trimmable.targets:77) ⚠️ Code organization: 8 types inManifestAttributeInfo.cs— one type per file (Scanner/ManifestAttributeInfo.cs:7)⚠️ Code organization:ManifestGeneratorunnecessarily madepublic— new helpers default tointernal(Generator/ManifestGenerator.cs:16)- 💡 MSBuild targets:
AfterTargets="CoreCompile"runs even ifCoreCompilefails; consider$(MSBuildLastTaskResult)guard
👍 The overall architecture is solid: AndroidTask with correct TaskPrefix, [Required] property conventions followed, error codes from Properties.Resources, IsNullOrEmpty() extension used consistently throughout, Files.CopyIfStringChanged in GenerateEmptyTypemapStub, and FileWrites tracked correctly for incremental builds. The JniNameToJavaName fix for $ → . in inner class names is correct and well-tested.
Review generated by android-reviewer from review guidelines.
| ManifestPlaceholders="$(AndroidManifestPlaceholders)" | ||
| CheckedBuild="$(_AndroidCheckedBuild)" | ||
| ApplicationJavaClass="$(AndroidApplicationJavaClass)" | ||
| AcwMapOutputFile="$(IntermediateOutputPath)acw-map.txt"> |
There was a problem hiding this comment.
🤖 ❌ MSBuild targets — AcwMapOutputFile is passed here but the GenerateTrimmableTypeMap C# task class has no such property. MSBuild will fail with MSB4064 ("The 'GenerateTrimmableTypeMap' task does not support the parameter 'AcwMapOutputFile'"). Either add the property to the task class, or remove this line if it was left from an earlier refactor.
Rule: Incremental builds / task parameter accuracy
src/Microsoft.Android.Sdk.TrimmableTypeMap/Scanner/ManifestAttributeInfo.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestGenerator.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Android.Sdk.TrimmableTypeMap/Generator/ManifestGenerator.cs
Show resolved
Hide resolved
- Fix AcwMapOutputFile: add property to GenerateTrimmableTypeMap task, wire through TrimmableTypeMapGenerator.Execute, and write acw-map.txt via AcwMapWriter so _ConvertCustomView and _UpdateAndroidResgen get real managed→Java mappings - Split ManifestAttributeInfo.cs: move each of the 8 types to its own file (one type per file rule) - Make ManifestGenerator internal: no external consumers; tests use InternalsVisibleTo - Return IReadOnlyList<string> from public Generate overloads (was IList) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…peMapImplementation=trimmable The unconditional Value="true" was causing all non-trimmable NativeAOT builds to fail: ILLink eliminated the legacy binary typemap code path (expecting the trimmable typemap assembly to be present), but the typemap DLLs are only generated when _AndroidTypeMapImplementation=trimmable. When _AndroidTypeMapImplementation=trimmable, Microsoft.Android.Sdk.TypeMap.Trimmable.targets (imported conditionally) already adds RuntimeHostConfigurationOption Value="true", so NativeAOT.targets should not also set it unconditionally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The trimmable typemap targets don't support MonoVM yet, so explicitly set UseMonoRuntime=false for the HelloWorld build in Linux and package test stages. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- GenerateTrimmableTypeMapTests: assert empty arrays instead of null (the task returns [].ToArray(), not null, for empty results) - Update MonoVM apkdesc baseline: ILLink now trims dead trimmable typemap branches thanks to RuntimeFeature.TrimmableTypeMap=false Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
89052b9 to
3c1d9d7
Compare
…ensions, make new types internal - Remove '!' on acwMapOutputPath (line 73) — flow analysis handles nullability - Use .IsNullOrEmpty() instead of 'is not null && .Length > 0' for consistency - Use .IsNullOrEmpty() extension in AssemblyIndex.TryGetNameFromDecodedAttribute - Avoid re-enumeration: store group.ToList() result and use .Count property - Make new types internal: TrimmableTypeMapGenerator, ManifestConfig, TrimmableTypeMapResult, AssemblyManifestInfo, and scanner model types (PermissionInfo, UsesPermissionInfo, etc.) - Add InternalsVisibleTo for Xamarin.Android.Build.Tasks - Keep ComponentInfo, IntentFilterInfo, MetaDataInfo public (referenced by public JavaPeerInfo.ComponentAttribute) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lyfill conflict InternalsVisibleTo exposes the internal NotNullWhenAttribute polyfill from the netstandard2.0 TrimmableTypeMap assembly, which conflicts with Build.Tasks' own polyfill from Java.Interop/NullableAttributes.cs. Remove the IVT and keep TrimmableTypeMapGenerator, ManifestConfig, TrimmableTypeMapResult public instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Closing in favor of splitting into smaller, focused PRs. Will open 5 fresh PRs from main with clean history. |
Summary
Build pipeline for the trimmable typemap. Wires up manifest generation, assembly store,
and JCW generation. HelloWorld app runs end-to-end on device with button Click events.
Closes #10807.
Closes #10800.
Depends on #10991 (ManifestGenerator), #10967 (runtime).
Scanner extensions (
AssemblyIndex.cs,JavaPeerInfo.cs,JavaPeerScanner.cs)ComponentInfo,IntentFilterInfo,MetaDataInfodata model for type-level component attributesAssemblyManifestInfowithScanAssemblyAttributes()for assembly-level attrsHasPublicParameterlessCtorvia SRM method signature inspection (XA4213)ScanAssemblyManifestInfo()returns assembly-level data alongside peer listGenerateTrimmableTypeMaptaskManifestGenerator(from [TrimmableTypeMap] Add ManifestGenerator #10991)mono/Implementors (e.g.View_OnClickListenerImplementor). Framework binding types exist injava_runtime.dex; Implementor types need fresh generation becausemono.android.jaruses legacyTypeManager.Activate()which is incompatible withRuntime.registerNatives(). Pre-generating SDK-compatible JCWs tracked by [TrimmableTypeMap] Pre-generating code for Mono.Android and other SDK assemblies #10792.JCW generation fixes
JniNameToJavaName: converts$→.for inner class references in Java source (javac requires., not$)GetNativeCallbackName: splits[Register]Connector string on:before checkingGet…HandlerpatternParseConnectorDeclaringType: extracts the Invoker type from the Connector so UCO wrappers resolve callbacks on the correct typeTrimmer size regression prevention
TrimmableTypeMap=falseviaRuntimeHostConfigurationOptioninMonoVM.targetsandNativeAOT.targetsso ILLink can eliminate trimmable typemap code pathsIsMonoRuntimeandIsCoreClrRuntimeTrimmableTypeMapuses private ctor +Initialize()+ singleton pattern — no type references leak outside feature guardsTrimmableTypeMapTypeManagerusesTrimmableTypeMap.Instancesingleton (no constructor parameter)_RemoveRegisterAttribute: empty target —[Register]attributes needed at runtime forTryGetJniNameForTypeMSBuild targets
Trimmable.targets_GenerateTrimmableTypeMap:AfterTargets="CoreCompile"withInputs/Outputsfor incremental builds_GenerateJavaStubs: assembly store wiring, TypeMap DLLs to_ResolvedAssembliesReplace('\','/'))Trimmable.CoreCLR.targetsManagedAssemblyToLink,--typemap-entry-assembly_AddTrimmableTypeMapAssembliesToStore: per-ABI batched,linked/totypemap/fallbackTests
PublishTrimmed=true(ILLink inner build) verified: builds and runs successfullyBuildReleaseArm64SimpleDotNet.MonoVMapkdesc baseline updatedOther
GenerateEmptyTypemapStub.csFiles.CopyIfStringChanged)GenerateNativeApplicationConfigSources.csTargetsCLRTrimmable.CoreCLR.xmlJNIEnvInit.InitializeTypeMapAssemblyEmitter.csHelloWorldsample_AndroidTypeMapImplementation=trimmablePart of #10807 (build pipeline, targets, manifest integration, native stubs)