-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[wasm][build] Disable networking targets from CMake build #4867
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] Disable networking targets from CMake build #4867
Conversation
@swift-ci test |
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.
Please follow CMake conventions and name this FOUNDATION_...
. Perhaps FOUNDATION_ENABLE_FOUNDATION_NETWORKING
? The option should default to true for all targets IMO.
f4f29a4
to
17b8520
Compare
@swift-ci test |
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 seems reasonable if it does not regress Windows.
install(TARGETS | ||
CFURLSessionInterface | ||
DESTINATION | ||
"${CMAKE_INSTALL_FULL_LIBDIR}/$<LOWER_CASE:${CMAKE_SYSTEM_NAME}>") |
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.
I'm concerned that this might negatively impact Windows. Can you please test Windows as well?
@swift-ci Please test Windows platform |
This is a follow-up to 5e7281b, which introduced `BUILD_NETWORKING` option but didn't update the CMake build to respect it. This patch renames the option to follow CMake conventions and updates the build to respect it.
17b8520
to
a63cfba
Compare
@swift-ci Please test Linux Platform |
@swift-ci Please test Windows Platform |
@swift-ci test |
@swift-ci test windows |
This is a follow-up to 5e7281b