Skip to content

Commit 4f325e1

Browse files
committed
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DaK9eAAF5G8wj6UT8VekAm
1 parent 1966cb3 commit 4f325e1

3 files changed

Lines changed: 132 additions & 20 deletions

File tree

.changeset/gentle-pandas-learn.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
"cmake-rn": minor
3+
---
4+
5+
Let a consumer override the Android `ANDROID_STL` CMake cache variable via
6+
the existing `-D`/`--define` option (e.g. `--define ANDROID_STL=c++_static`).
7+
It still defaults to `c++_shared`, matching what React Native itself uses,
8+
but an addon that must match a prebuilt third-party dependency's STL, or one
9+
that's genuinely self-contained, can now ask for a different value.
10+
11+
This also fixes an ordering bug where a `--define` targeting any of the
12+
Android platform's own default CMake variables (including `ANDROID_STL`) was
13+
silently discarded: our hardcoded defaults were appended to the CMake
14+
command line _after_ the user-provided `-D` arguments, and CMake resolves a
15+
cache variable set multiple times via `-D` to its last occurrence.
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import assert from "node:assert/strict";
2+
import { describe, it } from "node:test";
3+
4+
import { toDefineArguments } from "../helpers.js";
5+
import { buildCommonDefinitions } from "./android.js";
6+
7+
function baseArgs(
8+
overrides: Partial<Parameters<typeof buildCommonDefinitions>[0]> = {},
9+
) {
10+
return {
11+
configuration: "Release" as const,
12+
ndkPath: "/opt/ndk",
13+
androidSdkVersion: "24",
14+
ccachePath: null,
15+
define: [],
16+
...overrides,
17+
};
18+
}
19+
20+
describe("buildCommonDefinitions", () => {
21+
it("defaults ANDROID_STL to c++_shared", () => {
22+
const args = toDefineArguments(buildCommonDefinitions(baseArgs()));
23+
const index = args.indexOf("-D");
24+
assert(index >= 0);
25+
assert(args.includes("ANDROID_STL=c++_shared"));
26+
});
27+
28+
it("lets a consumer override ANDROID_STL via --define", () => {
29+
// CMake resolves a cache variable passed multiple times via `-D` to its
30+
// *last* occurrence on the command line, so what matters is which
31+
// ANDROID_STL entry comes last - not merely that c++_static is present.
32+
const args = toDefineArguments(
33+
buildCommonDefinitions(
34+
baseArgs({ define: [{ ANDROID_STL: "c++_static" }] }),
35+
),
36+
);
37+
const stlEntries = args.filter((arg) => arg.startsWith("ANDROID_STL="));
38+
assert.deepEqual(stlEntries, [
39+
"ANDROID_STL=c++_shared",
40+
"ANDROID_STL=c++_static",
41+
]);
42+
});
43+
44+
it("applies the user's --define after (so it wins over) every default", () => {
45+
const definitions = buildCommonDefinitions(
46+
baseArgs({ define: [{ ANDROID_STL: "c++_static" }] }),
47+
);
48+
// The user-provided define must be the last entry, since CMake resolves
49+
// a -D variable passed multiple times to its last occurrence.
50+
assert.deepEqual(definitions.at(-1), { ANDROID_STL: "c++_static" });
51+
});
52+
53+
it("includes ccache launcher variables when a ccache path is given", () => {
54+
const args = toDefineArguments(
55+
buildCommonDefinitions(baseArgs({ ccachePath: "/usr/bin/ccache" })),
56+
);
57+
assert(args.includes("CMAKE_C_COMPILER_LAUNCHER=/usr/bin/ccache"));
58+
assert(args.includes("CMAKE_CXX_COMPILER_LAUNCHER=/usr/bin/ccache"));
59+
});
60+
});

packages/cmake-rn/src/platforms/android.ts

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,56 @@ function getNdkLlvmBinPath(ndkPath: string) {
9191
return path.join(prebuiltPath, platforms[0], "bin");
9292
}
9393

94+
const DEFAULT_ANDROID_STL = "c++_shared";
95+
96+
/**
97+
* Builds the list of CMake cache variable definitions common to every
98+
* triplet's configure step.
99+
*
100+
* `define` (populated from the repeatable `-D`/`--define` CLI option) is
101+
* spread last, so a consumer's explicit `-D ANDROID_STL=c++_static` (or any
102+
* other variable set here by default) takes precedence over our own
103+
* defaults: CMake resolves a cache variable passed multiple times via `-D`
104+
* to its last occurrence on the command line.
105+
*/
106+
export function buildCommonDefinitions({
107+
configuration,
108+
ndkPath,
109+
androidSdkVersion,
110+
ccachePath,
111+
define,
112+
}: {
113+
configuration: BaseOpts["configuration"];
114+
ndkPath: string;
115+
androidSdkVersion: string;
116+
ccachePath: BaseOpts["ccachePath"];
117+
define: BaseOpts["define"];
118+
}) {
119+
return [
120+
{
121+
CMAKE_BUILD_TYPE: configuration,
122+
CMAKE_SYSTEM_NAME: "Android",
123+
// "CMAKE_INSTALL_PREFIX": installPath,
124+
CMAKE_MAKE_PROGRAM: "ninja",
125+
ANDROID_NDK: ndkPath,
126+
ANDROID_TOOLCHAIN: "clang",
127+
ANDROID_PLATFORM: androidSdkVersion,
128+
// Defaults to c++_shared, matching what React Native itself uses.
129+
// Override with -D/--define ANDROID_STL=c++_static (or another value
130+
// accepted by the NDK's CMake toolchain) when an addon must match a
131+
// prebuilt third-party dependency's STL.
132+
ANDROID_STL: DEFAULT_ANDROID_STL,
133+
},
134+
ccachePath
135+
? {
136+
CMAKE_C_COMPILER_LAUNCHER: ccachePath,
137+
CMAKE_CXX_COMPILER_LAUNCHER: ccachePath,
138+
}
139+
: {},
140+
...define,
141+
];
142+
}
143+
94144
export const platform: Platform<Triplet[], AndroidOpts> = {
95145
id: "android",
96146
name: "Android",
@@ -140,26 +190,13 @@ export const platform: Platform<Triplet[], AndroidOpts> = {
140190
const ndkPath = getNdkPath(ndkVersion);
141191
const toolchainPath = getNdkToolchainPath(ndkPath);
142192

143-
const commonDefinitions = [
144-
...define,
145-
{
146-
CMAKE_BUILD_TYPE: configuration,
147-
CMAKE_SYSTEM_NAME: "Android",
148-
// "CMAKE_INSTALL_PREFIX": installPath,
149-
CMAKE_MAKE_PROGRAM: "ninja",
150-
ANDROID_NDK: ndkPath,
151-
ANDROID_TOOLCHAIN: "clang",
152-
ANDROID_PLATFORM: androidSdkVersion,
153-
// TODO: Make this configurable
154-
ANDROID_STL: "c++_shared",
155-
},
156-
ccachePath
157-
? {
158-
CMAKE_C_COMPILER_LAUNCHER: ccachePath,
159-
CMAKE_CXX_COMPILER_LAUNCHER: ccachePath,
160-
}
161-
: {},
162-
];
193+
const commonDefinitions = buildCommonDefinitions({
194+
configuration,
195+
ndkPath,
196+
androidSdkVersion,
197+
ccachePath,
198+
define,
199+
});
163200

164201
await Promise.all(
165202
triplets.map(async ({ triplet, spawn }) => {

0 commit comments

Comments
 (0)