Skip to content

Emit prebuilds per target, next to their sources - #413

Draft
kraenhansen wants to merge 1 commit into
kh/multi-addon-projectsfrom
claude/pr-363-feedback-qqu312
Draft

Emit prebuilds per target, next to their sources#413
kraenhansen wants to merge 1 commit into
kh/multi-addon-projectsfrom
claude/pr-363-feedback-qqu312

Conversation

@kraenhansen

Copy link
Copy Markdown
Collaborator

Follow-up to #363, targeting kh/multi-addon-projects so the diff shows only the changes on top of it. Every change here touches code #363 introduces, so it does not stand alone against main.

This takes a run at the open question in #363"I need to figure out how to handle the final output" — and fixes a set of defects found while reading that diff.

The output question

The flat output directory breaks the consumer side in two ways, and both have to be addressed together:

  • Location. The Babel plugin resolves requires literally against the requiring file's directory (plugin.ts, and findNodeAddonForBindings in path-utils.ts), so a prebuild at the package root is invisible to it.
  • Name. getLibraryName derives the library name from the prebuild's path within its package, and the auto-linker renames the .so to lib${libraryName}.so. The plugin derives the same name from the require path. Move or rename the prebuild and the two disagree.

Both are now derived per target from the CMake File API, rather than from the target name:

  • The output directory defaults to {targetSourceDir}/build/{configuration}. The new {targetSourceDir} placeholder expands to target.paths.source, resolved against --source. A project declaring a single addon at the top level reports ., so it resolves to the same path as before.
  • The prebuild is named after the artifact on disk — the target's OUTPUT_NAME — rather than the CMake target name.

Together these reduce --namespaced-targets to a purely internal concern: it disambiguates target names within the project, and nothing downstream moves or changes name.

Grouping stays keyed by CMake target name, which CMake guarantees unique within a project; the artifact name is deliberately not used as the key, since every addon may well build an addon.node.

Verified against real CMake

Configuring the generated root project (13 sub-projects) and running the new resolver over the File API output:

async-test-addon     -> tests/async/build/RelWithDebInfo/addon.android.node
example-0-hello      -> examples/1-getting-started/1_hello_world/napi/build/RelWithDebInfo/hello.android.node
example-10-binding   -> examples/5-async-work/async_work_thread_safe_function/napi/build/RelWithDebInfo/binding.android.node
...
targets: 13 | unique outputs: 13

./build/RelWithDebInfo is on the bindings lookup path, and the basenames are the original addon names, so require('bindings')('hello') resolves again. A single-addon project was separately confirmed to report paths.source === "." and emit to exactly the path it does today.

Fixes

