Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/loud-dodos-kneel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"cmake-rn": minor
---

Derive default targets from the CMAKE_RN_TARGETS environment variable
5 changes: 5 additions & 0 deletions .changeset/loud-paths-eat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"ferric-cli": minor
---

Derive default targets from the FERRIC_TARGETS environment variable
6 changes: 6 additions & 0 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ jobs:
- run: rustup target add x86_64-linux-android aarch64-linux-android armv7-linux-androideabi i686-linux-android aarch64-apple-ios-sim
- run: npm ci
- run: npm run bootstrap
env:
CMAKE_RN_TARGETS: arm64-apple-ios-sim
FERRIC_TARGETS: aarch64-apple-ios-sim
Comment on lines +79 to +80
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason why Ferric handles the build of aarch64 while cmake-rn handles arm64?

Copy link
Collaborator Author

@kraenhansen kraenhansen Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a naming difference between Xcode and the Rust compiler targets.

- run: npm run pod-install
working-directory: apps/test-app
- name: Run tests (iOS)
Expand Down Expand Up @@ -103,6 +106,9 @@ jobs:
- run: rustup target add x86_64-linux-android aarch64-linux-android armv7-linux-androideabi i686-linux-android aarch64-apple-ios-sim
- run: npm ci
- run: npm run bootstrap
env:
CMAKE_RN_TARGETS: i686-linux-android
FERRIC_TARGETS: i686-linux-android
- name: Clone patched Hermes version
shell: bash
run: |
Expand Down
45 changes: 39 additions & 6 deletions packages/cmake-rn/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,23 @@ const configurationOption = new Option("--configuration <configuration>")
// TODO: Derive default targets
// This is especially important when driving the build from within a React Native app package.

const targetOption = new Option(
"--target <target...>",
"Targets to build for"
).choices(allTargets);
const { CMAKE_RN_TARGETS } = process.env;

const defaultTargets = CMAKE_RN_TARGETS ? CMAKE_RN_TARGETS.split(",") : [];

for (const target of defaultTargets) {
Copy link

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The target validation logic is duplicated between cmake-rn and ferric packages. Consider extracting this validation into a shared utility function to maintain consistency and reduce code duplication.

Copilot uses AI. Check for mistakes.
assert(
(allTargets as string[]).includes(target),
`Unexpected target in CMAKE_RN_TARGETS: ${target}`
);
}

const targetOption = new Option("--target <target...>", "Targets to build for")
.choices(allTargets)
.default(
defaultTargets,
"CMAKE_RN_TARGETS environment variable split by ','"
);
Comment on lines +55 to +60
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I last used [email protected], when invoking the cmake-rn and cmake-rn --android --ios commands, the CLI didn't prompt me for what targets to build. Is that still the case, or has something changed recently?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's another fallback default if no targets are provided through the CLI runtime options. The reason that is not handled here is because I wanted to print a warning in that case.


const buildPathOption = new Option(
"--build <path>",
Expand Down Expand Up @@ -148,16 +161,19 @@ program = program.action(
});

// Configure every triplet project
const targetsSummary = chalk.dim(
`(${getTargetsSummary(targetContexts)})`
);
await oraPromise(
Promise.all(
targetContexts.map(({ platform, ...context }) =>
configureProject(platform, context, baseOptions)
)
),
{
text: "Configuring projects",
text: `Configuring projects ${targetsSummary}`,
isSilent: baseOptions.verbose,
successText: "Configured projects",
successText: `Configured projects ${targetsSummary}`,
failText: ({ message }) => `Failed to configure projects: ${message}`,
}
);
Expand Down Expand Up @@ -208,6 +224,23 @@ program = program.action(
}
);

function getTargetsSummary(
targetContexts: { target: string; platform: Platform }[]
) {
const targetsPerPlatform: Record<string, string[]> = {};
for (const { target, platform } of targetContexts) {
if (!targetsPerPlatform[platform.id]) {
targetsPerPlatform[platform.id] = [];
}
targetsPerPlatform[platform.id].push(target);
}
return Object.entries(targetsPerPlatform)
.map(([platformId, targets]) => {
return `${platformId}: ${targets.join(", ")}`;
})
.join(" / ");
}

function getBuildPath({ build, source }: BaseOpts) {
// TODO: Add configuration (debug vs release)
return path.resolve(process.cwd(), build || path.join(source, "build"));
Expand Down
21 changes: 19 additions & 2 deletions packages/ferric/src/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
prettyPath,
} from "react-native-node-api";

import { UsageError } from "./errors.js";
import { UsageError, assertFixable } from "./errors.js";
import { ensureCargo, build } from "./cargo.js";
import {
ALL_TARGETS,
Expand Down Expand Up @@ -62,9 +62,26 @@ const ANDROID_TRIPLET_PER_TARGET: Record<AndroidTargetName, AndroidTriplet> = {
const DEFAULT_NDK_VERSION = "27.1.12297006";
const ANDROID_API_LEVEL = 24;

const { FERRIC_TARGETS } = process.env;

function getDefaultTargets() {
Comment on lines +65 to +67
Copy link

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The environment variable extraction is placed at module level but only used within getDefaultTargets(). Consider moving this extraction inside the function to improve encapsulation and make the dependency more explicit.

Suggested change
const { FERRIC_TARGETS } = process.env;
function getDefaultTargets() {
function getDefaultTargets() {
const { FERRIC_TARGETS } = process.env;

Copilot uses AI. Check for mistakes.
const result = FERRIC_TARGETS ? FERRIC_TARGETS.split(",") : [];
for (const target of result) {
assertFixable(
(ALL_TARGETS as readonly string[]).includes(target),
`Unexpected target in FERRIC_TARGETS: ${target}`,
{
instructions:
"Pass only valid targets via FERRIC_TARGETS (or remove them)",
}
);
}
return result as (typeof ALL_TARGETS)[number][];
}

const targetOption = new Option("--target <target...>", "Target triple")
.choices(ALL_TARGETS)
.default([]);
.default(getDefaultTargets());
const appleTarget = new Option("--apple", "Use all Apple targets");
const androidTarget = new Option("--android", "Use all Android targets");
const ndkVersionOption = new Option(
Expand Down
Loading