Enable CoreCLR runtime pack for android-arm (armeabi-v7a)#11172
Conversation
There was a problem hiding this comment.
Pull request overview
Enables CoreCLR runtime-pack support metadata for the android-arm (armeabi-v7a) ABI so the Android workload can include a CoreCLR pack for that RID, matching existing arm64/x64 patterns.
Changes:
- Add
Microsoft.Android.Runtime.CoreCLR.36.1.android-armto the Android workload manifest packs list and pack definitions. - Update
Ndk.projitems.into markarmeabi-v7aasSupportCoreCLR=Trueand add anApiLevelNonMonovalue for that ABI. - Extend xaprepare placeholder generation to replace the new
@NDK_ARMEABI_V7_API_NON_MONO@placeholder.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/Xamarin.Android.Build.Tasks/Microsoft.NET.Sdk.Android/WorkloadManifest.in.json |
Adds the CoreCLR android-arm runtime pack to the workload manifest. |
build-tools/xaprepare/xaprepare/Steps/Step_GenerateFiles.cs |
Adds placeholder substitution for the new arm32 non-mono NDK API placeholder used by generated projitems. |
build-tools/scripts/Ndk.projitems.in |
Marks armeabi-v7a as CoreCLR-supported and introduces an arm32 non-mono API-level property/metadata. |
|
@copilot address the review and make sure the rest of the repo is ready for armeabi-v7a support |
Wired up the remaining pieces in 84c9882:
|
|
@copilot build failed: |
... Fixed in 483822f. Root cause: on 32-bit (armeabi-v7a) The BG4304 metadata error ( |
483822f to
35be785
Compare
|
|
... Addressed in 44570a4. Root cause: the android-arm CoreCLR runtime pack ( Reverted just the |
b3881ad to
66397af
Compare
|
@copilot fix build failure: |
... Fixed in 7e27549. Root cause: PR #11331 ( Cleaned up:
Verified xaprepare builds cleanly locally. |
|
@copilot build is failing with an error we've seen before: |
|
@copilot resolve the merge conflicts in this pull request |
Resolved in 71591e3. Only conflict was in All of this PR's actual arm32 wiring (workload manifest, |
|
@copilot fix these build errors: |
Fixed in the latest commit. Root cause: an earlier commit on this branch ( Restored both submodules to |
Enable the CoreCLR runtime flavor for the armeabi-v7a (android-arm) ABI, mirroring the existing android-arm64 and android-x64 wiring. Scope is CoreCLR only; NativeAOT for android-arm remains tracked separately (#11423). - Ndk.projitems: set SupportCoreCLR=True for armeabi-v7a. This drives both the CoreCLR pack production (build-tools/create-packs) and the native runtime RID list. - native-clr.csproj: add armeabi-v7a/arm to the native CoreCLR build ABIs and replace the stale "arm64 only" comment. - CMakeLists.txt: set RUNTIME_DIR_ARM for the CoreCLR local and packaged runtime paths (it was only set for MonoVM before). - xa_build_configuration.cmake.in + cmake-config.csproj: add the CORECLR_APP_RUNTIME_DIR_ARM path and its @CORECLR_APP_RUNTIME_ANDROID_ARM@ substitution. - CMakePresets.json.in: add coreclr-default-debug/release-armeabi-v7a presets. - WorkloadManifest.in.json: add Microsoft.Android.Runtime.CoreCLR.37.android-arm to the workload pack list and pack definitions. Requires the microsoft.netcore.app.runtime.android-arm CoreCLR runtime pack from dotnet/runtime to be available end-to-end; CI may be red until that dependency flows in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
660095d to
869ed69
Compare
Add the missing Microsoft.NETCore.App.Runtime.android-arm PackageDownload to workloads.csproj. Without it the CoreCLR arm32 runtime pack is never restored into $(XAPackagesDir), so RUNTIME_DIR_ARM (set in CMakeLists.txt / cmake-config.csproj) points at a non-existent directory and the native CoreCLR arm build cannot find the runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 Android PR Review — ⚠️ Needs Changes (pending CI / external dependency)
The arm32 wiring is complete and consistent. I traced each place android-arm64/android-x64 CoreCLR is referenced and confirmed the arm32 sibling was added everywhere:
- ✅
Ndk.projitems—SupportCoreCLR=Trueforarmeabi-v7a(drives pack creation + native RID list via theSupportCoreCLRmetadata consumed inConfiguration.propsandcreate-packs/Directory.Build.targets). - ✅
workloads.csproj—Microsoft.NETCore.App.Runtime.android-armPackageDownload. - ✅
WorkloadManifest.in.json— pack listed inpacks[]and defined in thepacks{}map. - ✅
CMakeLists.txt—RUNTIME_DIR_ARMset for both the local and packaged CoreCLR paths (and it is consumed via theANDROID_ABI MATCHES "^armeabi-v7a"branch →NET_RUNTIME_DIR). - ✅
xa_build_configuration.cmake.in+cmake-config.csproj—CORECLR_APP_RUNTIME_DIR_ARMvariable +@CORECLR_APP_RUNTIME_ANDROID_ARM@substitution. - ✅
CMakePresets.json.in—coreclr-default-{debug,release}-armeabi-v7apresets, matching the existing arm64inheritspattern exactly. - ✅
create-packs/Microsoft.Android.Runtime.projalready handlesandroid-armgenerically (_ClangArch=arm), so no hardcoded ABI list needed touching. - ✅
AndroidMinimumDotNetApiLevelis already24, which satisfies CoreCLR arm32's minimum-API requirement — nothing to bump here.
Main concern (see inline)
Adding armeabi-v7a to AndroidSupportedTargetJitAbis re-enables building the CoreCLR native host for arm32, which must link libcoreclr.so from Microsoft.NETCore.App.Runtime.android-arm. If that pack isn't yet restorable at $(DotNetRuntimePacksVersion), the build fails with ld.lld: error: unable to find library -lcoreclr — the failure that recurred repeatedly in this PR's history. CI build 1491573 is still in progress at review time, so please confirm the arm32 native runtime links green before merging.
Process note
The PR description says to keep this as draft until the runtime/SDK dependencies (dotnet/runtime#127225, dotnet/runtime#126838, dotnet/sdk#54015) flow in, but the PR is not currently marked draft and mergeable_state is blocked. Consider re-marking it draft until CI is green.
Verdict
0 ❌ · 1 android-arm runtime-pack dependency right next to the ABI list is a nice touch. Not mergeable until the arm32 CoreCLR native build links successfully in CI.
Generated by Android PR Reviewer for #11172 · 293.9 AIC · ⌖ 36.5 AIC · ⊞ 6.8K
Comment /review to run again
| armeabi-v7a (android-arm) requires the microsoft.netcore.app.runtime.android-arm | ||
| pack from dotnet/runtime; x86 (android-x86) has no CoreCLR runtime pack. --> | ||
| <AndroidSupportedTargetAotAbis>armeabi-v7a:arm64:x86_64</AndroidSupportedTargetAotAbis> | ||
| <AndroidSupportedTargetJitAbis>armeabi-v7a:arm64-v8a:x86_64</AndroidSupportedTargetJitAbis> |
There was a problem hiding this comment.
🤖 armeabi-v7a here makes _BuildAndroidRuntimes configure and link the CoreCLR native host for arm32, which resolves -lcoreclr against libcoreclr.so from Microsoft.NETCore.App.Runtime.android-arm (the pack added in workloads.csproj). If that runtime pack isn't flowing yet at $(DotNetRuntimePacksVersion), _BuildAndroidRuntimes fails with ld.lld: error: unable to find library -lcoreclr — the exact failure that recurred several times earlier in this PR. Please confirm the arm32 CoreCLR runtime pack actually restores in the current dependency flow before merging (CI build 1491573 is still running at review time, so the arm32 native link is the thing to watch).
Rule: Verify the external dependency is available before enabling the build path
|
@copilot fix the following build error in C++ code: |
Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
... Fixed. Reapplied |
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 Code Review — ✅ LGTM
Issues: 0 ❌ · 0
Enables the CoreCLR runtime flavor for armeabi-v7a (android-arm), mirroring the existing arm64/x64 wiring. I reviewed all 9 changed files and traced the enablement chain end-to-end.
Verified correct & complete
Ndk.projitems—SupportCoreCLR=Trueforarmeabi-v7a; this metadata gates CoreCLR pack production increate-packs/Directory.Build.targets(_AndroidRIDs ... Runtime="CoreCLR"). ✓native-clr.csproj—armeabi-v7aadded to the JIT/AOT ABI lists. The token matches the global convention inConfiguration.props(armeabi-v7a:arm64:x86:x86_64), and...JitAbisForConditionalChecksis recomputed from the local override beforeNdk.projitemsreads it. ✓CMakeLists.txt—RUNTIME_DIR_ARMnow set in both the local and packaged CoreCLR branches; it is consumed at thearmeabi-v7aselector (set(NET_RUNTIME_DIR "${RUNTIME_DIR_ARM}")). ✓CMakePresets.json.in— newcoreclr-default-{debug,release}-armeabi-v7apresets exactly mirror the arm64 ones and match the namenative.targetsconstructs ($(_PresetPrefix)default-<config>-%(Identity)). No NativeAOT preset, consistent with the PR scope. ✓cmake-config.csproj+xa_build_configuration.cmake.in—CORECLR_APP_RUNTIME_ANDROID_ARMsubstitution follows the arm64/x64 prefix/suffix pattern; the generating target keeps correctInputs/Outputs. ✓WorkloadManifest.in.json+workloads.csproj— the CoreCLR arm pack is added to all three parallel spots (pack list, pack definition,PackageDownload). No spot missed; the SDK'sRuntimePackRuntimeIdentifiersalready listsandroid-arm. ✓typemap.cc— thereinterpret_cast<const char*>is required to compile the DEBUG typemap path on 32-bit; byte-safe, and the RELEASEmemcmppath is unaffected (see inline 💡). ✓
Notes (non-blocking)
- CI is fully green (42/42), including the
dotnet-androidAzDO checks that build native code for every ABI — strong evidence the arm32 CoreCLR host now compiles and links. - The PR description's “keep this PR as draft... CI may remain red until the dependent runtime/SDK changes flow in” now appears stale: CI is green,
AndroidMinimumDotNetApiLevelis already24(satisfying the arm32 API-24 requirement), and the dependency packs (dotnet/runtime#127225, dotnet/sdk#54015) seem to have landed. Consider updating the description / draft state before merge.
Nicely scoped and consistent with the established multi-ABI patterns. 👍
Generated by Android PR Reviewer for #11172 · 336.4 AIC · ⌖ 27.1 AIC · ⊞ 6.8K
Comment /review to run again
| full_type_name.append (typeName); | ||
|
|
||
| hash_t mvid_hash = xxhash::hash (mvid, 16z); // we must hope managed land called us with valid data | ||
| hash_t mvid_hash = xxhash::hash (reinterpret_cast<const char*>(mvid), 16z); // we must hope managed land called us with valid data |
There was a problem hiding this comment.
🤖 💡 Native C++ — The cast is correct and byte-safe, so this is not a bug. It's only needed because on 32-bit ABIs (like the newly-enabled armeabi-v7a) xxhash resolves to xxhash32, whose hash accepts only const char*, whereas the 64-bit xxhash64::hash is a template<typename T> hash(const T*, size_t) that takes const uint8_t* directly. Optional / out-of-scope: giving xxhash32::hash the same generic-pointer overload in xxhash.hh would let this (the sole const uint8_t* caller) drop the per-arch cast and keep the two implementations symmetric.
(Rule: API consistency — align xxhash32 with xxhash64)
Note
This PR was AI/Copilot-generated.
Summary
Enable the CoreCLR runtime flavor for the
armeabi-v7a(android-arm) ABI, mirroring the existingandroid-arm64andandroid-x64wiring. Scope is CoreCLR only — NativeAOT for android-arm is tracked separately in #11423.Changes
build-tools/scripts/Ndk.projitemsSupportCoreCLR=Trueforarmeabi-v7a(wasFalse). This single flag drives both the CoreCLR pack production (build-tools/create-packs) and the native runtime RID list.src/workloads/workloads.csprojPackageDownloadforMicrosoft.NETCore.App.Runtime.android-armso the CoreCLR arm32 runtime pack is restored into$(XAPackagesDir)(otherwiseRUNTIME_DIR_ARMpoints at a non-existent directory).src/native/native-clr.csprojarmeabi-v7a/armtoAndroidSupportedTargetJitAbis/AndroidSupportedTargetAotAbisso the native CoreCLR host is actually built for arm32; replace the stale "arm64 only" comment.src/native/CMakeLists.txtRUNTIME_DIR_ARMfor the CoreCLR local and packaged runtime paths (previously only set for MonoVM).build-tools/scripts/xa_build_configuration.cmake.in+src/native/cmake-config/cmake-config.csprojCORECLR_APP_RUNTIME_DIR_ARMvariable and its@CORECLR_APP_RUNTIME_ANDROID_ARM@substitution.src/native/CMakePresets.json.incoreclr-default-debug-armeabi-v7aandcoreclr-default-release-armeabi-v7apresets.WorkloadManifest.in.jsonMicrosoft.Android.Runtime.CoreCLR.37.android-armto the workload pack list and pack definitions.Status
This PR assumes the
android-armCoreCLR runtime pack is available end-to-end. CI may remain red until the dependent runtime/SDK changes flow into this repo:Keep this PR as draft while those dependencies are missing.
NativeAOT android-arm wiring is tracked separately in #11423.