-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[wasm][build] Handle FOUNDATION_BUILD_TOOLS
value set to OFF
#4868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[wasm][build] Handle FOUNDATION_BUILD_TOOLS
value set to OFF
#4868
Conversation
@swift-ci test |
Sources/CMakeLists.txt
Outdated
@@ -2,4 +2,6 @@ add_subdirectory(UUID) | |||
add_subdirectory(Foundation) | |||
add_subdirectory(FoundationNetworking) | |||
add_subdirectory(FoundationXML) | |||
add_subdirectory(Tools) | |||
if(BUILD_TOOLS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a standard CMake flag. Can you create a proper option
for this for something like FOUNDATION_BUILD_TOOLS
? We can control the default for it to disable it by default on WASM targets if desired.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The option is already declared here as option
https://github.com/apple/swift-corelibs-foundation/blob/7f62c8c52aeed9a86b503fe7b662c48eda5d0f8a/CMakeLists.txt#L43 but I agree the option name should be prefixed by FOUNDATION
This is a follow up to 5e7281b, which just adds the `BUILD_TOOLS` option but doesn't actually use it. This patch renames the option to follow the CMake conventions and uses it to actually exclude the tools from the build.
56330a7
to
004a9dd
Compare
@swift-ci test |
@swift-ci test windows |
FOUNDATION_BUILD_TOOLS
value set to OFF
@swift-ci test windows |
This is a follow up to 5e7281b, which just adds the
BUILD_TOOLS
option but doesn't actually use it.