Remove legacy LLVM IR type map build pipeline - #12889
simonrozsival wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved provider-source generation and library llvm-ir runtime-jar issues remain, alongside a P/Invoke coverage gap.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This pull request removes the obsolete LLVM IR typemap and marshal-method pipelines while retaining trimmable typemaps, compatibility symbols, P/Invoke preservation, NativeAOT support, and remapping.
Changes:
- Removes legacy typemap, Java-stub, marshal-rewriting, and post-link tasks.
- Updates incremental native-source handling, tests, resources, and documentation.
- Rejects unsupported marshal methods with XA1049.
Review findings:
- Critical: Restore CoreCLR additional provider-source generation for secondary-process manifest providers.
- Moderate: Add multi-ABI P/Invoke preservation coverage.
- Moderate: Preserve or consistently reject legacy
llvm-irruntime-jar selection for libraries. - Nit: Update the native marshal source task description.
| File | Change |
|---|---|
tests/MSBuildDeviceIntegration/Tests/MarshalMethodsGCHangTests.cs |
Removes obsolete marshal-method device test. |
tests/MSBuildDeviceIntegration/Tests/InstallAndRunTests.cs |
Removes marshal-method test variants. |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/TypeDataBuilder.cs |
Adapts legacy entry generation. |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.IntegrationTests/ScannerRunner.cs |
Adds a legacy scanner entry helper. |
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets |
Removes legacy wiring and updates compatibility generation. |
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj |
Removes obsolete sources and resources. |
src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingReleaseNativeAssemblyGenerator.cs |
Deletes the legacy release generator. |
src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingDebugNativeAssemblyGenerator.cs |
Deletes the legacy debug generator. |
src/Xamarin.Android.Build.Tasks/Utilities/TypeMappingAssemblyGenerator.cs |
Deletes the legacy generator base. |
src/Xamarin.Android.Build.Tasks/Utilities/TypeMapObjectsXmlFile.cs |
Deletes legacy typemap XML support. |
src/Xamarin.Android.Build.Tasks/Utilities/TypeMapGenerator.cs |
Deletes the legacy typemap generator. |
src/Xamarin.Android.Build.Tasks/Utilities/TypeMapCecilAdapter.cs |
Deletes the legacy Cecil adapter. |
src/Xamarin.Android.Build.Tasks/Utilities/PreservePinvokesNativeAssemblyGenerator.cs |
Decouples P/Invoke preservation from removed state. |
src/Xamarin.Android.Build.Tasks/Utilities/NativeTypeMappingData.cs |
Deletes legacy native mapping data. |
src/Xamarin.Android.Build.Tasks/Utilities/NativeCodeGenState.cs |
Deletes obsolete code-generation state. |
src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs |
Simplifies assembly resolver setup. |
src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsState.cs |
Deletes obsolete marshal state. |
src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsNativeAssemblyGenerator.cs |
Generates compatibility native symbols. |
src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsHelpers.cs |
Deletes marshal helpers. |
src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsCollection.cs |
Deletes the marshal classifier collection. |
src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodCecilAdapter.cs |
Deletes marshal state conversion. |
src/Xamarin.Android.Build.Tasks/Utilities/ManifestDocument.cs |
Updates manifest-generation documentation. |
src/Xamarin.Android.Build.Tasks/Utilities/JCWGenerator.cs |
Deletes the legacy JCW generator. |
src/Xamarin.Android.Build.Tasks/Utilities/AssemblyPipeline.cs |
Removes marshal-method pipeline state. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LlvmIrGeneratorTests.cs |
Tests compatibility stub generation. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/LinkerTests.cs |
Removes legacy typemap assertions. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/MarshalMethodTests.cs |
Deletes obsolete marshal tests. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/ManifestTest.cs |
Updates manifest-generation documentation. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/InvalidConfigTests.cs |
Removes an obsolete pipeline assertion. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs |
Updates typemap paths and removes obsolete tests. |
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs |
Expands XA1049 validation coverage. |
src/Xamarin.Android.Build.Tasks/Tasks/RewriteMarshalMethods.cs |
Deletes the marshal rewriter task. |
src/Xamarin.Android.Build.Tasks/Tasks/RemoveRegisterAttribute.cs |
Deletes the legacy attribute-removal task. |
src/Xamarin.Android.Build.Tasks/Tasks/PreTrimmingFixLegacyDesigner.cs |
Deletes the legacy pre-trimming task. |
src/Xamarin.Android.Build.Tasks/Tasks/PrepareAbiItems.cs |
Removes the legacy typemap mode. |
src/Xamarin.Android.Build.Tasks/Tasks/GetMonoPlatformJar.cs |
Deletes the obsolete platform-jar task. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateTypeMappings.cs |
Deletes the legacy mapping task. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeMarshalMethodSources.cs |
Retains compatibility and P/Invoke source generation. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeApplicationConfigSources.cs |
Disables removed marshal state in app configuration. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeAotLibraryLoadAssemblerSources.cs |
Updates resolver usage. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeAotBootstrapSources.cs |
Simplifies bootstrap documentation. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateMainAndroidManifest.cs |
Deletes the legacy manifest task. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaStubs.cs |
Deletes the legacy Java-stub task. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateJavaCallableWrappers.cs |
Deletes the legacy wrapper task. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateEmptyTypemapStub.cs |
Retains stub generation with updated metadata handling. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateAdditionalProviderSources.cs |
Retains shared provider-generation helpers. |
src/Xamarin.Android.Build.Tasks/Tasks/GenerateACWMap.cs |
Deletes the legacy ACW map task. |
src/Xamarin.Android.Build.Tasks/Tasks/CreateTypeManagerJava.cs |
Deletes the obsolete Java resource task. |
src/Xamarin.Android.Build.Tasks/Tasks/CollectTypeMapFilesForArchive.cs |
Deletes legacy archive collection. |
src/Xamarin.Android.Build.Tasks/Tasks/AssemblyModifierPipeline.cs |
Removes the marshal-method pipeline option. |
src/Xamarin.Android.Build.Tasks/Properties/Resources.resx |
Updates XA1049 text. |
src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs |
Updates generated XA1049 metadata. |
src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Resource.Designer.targets |
Updates resource-designer trimming documentation. |
src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets |
Preserves typemap stub items incrementally. |
src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.BuildOrder.targets |
Removes obsolete post-link ordering. |
src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets |
Removes obsolete post-link dependency. |
src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FindTypeMapObjectsStep.cs |
Deletes the legacy linker scanner. |
src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FindJavaObjectsStep.cs |
Removes marshal-method-specific behavior. |
Documentation/docs-mobile/messages/xa1049.md |
Documents marshal-method removal. |
Documentation/docs-mobile/messages/index.md |
Updates the XA1049 index entry. |
Documentation/docs-mobile/building-apps/build-properties.md |
Documents obsolete property behavior. |
Files not reviewed (1)
- src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
5866c12 to
6b8b57e
Compare
df2de16 to
ab4d8d4
Compare
03fb13c to
31791c5
Compare
85dd239 to
1ddbe25
Compare
Keep native host compatibility stubs and P/Invoke preservation while removing unreachable typemap and marshal-method generation, rewriting, and linker steps. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use trimmable Java APIs for explicit library opt-ins without importing application type map targets, and retain the packaged CoreCLR/default jars for libraries without an implementation selection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep generating additional Java runtime providers for trimmable CoreCLR apps after removing the legacy GenerateAdditionalProviderSources task. Register a dedicated task and track generated Java files for incremental Clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The shared CoreCLR provider generation is already supplied by main. Keep a single call while preserving the legacy typemap removals and upstream prelink fixes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1ddbe25 to
d52cab0
Compare
Exercise the supported typemap validation target instead of deleted build targets. Remove obsolete Cecil JCW/XML scanning from the no-shrink pipeline, retaining compatibility fixups and assembly copying, and update managed-typemap regression coverage. Isolate relocation-test builds from persistent compiler and MSBuild servers without weakening move, incremental-build, or signing assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Integrate main through c35205f, including PR #12976's collection-export and explicit-constructor JNI regressions. Resolve conflicts by retaining the deleted legacy scanners and unconditional assembly copying; preserve both managed typemap and JCW/Dex multi-TFM assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@dalexsoto review |
dalexsoto
left a comment
dalexsoto
left a comment
There was a problem hiding this comment.
The prior CoreCLR additional-provider finding is resolved. The deliberate LLVM-IR removal, unsupported marshal-method rejection and retained compatibility symbols are acceptable. The complete 67-file review and separate integration/completeness pass identify one conditional native-linking build blocker.
Filter or deduplicate typemap candidates before building the P/Invoke ABI dictionaries (GenerateNativeMarshalMethodSources.cs:163-174). With _AndroidEnableNativeRuntimeLinking=true, a trimmed CoreCLR build supplies both the final linked/ReadyToRun typemap assembly and its pre-trim copy to ScanPInvokes(). Final typemaps are published by CoreCLR.targets:317-329 and included in resolved assemblies by AssemblyResolution.targets:268-271; Trimmable.targets:419-430 additionally appends the pre-trim typemaps for the first ABI. Common.targets:1661-1666 passes that combined collection to the changed task.
Both copies of _Microsoft.Android.TypeMaps.dll, for example, have the same assembly-name key for arm64-v8a. GetPerArchAssemblies() calls Dictionary.Add for each input (MonoAndroidHelper.cs:758-768), so it throws before the subsequent framework-only filtering can exclude these non-framework candidates. validateCollection:false does not disable duplicate-key rejection. Native source generation therefore fails for this opt-in configuration. This is a concrete replacement-scanner/item-flow defect, not just the older request for more ABI test coverage, and does not require restoring any legacy LLVM behavior.
Exclude pre-trim/non-framework candidates before constructing the per-ABI dictionaries, or deduplicate by ABI and assembly name while preferring the final linked/ReadyToRun copy. Add a trimmed CoreCLR native-linking regression using the actual combined typemap inputs. The evidence comes from immutable task/target declarations and item-identity comparison; no MSBuild reproduction or repository tests were executed during this review.
Exclude non-framework assemblies through GetPerArchAssemblies' existing predicate before Dictionary.Add can encounter final/pre-trim typemap copies with the same ABI and name. Preserve framework-only scanning and per-ABI native symbols without changing packaging or native-config item collections. Add linked/R2R task regressions with duplicate root/per-assembly typemap candidates, per-ABI framework PInvokes and ignored user PInvokes, plus trimmed multi-RID source-generation integration coverage. Both original task cases reproduced the duplicate _Microsoft.Android.TypeMaps key; both repaired production-source task cases pass. Full SDK integration tests require unavailable local Android build output. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@dalexsoto Fixed the native-linking blocker in 872e037307.
Added two task regressions covering linked and R2R candidates for arm64/x64, duplicated root/per-assembly typemaps, separate per-ABI framework P/Invoke symbols, and exclusion of user/typemap P/Invokes. Both cases reproduce the original Also added |
Compile defaults to DesignTimeBuild=true, which suppresses trimmable manifest generation before the explicit native-source target. Set DesignTimeBuild=false for this source-generation-only test invocation, preserving the linked/R2R and combined multi-ABI typemap assertions. Verified both old invocations select design-time mode and both corrected invocations retain normal mode/stamp paths using the production Xamarin.Android.DesignTime.targets. Full Android SDK integration remains unavailable locally. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Scope Filename, Abi and _AndroidPreTrimTypeMapCandidate to _ResolvedAssemblies in the combined-typemap assertion. Missing optional metadata on resource-designer/framework/user items now evaluates per source item instead of raising MSB4096 during batching. Ran the exact embedded assertion with linked and R2R item fixtures: reproduced both CI candidate-metadata errors, verified all three root selections with optional metadata absent, and confirmed six missing-root cases remain rejected. Full Android integration coverage still requires local SDK output unavailable here. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
@dalexsoto review |
dalexsoto
left a comment
dalexsoto
left a comment
There was a problem hiding this comment.
The complete current cleanup and replacement paths are consistent with the accepted trimmable/NativeAOT design. The native-linking duplicate-key blocker is fixed by excluding non-framework inputs before ABI dictionary construction; supported provider, packaging, native configuration and compatibility-symbol paths remain integrated. No remaining high-confidence blockers found.
Stacked on #12887; base:
simonrozsival-default-trimmable-typemap.Summary
LinkDescription,LinkerDumpDependencies, and incremental file tracking. RejectAndroidEnableMarshalMethods=truewithXA1049rather than accepting an unsupported path.The default property and
XA4265validation remain in the small bottom PR. JVM-only test cleanup is in #12894; this PR does not replay the old bottom branch's broad CI and test-fixture changes.Validation
generator.dll; CI must validate the combined path.