gyp-to-cmake

  • OUTPUT_NAME was emitted regardless of --namespaced-targets — the condition tested actualTargetName, which is always a non-empty string.
  • OUTPUT_NAME was never set for Apple framework targets, which CMake names after it. Without it the framework, and the prebuild assembled from it, was named after the namespaced target. This is the change visible in tests/*/CMakeLists.txt.

cmake-rn / Apple

  • postBuild looked for `${target name}.framework` while createAppleFramework names it after the artifact. Under namespacing these diverge and the assert fires.
  • build() ran a full cmake --build once per shared library, concurrently against the same build tree — its arguments never depended on the library.
  • listXcodeProject() is a synchronous spawn and was called once per library per triplet. Now once per build directory, and async.
  • xcodebuild invocations run in sequence per build directory; concurrent invocations against a single Xcode project and its derived data are not reliable.
  • Dropped the "Building for multiple targets is not supported yet" guard, which made --target unusable for the multi-addon case this work exists to serve.

cmake-rn / CLI

  • --concurrency accepted anything parseInt returned, passing NaN/0 into pLimit. Now rejected with a proper Commander usage error.
  • It also now defaults to 1 under --verbose, which the help text already claimed.
  • EventEmitter.defaultMaxListeners is derived from the concurrency instead of being raised to 500. Each spawned child attaches three process listeners and removes them on exit, so the live count tracks concurrent children — which pLimit already bounds.

node-addon-examples

  • verify-prebuilds globbed examples/, which the prebuilds had moved out of, so it passed by finding nothing. It now covers tests/ too and asserts a non-zero count, so that failure mode cannot recur silently.
  • The root CMakeLists.txt used file(GLOB_RECURSE ...), which is evaluated once at configure time (missing examples copied in afterwards) and, being recursive, would add_subdirectory() both a parent and a nested project — three allow-list entries in copy-examples.mts are commented out as "Brings its own CMake project 👀". It is now generated by scripts/generate-root-project.mts, which stops recursing at the first CMakeLists.txt on a path. Being generated from examples/ (gitignored), the file is now gitignored too.
  • Removed findCMakeProjects/findCMakeProjectsRecursively, dead since build-examples.mts was deleted.

Testing

Run on Linux:

  • npm run build, npx eslint ., prettier --check . — clean. (eslint reports 4 pre-existing errors in apps/test-app/App.tsx, unrelated and reproducible with these changes stashed.)
  • npm test --workspace gyp-to-cmake — 30 pass, including new --namespaced-targets coverage, of which there was none.
  • npm test --workspace cmake-rn — 11 pass, including new coverage for the output-path resolution and artifact naming.
  • npm test --workspace react-native-node-api — 43 pass, 4 fail. The 4 are permission-bit tests that cannot pass as root; nothing in packages/host is touched here.
  • Configured the full 13-project example tree with a stubbed weak-node-api config to confirm it configures without duplicate targets.

Not verified: the Apple changes. This worker is Linux, and per AGENTS.md native builds are not bootstrapped here, so the xcodebuild sequencing, the hoisted cmake --build, and the framework-path fix are reasoned from the code rather than run. The end-to-end check that matters is npm test --workspace @react-native-node-api/node-addon-examples on macOS, plus the Test app (iOS/Android) CI jobs — currently skipped on #363 because it is a draft.

Opened as a draft to match #363.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KfKQDvEkxNtkSE4aaF9yG8


Generated by Claude Code

A project declaring multiple addons wrote every prebuild into a single
output directory, named after the CMake target. Both the location and the
name are now derived per target from the CMake File API:

- The output directory defaults to {targetSourceDir}/build/{configuration},
  where the new {targetSourceDir} placeholder expands to the target's own
  source directory. A single-addon project reports "." and so resolves to
  the same path as before.
- The prebuild is named after the artifact on disk (the target's
  OUTPUT_NAME) rather than the target name, so a target renamed to avoid a
  clash within the project still produces the name the JS require expects.

Together this keeps a prebuild where the Babel plugin and auto-linking
resolve it from, and reduces --namespaced-targets to an internal concern.

Also fixes, in the same area:

- gyp-to-cmake emitted OUTPUT_NAME regardless of --namespaced-targets, due
  to an always-truthy condition, and never emitted it for Apple framework
  targets, which CMake names after it.
- The Apple build ran a full "cmake --build" once per shared library,
  concurrently against one build tree, and called "xcodebuild -list" (a
  synchronous spawn) once per library per triplet.
- xcodebuild invocations now run in sequence per build directory, as
  concurrent invocations against a single Xcode project are not reliable.
- postBuild looked for "<target name>.framework" while createAppleFramework
  names it after the artifact, so the two diverged under namespacing.
- --concurrency accepted any value, and did not implement the documented
  fallback to 1 under --verbose. Max listeners is now derived from it.
- verify-prebuilds globbed a directory the prebuilds had moved out of, so
  it passed by finding nothing. It now covers tests/ too and requires a
  non-zero count.
- The root example project globbed recursively, which both missed examples
  copied in after configure and would add a nested project twice. It is
  now generated from the same script pipeline.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KfKQDvEkxNtkSE4aaF9yG8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants