Skip to content

Commit 1dee1a0

Browse files
kraenhansenclaude
andauthored
Split public header packaging between framework and file set (#382)
* Split public header packaging between framework and file set CMake 4.2 rejects a `HEADERS` file set on a `FRAMEWORK` target, which broke every macOS job that builds the weak-node-api framework: CMake Error in CMakeLists.txt: The file set "HEADERS", of type "HEADERS", is incompatible with the "FRAMEWORK" target "weak-node-api". The workaround pinned CMake to 4.1.2 on five macOS jobs (#374, #375, #377, and the pre-existing test-macos pin). This fixes the root cause instead. On Apple, the public headers are shipped via the framework's PUBLIC_HEADER property; other platforms use a HEADERS file set for install packaging. The build-interface include directories that the file set previously supplied to in-tree consumers (e.g. the C++ tests) are now provided explicitly via target_include_directories, so both branches build unchanged. With the root cause fixed, revert the macOS CMake version pin from all five jobs. CMAKE_VERSION is retained since it still selects the Android SDK cmake package for test-android. Closes #376 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Aq4PqMYq8cQTP9Lf43JiwH * ci: trigger CI with Apple/macOS/Ferric/weak-node-api labels Empty commit to re-run the label-gated macOS jobs now that the labels are applied, so the reverted CMake pin is validated on the runner's default CMake. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Aq4PqMYq8cQTP9Lf43JiwH --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c77f968 commit 1dee1a0

2 files changed

Lines changed: 24 additions & 35 deletions

File tree

.github/workflows/check.yml

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,6 @@ jobs:
9292
with:
9393
packages: tools platform-tools ndk;${{ env.NDK_VERSION }}
9494
- run: rustup target add x86_64-linux-android aarch64-linux-android armv7-linux-androideabi i686-linux-android aarch64-apple-ios-sim
95-
# macOS runners ship a newer CMake than the project supports; pin it to match the version used elsewhere in CI
96-
- name: Install compatible CMake version
97-
if: runner.os == 'macOS'
98-
uses: jwlawson/actions-setup-cmake@v2
99-
with:
100-
cmake-version: ${{ env.CMAKE_VERSION }}
10195
- run: npm ci
10296
- run: npm run bootstrap
10397
- run: npm test
@@ -125,12 +119,6 @@ jobs:
125119
uses: hendrikmuhs/ccache-action@v1.2
126120
with:
127121
key: ${{ github.job }}-${{ runner.os }}
128-
# macOS runners ship a newer CMake than the project supports; pin it to match the version used elsewhere in CI
129-
- name: Install compatible CMake version
130-
if: runner.os == 'macOS'
131-
uses: jwlawson/actions-setup-cmake@v2
132-
with:
133-
cmake-version: ${{ env.CMAKE_VERSION }}
134122
- run: npm ci
135123
- run: npm run build
136124
- name: Prepare weak-node-api
@@ -168,11 +156,6 @@ jobs:
168156
with:
169157
packages: tools platform-tools ndk;${{ env.NDK_VERSION }}
170158
- run: rustup target add x86_64-linux-android aarch64-linux-android armv7-linux-androideabi i686-linux-android aarch64-apple-ios-sim
171-
# macOS runners ship a newer CMake than the project supports; pin it to match the version used elsewhere in CI
172-
- name: Install compatible CMake version
173-
uses: jwlawson/actions-setup-cmake@v2
174-
with:
175-
cmake-version: ${{ env.CMAKE_VERSION }}
176159
- run: npm ci
177160
- run: npm run bootstrap
178161
env:
@@ -209,10 +192,6 @@ jobs:
209192
with:
210193
java-version: "17"
211194
distribution: "temurin"
212-
- name: Install compatible CMake version
213-
uses: jwlawson/actions-setup-cmake@v2
214-
with:
215-
cmake-version: ${{ env.CMAKE_VERSION }}
216195
- run: rustup target add x86_64-apple-darwin
217196
- run: npm ci
218197
- run: npm run bootstrap
@@ -349,11 +328,6 @@ jobs:
349328
distribution: "temurin"
350329
- run: rustup target add x86_64-apple-darwin x86_64-apple-ios aarch64-apple-ios aarch64-apple-ios-sim
351330
- run: rustup toolchain install nightly --component rust-src
352-
# macOS runners ship a newer CMake than the project supports; pin it to match the version used elsewhere in CI
353-
- name: Install compatible CMake version
354-
uses: jwlawson/actions-setup-cmake@v2
355-
with:
356-
cmake-version: ${{ env.CMAKE_VERSION }}
357331
- run: npm ci
358332
- run: npm run build
359333
- name: Build weak-node-api for all Apple architectures

packages/weak-node-api/CMakeLists.txt

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,32 @@ add_library(${PROJECT_NAME} SHARED)
1010
set(INCLUDE_DIR "include")
1111
set(GENERATED_SOURCE_DIR "generated")
1212

13+
set(PUBLIC_HEADER_FILES
14+
${GENERATED_SOURCE_DIR}/weak_node_api.hpp
15+
${GENERATED_SOURCE_DIR}/NodeApiHost.hpp
16+
${INCLUDE_DIR}/js_native_api_types.h
17+
${INCLUDE_DIR}/js_native_api.h
18+
${INCLUDE_DIR}/node_api_types.h
19+
${INCLUDE_DIR}/node_api.h
20+
)
21+
1322
target_sources(${PROJECT_NAME}
1423
PUBLIC
1524
${GENERATED_SOURCE_DIR}/weak_node_api.cpp
16-
PUBLIC FILE_SET HEADERS
17-
BASE_DIRS ${GENERATED_SOURCE_DIR} ${INCLUDE_DIR} FILES
18-
${GENERATED_SOURCE_DIR}/weak_node_api.hpp
19-
${GENERATED_SOURCE_DIR}/NodeApiHost.hpp
20-
${INCLUDE_DIR}/js_native_api_types.h
21-
${INCLUDE_DIR}/js_native_api.h
22-
${INCLUDE_DIR}/node_api_types.h
23-
${INCLUDE_DIR}/node_api.h
2425
)
2526

26-
get_target_property(PUBLIC_HEADER_FILES ${PROJECT_NAME} HEADER_SET)
27+
# Expose the public headers to consumers within the build tree (e.g. the tests)
28+
# regardless of how they are packaged below.
29+
target_include_directories(${PROJECT_NAME}
30+
PUBLIC
31+
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${GENERATED_SOURCE_DIR}>
32+
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/${INCLUDE_DIR}>
33+
)
2734

2835
# Stripping the prefix from the library name
2936
# to make sure the name of the XCFramework will match the name of the library
3037
if(APPLE)
38+
# Frameworks ship their public headers via the PUBLIC_HEADER property.
3139
set_target_properties(${PROJECT_NAME} PROPERTIES
3240
FRAMEWORK TRUE
3341
MACOSX_FRAMEWORK_IDENTIFIER com.callstack.${PROJECT_NAME}
@@ -37,6 +45,13 @@ if(APPLE)
3745
XCODE_ATTRIBUTE_SKIP_INSTALL NO
3846
PUBLIC_HEADER "${PUBLIC_HEADER_FILES}"
3947
)
48+
else()
49+
# Elsewhere, ship the public headers via a HEADERS file set for install packaging.
50+
target_sources(${PROJECT_NAME}
51+
PUBLIC FILE_SET HEADERS
52+
BASE_DIRS ${GENERATED_SOURCE_DIR} ${INCLUDE_DIR} FILES
53+
${PUBLIC_HEADER_FILES}
54+
)
4055
endif()
4156

4257
# C++20 is needed to use designated initializers

0 commit comments

Comments
 (0)