From 1ad025ceec4ed76e2c9af3e122128873832c0aae Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 07:32:00 +0000 Subject: [PATCH] fix(cmake-rn): let ANDROID_STL be overridden via --define `ANDROID_STL` was hardcoded to `c++_shared` when configuring Android builds, with no escape hatch for an addon that needs `c++_static` or must match a prebuilt third-party dependency's STL (#418). The generic `-D`/`--define` cache-variable pass-through (added for #332, which #227 also asks for) already lets a consumer set arbitrary CMake cache variables, including `ANDROID_STL` - but it didn't actually work: our hardcoded Android defaults were appended to the CMake command line *after* the user-provided `-D` arguments, and CMake resolves a variable set multiple times via `-D` to its last occurrence, so the hardcoded value always won. Fix the ordering so the user's `--define` is applied last. `ANDROID_STL` still defaults to `c++_shared`, matching what React Native itself uses. Extract the CMake definitions building into an exported `buildCommonDefinitions` and add unit tests covering the default and the override precedence. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm --- .changeset/gentle-pandas-learn.md | 15 ++++ .../cmake-rn/src/platforms/android.test.ts | 60 +++++++++++++++ packages/cmake-rn/src/platforms/android.ts | 77 ++++++++++++++----- 3 files changed, 132 insertions(+), 20 deletions(-) create mode 100644 .changeset/gentle-pandas-learn.md create mode 100644 packages/cmake-rn/src/platforms/android.test.ts diff --git a/.changeset/gentle-pandas-learn.md b/.changeset/gentle-pandas-learn.md new file mode 100644 index 00000000..f291a8cd --- /dev/null +++ b/.changeset/gentle-pandas-learn.md @@ -0,0 +1,15 @@ +--- +"cmake-rn": minor +--- + +Let a consumer override the Android `ANDROID_STL` CMake cache variable via +the existing `-D`/`--define` option (e.g. `--define ANDROID_STL=c++_static`). +It still defaults to `c++_shared`, matching what React Native itself uses, +but an addon that must match a prebuilt third-party dependency's STL, or one +that's genuinely self-contained, can now ask for a different value. + +This also fixes an ordering bug where a `--define` targeting any of the +Android platform's own default CMake variables (including `ANDROID_STL`) was +silently discarded: our hardcoded defaults were appended to the CMake +command line _after_ the user-provided `-D` arguments, and CMake resolves a +cache variable set multiple times via `-D` to its last occurrence. diff --git a/packages/cmake-rn/src/platforms/android.test.ts b/packages/cmake-rn/src/platforms/android.test.ts new file mode 100644 index 00000000..88a4011e --- /dev/null +++ b/packages/cmake-rn/src/platforms/android.test.ts @@ -0,0 +1,60 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; + +import { toDefineArguments } from "../helpers.js"; +import { buildCommonDefinitions } from "./android.js"; + +function baseArgs( + overrides: Partial[0]> = {}, +) { + return { + configuration: "Release" as const, + ndkPath: "/opt/ndk", + androidSdkVersion: "24", + ccachePath: null, + define: [], + ...overrides, + }; +} + +describe("buildCommonDefinitions", () => { + it("defaults ANDROID_STL to c++_shared", () => { + const args = toDefineArguments(buildCommonDefinitions(baseArgs())); + const index = args.indexOf("-D"); + assert(index >= 0); + assert(args.includes("ANDROID_STL=c++_shared")); + }); + + it("lets a consumer override ANDROID_STL via --define", () => { + // CMake resolves a cache variable passed multiple times via `-D` to its + // *last* occurrence on the command line, so what matters is which + // ANDROID_STL entry comes last - not merely that c++_static is present. + const args = toDefineArguments( + buildCommonDefinitions( + baseArgs({ define: [{ ANDROID_STL: "c++_static" }] }), + ), + ); + const stlEntries = args.filter((arg) => arg.startsWith("ANDROID_STL=")); + assert.deepEqual(stlEntries, [ + "ANDROID_STL=c++_shared", + "ANDROID_STL=c++_static", + ]); + }); + + it("applies the user's --define after (so it wins over) every default", () => { + const definitions = buildCommonDefinitions( + baseArgs({ define: [{ ANDROID_STL: "c++_static" }] }), + ); + // The user-provided define must be the last entry, since CMake resolves + // a -D variable passed multiple times to its last occurrence. + assert.deepEqual(definitions.at(-1), { ANDROID_STL: "c++_static" }); + }); + + it("includes ccache launcher variables when a ccache path is given", () => { + const args = toDefineArguments( + buildCommonDefinitions(baseArgs({ ccachePath: "/usr/bin/ccache" })), + ); + assert(args.includes("CMAKE_C_COMPILER_LAUNCHER=/usr/bin/ccache")); + assert(args.includes("CMAKE_CXX_COMPILER_LAUNCHER=/usr/bin/ccache")); + }); +}); diff --git a/packages/cmake-rn/src/platforms/android.ts b/packages/cmake-rn/src/platforms/android.ts index da5f19c2..5c8b16ae 100644 --- a/packages/cmake-rn/src/platforms/android.ts +++ b/packages/cmake-rn/src/platforms/android.ts @@ -91,6 +91,56 @@ function getNdkLlvmBinPath(ndkPath: string) { return path.join(prebuiltPath, platforms[0], "bin"); } +const DEFAULT_ANDROID_STL = "c++_shared"; + +/** + * Builds the list of CMake cache variable definitions common to every + * triplet's configure step. + * + * `define` (populated from the repeatable `-D`/`--define` CLI option) is + * spread last, so a consumer's explicit `-D ANDROID_STL=c++_static` (or any + * other variable set here by default) takes precedence over our own + * defaults: CMake resolves a cache variable passed multiple times via `-D` + * to its last occurrence on the command line. + */ +export function buildCommonDefinitions({ + configuration, + ndkPath, + androidSdkVersion, + ccachePath, + define, +}: { + configuration: BaseOpts["configuration"]; + ndkPath: string; + androidSdkVersion: string; + ccachePath: BaseOpts["ccachePath"]; + define: BaseOpts["define"]; +}) { + return [ + { + CMAKE_BUILD_TYPE: configuration, + CMAKE_SYSTEM_NAME: "Android", + // "CMAKE_INSTALL_PREFIX": installPath, + CMAKE_MAKE_PROGRAM: "ninja", + ANDROID_NDK: ndkPath, + ANDROID_TOOLCHAIN: "clang", + ANDROID_PLATFORM: androidSdkVersion, + // Defaults to c++_shared, matching what React Native itself uses. + // Override with -D/--define ANDROID_STL=c++_static (or another value + // accepted by the NDK's CMake toolchain) when an addon must match a + // prebuilt third-party dependency's STL. + ANDROID_STL: DEFAULT_ANDROID_STL, + }, + ccachePath + ? { + CMAKE_C_COMPILER_LAUNCHER: ccachePath, + CMAKE_CXX_COMPILER_LAUNCHER: ccachePath, + } + : {}, + ...define, + ]; +} + export const platform: Platform = { id: "android", name: "Android", @@ -140,26 +190,13 @@ export const platform: Platform = { const ndkPath = getNdkPath(ndkVersion); const toolchainPath = getNdkToolchainPath(ndkPath); - const commonDefinitions = [ - ...define, - { - CMAKE_BUILD_TYPE: configuration, - CMAKE_SYSTEM_NAME: "Android", - // "CMAKE_INSTALL_PREFIX": installPath, - CMAKE_MAKE_PROGRAM: "ninja", - ANDROID_NDK: ndkPath, - ANDROID_TOOLCHAIN: "clang", - ANDROID_PLATFORM: androidSdkVersion, - // TODO: Make this configurable - ANDROID_STL: "c++_shared", - }, - ccachePath - ? { - CMAKE_C_COMPILER_LAUNCHER: ccachePath, - CMAKE_CXX_COMPILER_LAUNCHER: ccachePath, - } - : {}, - ]; + const commonDefinitions = buildCommonDefinitions({ + configuration, + ndkPath, + androidSdkVersion, + ccachePath, + define, + }); await Promise.all( triplets.map(async ({ triplet, spawn }) => {