fix(mobile): honor requested terminal native architectures - #10709
Conversation
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — Although the diff is small and leaves builds without You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe Android library now reads the ChangesAndroid build configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The terminal Android module now builds only requested native ABIs when configured, while retaining default ABI behavior otherwise. The change is ready to merge with minimal current risk. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Answering the approvability hold, since the concern is about ABI coverage rather than a defect in the diff. The property has exactly one producer in this repo, ...(abis.length > 0 ? [`-PreactNativeArchitectures=${abis.join(",")}`] : []),so the value is always a comma-joined list with no spaces and no empty elements. Those ABIs come from real attached devices, not a static matrix — The no-property path is provably unchanged. When no android device is captured, The shape matches the React Native template verbatim — same def reactNativeArchitectures() {
def value = project.getProperties().get("reactNativeArchitectures")
return value ? value.split(",") : ["armeabi-v7a", "arm64-v8a", "x86", "x86_64"]
}
...
ndk { abiFilters (*reactNativeArchitectures()) }RN does not trim elements either, so this does not introduce a handling difference from the framework convention. On the bug being fixed: One deliberate omission: |
What Changed
The terminal Android module now applies
reactNativeArchitecturesto its NDK ABI filters when the property is supplied. When it is absent, the module retains its existing defaults.Why
Building with
-PreactNativeArchitectures=arm64-v8astill scheduled terminal CMake builds for all four ABIs, even though the APK only needed ARM64. Honoring the requested architectures avoids this unnecessary native compilation.Validation on Windows with Gradle 9.3.1:
arm64-v8a, and exactly ARM64 and x86_64 forarm64-v8a,x86_64.llvm-readelfconfirmed the resultinglibt3terminal.sois an AArch64 shared library.git diff --checkpassed. A full APK build was not repeated for this change.Checklist
Prepared with GPT-6 in the Codex desktop app.
Note
Honor
reactNativeArchitecturesfor Android NDK ABI filters int3-terminalAdds conditional logic to build.gradle that reads the
reactNativeArchitecturesproject property and applies it as NDK ABI filters when present. Without the property, build behavior is unchanged.Macroscope summarized 6aaa461.
Summary by CodeRabbit
reactNativeArchitecturesbuild setting.