fix(map): show a message instead of crashing when a device has no map engine, and fail CI on the gap - #7005
Conversation
… engine MapLibre Compose reaches its renderer through a JNI shim, and an app can be built for an architecture that shim was never published for. maplibre-native-ffi 0.202608.3 ships arm64-v8a and x86_64 only, so the armeabi-v7a split carries no engine and the first frame of a map takes the whole app down with UnsatisfiedLinkError (#7001). Ask whether the engine can load before composing a map, and render a short explanation when it cannot. The probe loads the JNI shim, whose DT_NEEDED pulls the 13 MB engine in behind it, so one load answers for both; System.loadLibrary is process-wide and idempotent, so MapLibre's own load later finds it done. Guarded at all three points that compose MaplibreMap: the provider, so the toolbar and zoom controls do not float over an empty screen, and MeshMap and SecondaryMapSurface, which reach it directly. Off Android every published artifact carries its own engine, so those actuals answer true and nothing changes. This does not restore the map on 32-bit devices — only an upstream release with armeabi-v7a can (see #7003). It stops a missing renderer from costing the user every other screen in the app.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change adds MapLibre runtime detection with a localized fallback UI and adds Android ABI parity checks. Build workflows run the parity verifier and self-test. Tests cover runtime probing, fallback rendering, UI behavior, and screenshot previews. ChangesMapLibre runtime fallback
ABI parity validation
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The PR replaces a map-engine crash with a map-only fallback and adds CI checks for missing native libraries; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant MapScreen
participant LocalMapLibreRuntimeProbe
participant MapEngineUnavailable
MapScreen->>LocalMapLibreRuntimeProbe: Check runtime availability
LocalMapLibreRuntimeProbe-->>MapScreen: Return availability
MapScreen->>MapEngineUnavailable: Render fallback when unavailable
sequenceDiagram
participant AndroidBuild
participant verify_abi_parity
participant check_abi_parity
participant ABISplitAPKs
AndroidBuild->>verify_abi_parity: Run after APK assembly
verify_abi_parity->>ABISplitAPKs: Collect per-ABI APKs
verify_abi_parity->>check_abi_parity: Compare native libraries
check_abi_parity-->>verify_abi_parity: Return parity results
verify_abi_parity-->>AndroidBuild: Report success or failure
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
Full details: Sibling Call Sites And Presence SemanticsExplanation PASS. The PR does not change any metric field representation, nullable declaration, or zero-guard. The PR diff contains no changes to NodeItem.kt or NodeItemCompact.kt. Both files already use nullable presence checks for ambient temperature; NodeItemCompact.kt does not retain an ambient-temperature zero-guard. The only new presence-style behavior is the MapLibre runtime probe. All three probe call sites are updated, and the only two direct MaplibreMap invocations are behind those guards. No new RSSI, temperature, current, voltage, or particulate field defaults to 0. Full details: Tests Prove The Path, Not The End StateExplanation The ABI wrapper self-test has one non-diagnostic case. Resolution Make the universal fixture contain a library that is absent from both ABI splits, such as Full details: Regression Coverage For Changed BehaviorExplanation Coverage gaps remain in the new MapLibre fallback paths. Resolution Add these regression tests: - Compose Full details: Moved Code Diffed Against Its OriginalExplanation PASS — The PR range adds new files and guards. It does not delete or relocate an existing type or function. The only extraction-like change replaces
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ings A dependency published for only some of our ABIs builds and installs cleanly on the rest and dies with UnsatisfiedLinkError the first time it is touched. Nothing at build time says a word. That is #7001: maplibre-native-ffi 0.202608.3 ships arm64-v8a and x86_64 only, so the F-Droid armeabi-v7a split had no map engine and opening the map took the app down. Compare the split APKs in android-check, where every PR already builds them. The reference set is the union across splits — a library any ABI ships is one the app expects to load — and each split must carry all of it unless a known-gap line in scripts/lib/abi-parity.sh says otherwise. Known gaps are checked both ways. The library must be absent, and the check fails once it turns up, so an entry cannot outlive its reason: the maplibre-compose bump that closes this one goes red until its two lines are deleted with it. Without that, Renovate would land the bump and nothing would mark #7001 closed. The two lines recorded now are the MapLibre gap on fdroid/armeabi-v7a. The 32-bit build upstream merged on 2026-08-24 (maplibre-native-ffi #658, #659, #660) has no release yet; on those devices the app shows a message instead of a map meanwhile. A fixture self-test runs in lint-check, mirroring verify-rb-selftest.sh: the real check only ever sees what the current dependencies happen to ship, so both failure modes would otherwise go unexercised until they bit.
jamesarich
changed the title
|
…apper Review pointed out that the screenshot test renders the fallback directly, so it would still pass with all three runtime guards removed, and that the parity self-test never invokes the wrapper whose exit status CI acts on. Give the probe a composition-local seam so a test can declare the engine missing without a device that lacks it, and factor the load into probeNativeRuntime so the one property that matters — an UnsatisfiedLinkError is an Error, not an Exception, and must come back as false rather than propagate — is asserted directly. The SecondaryMapSurface test counts compositions of its content: zero when the engine is missing, and removing the guard fails it with "No ComposeMapHost is installed", which is MapLibre being reached. MeshMap and the provider carry the identical one-line guard but need a seven-repository view model and Koin to compose, so they are not harnessed here. The fallback screen gets a component test for both strings and for having nothing to tap. The self-test grows seven cases that run verify-abi-parity.sh against fixture output trees: matching splits with a universal APK, an unrecorded gap, the checked-in fdroid gap held and then closed, a lone split, an empty directory, and failures in two flavors summing in the exit status.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Only visible in the merge group: #7005's engine-probe guard on main plus the offline indicator here put MapView at 61 lines against a limit of 60, so the queue run failed detekt while both sides passed alone. Inline the layer-opacity collect into the MeshMap argument to get the line back.
1 participant
Why
MapLibre Compose reaches its renderer through a JNI shim, and an app can be built for an architecture that shim was never published for.
maplibre-native-ffi 0.202608.3publishes arm64-v8a and x86_64 only, so the F-Droidarmeabi-v7asplit carries no engine — 18.2 MB against the arm64 split's 32.5 MB, with CI green — and the first frame of a map takes the whole app down:Losing the map on those devices is unavoidable until upstream ships a 32-bit build. Losing every other screen is not, and neither is shipping the gap without knowing. Two commits:
Relates to #7001.
🐛 Bug Fixes
isMapLibreRuntimeAvailable()— anexpect/actualprobe. Android loads the JNI shim insiderunCatching;UnsatisfiedLinkErroris anError, so acatch (e: Exception)would have missed it and kept the crash.MaplibreMap:MapLibreMapViewProvider(so the toolbar and zoom controls do not float over an empty screen driving a map that is not there), plusMeshMapandSecondaryMapSurface, which reach it directly.MapEngineUnavailable— a deliberately actionless screen. There is nothing a user can do about their phone's architecture, so it states what happened, states the rest of the app is fine, and stops.🧹 Chores
scripts/lib/abi-parity.sh— the classifier. The reference set is the union of native libs across a flavor's split APKs; every split must carry all of it unless a known-gap line says otherwise.scripts/verify-abi-parity.sh— runs inandroid-checkright after the debug assemble, where every PR already builds the splits.scripts/verify-abi-parity-selftest.sh— fixture self-test inlint-check, mirroringverify-rb-selftest.sh.scripts/sort-strings.py;strings-index.txtregenerated.Notes for review
fdroid/armeabi-v7a. An entry must be absent from its split, and the check fails once the library turns up, so an entry cannot outlive its reason. Renovate tracks the 8org.maplibre.composecatalog entries and will bumpmaplibre-composewhen a release with the 32-bit build lands (upstream merged it 2026-08-24: maplibre-native-ffi #658, #659, #660; no release yet). Without this, that bump lands green and nothing marks [Bug]: v2.8.2-closed.1 (29322131) map tab crash the FDroid app #7001 closed. With it, the bump goes red with "the gap has closed, delete those lines" — and deleting them is the proof the v7a APK really has an engine.libjniMaplibreNativeC.so'sDT_NEEDEDlistslibmaplibre-native-c.so, so one load answers for both.System.loadLibraryis process-wide and idempotent — MapLibre's own load later finds it already done.trueunconditionally.rb-checkassembles once and would never have noticed.Screenshots
Testing Performed
Runtime guard
MapLibreRuntimeTest—probeNativeRuntimereturnsfalsefor a thrownUnsatisfiedLinkError(anError, which is the whole point) andtruefor a load that returns;SecondaryMapSurfaceunder aLocalMapLibreRuntimeProbethat says no composes the fallback and its content zero times. Mutation-checked: deleting the guard fails the test withNo ComposeMapHost is installed— MapLibre being reached.MeshMapand the provider carry the identical one-line guard but need a seven-repository view model and Koin to compose, so they are not harnessed.MapEngineUnavailableTest— both strings displayed, andonAllNodes(hasClickAction())is empty.ScreenshotMapEngineUnavailableadded toMapScreenshotTests, light and dark references committed;:screenshot-tests:validateDebugScreenshotTestgreen.maplibre-compose: Rendered the first map frame with OPENGL, no fallback in the view tree.spotlessApply spotlessCheck detekt assembleDebug test allTests kmpSmokeCompile. detekt'sLongMethodcaught the first draft of the guard at 63 lines against a 60 limit, hence the single-line early return.Parity check, run against three real APK sets:
v2.8.2-closed.1fdroid splits + google splitsverify-abi-parity.shitself against fixture output trees (universal APK ignored, unrecorded gap, the checked-in gap held and then closed, a lone split, an empty directory, two failing flavors summing in the exit status).shellcheck -xoverscripts/(the lint-check invocation) andactionlinton both workflows: clean.Not verified: execution on real 32-bit ARM hardware. armeabi-v7a emulator images stop at API 25 and
minSdkis 26, and the only device here is arm64-only — the probe-fail path was exercised by removing the libraries rather than by running on a v7a phone.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes