Skip to content

Use a coherent system SDK for Apple builds - #1783

Open
matthargett wants to merge 1 commit into
BabylonJS:masterfrom
rebeckerspecialties:apple-sdk-build-consistency
Open

Use a coherent system SDK for Apple builds#1783
matthargett wants to merge 1 commit into
BabylonJS:masterfrom
rebeckerspecialties:apple-sdk-build-consistency

Conversation

@matthargett

Copy link
Copy Markdown

Summary

  • default top-level macOS builds to the selected Xcode macosx SDK when no toolchain or sysroot was supplied
  • classify JavaScriptCore framework headers as system headers for napi and its consumers
  • preserve explicit toolchains, explicit sysroots, and Apple mobile configurations

Why

An empty macOS sysroot can mix Command Line Tools headers with frameworks from the selected Xcode installation. With Xcode 26.5, diagnostics from that mixed SDK surface become errors under JsRuntimeHost warnings-as-errors builds.

Using one SDK for headers and frameworks fixes the mismatch. Marking the JavaScriptCore framework directory as a system framework keeps SDK implementation diagnostics out of project warning policy without suppressing warnings in BabylonNative or JsRuntimeHost sources.

Validation

  • fresh Release/Ninja configure from current BabylonNative master
  • confirmed CMAKE_OSX_SYSROOT=macosx and NAPI_JAVASCRIPT_ENGINE=JavaScriptCore
  • built napi and AppRuntime with Xcode 26.5
  • verified compile commands use the same MacOSX26.5.sdk for -isysroot and -iframework
  • git diff --check

Default top-level macOS builds to the selected Xcode SDK instead of allowing Command Line Tools headers to mix with Xcode frameworks. Mark the JavaScriptCore framework search path as a system framework through napi so JsRuntimeHost's pedantic warnings-as-errors policy ignores SDK implementation diagnostics without suppressing project warnings.\n\nThis fixes Xcode 26.5 nullability, invalid UTF-8, and related framework-header failures while leaving explicit cross-compilation toolchains and Apple mobile platforms unchanged.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to make Apple/macOS builds use a consistent Xcode SDK sysroot (avoiding mixed Command Line Tools + Xcode SDK headers/frameworks) and to treat JavaScriptCore framework headers as system headers when building napi with the JavaScriptCore engine, preventing SDK diagnostics from being promoted to errors under warnings-as-errors builds.

Changes:

  • Default top-level macOS builds (when not using an explicit toolchain/sysroot and not targeting iOS/visionOS) to CMAKE_OSX_SYSROOT=macosx.
  • Add -iframework propagation on Apple JavaScriptCore builds so JavaScriptCore framework headers are treated as system headers for napi and its consumers.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
CMakeLists.txt Defaults top-level macOS builds to the selected Xcode macosx SDK when no sysroot/toolchain is explicitly provided.
Dependencies/CMakeLists.txt Adds compiler flags to classify JavaScriptCore framework headers as system headers for napi consumers on Apple builds.

Comment on lines +194 to +202
if(APPLE AND NAPI_JAVASCRIPT_ENGINE STREQUAL "JavaScriptCore" AND TARGET napi)
# find_library adds the SDK framework directory with -F, which does not
# classify framework headers as system headers. JsRuntimeHost uses
# -pedantic -Werror, so make that classification explicit for napi and all
# consumers while preserving warnings-as-errors for project sources.
get_filename_component(_JAVASCRIPTCORE_FRAMEWORKS_DIR "${JAVASCRIPTCORE_LIBRARY}" DIRECTORY)
target_compile_options(napi PUBLIC "SHELL:-iframework ${_JAVASCRIPTCORE_FRAMEWORKS_DIR}")
unset(_JAVASCRIPTCORE_FRAMEWORKS_DIR)
endif()
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