build: honour BUILD_SHARED_LIBS
#396
Conversation
|
@swift-ci please test |
|
Please test with following PRs: @swift-ci please test |
CMakeLists.txt
Outdated
| @@ -74,14 +74,12 @@ if(ENABLE_SWIFT) | |||
|
|
|||
| set(INSTALL_TARGET_DIR "${INSTALL_LIBDIR}/swift/${SWIFT_OS}" CACHE PATH "Path where the libraries will be installed") | |||
| set(INSTALL_DISPATCH_HEADERS_DIR "${INSTALL_LIBDIR}/swift/dispatch" CACHE PATH "Path where the headers will be installed for libdispatch") | |||
| set(INSTALL_BLOCK_HEADERS_DIR "${INSTALL_LIBDIR}/swift/Block" CACHE PATH "Path where the headers will be installed for the blocks runtime") | |||
There was a problem hiding this comment.
These directories should be swift or swift_static depending on the value of BUILD_SHARED_LIBS
There was a problem hiding this comment.
Sure, though this issue has existed for some time now and is not made any worse by this change. I'll put up a separate PR to address that.
| set_target_properties(BlocksRuntime | ||
| PROPERTIES | ||
| INTERFACE_LINK_LIBRARIES ${CMAKE_DL_LIBS}) | ||
| target_link_libraries(BlocksRuntime |
There was a problem hiding this comment.
This is a functional linkage change that should go into another PR (though I suspect this branch is never executed). If this is required then we need to change our approach to the module map as well (as we'd now need to link dl and objc).
There was a problem hiding this comment.
INTERFACE doesn't work with shared libraries, so this should be public (since INTERFACE is equal to PUBLIC with shared).
compnerd
force-pushed
the
build-type-blocks
branch
from
be47984 to
98f8021
Compare
|
Please test with following PRs: @swift-ci please test |
compnerd
force-pushed
the
build-type-blocks
branch
from
98f8021 to
4a890d0
Compare
There was a problem hiding this comment.
As we discussed offline, can you add in a follow on commit:
- A comment here that explicitly says that we define BUILD_SHARED_LIBS to control shared/static.
- Above where we create the option BUILD_SHARED_LIBS can you include a comment that says that this is a way that cmake allows one to control globally whether add_library defaults to shared or static and that we always only produce one or the other.
The reason I am asking for these comments is that I am worried about less experienced cmake people not understanding what we are doing here since in other parts of swift.org cmake we are always explicit about shared vs static.
This is needed for proper shared linkage to the BlocksRuntime (e.g. when building SourceKit). We now build BlocksRuntime shared or static as requested.
compnerd
force-pushed
the
build-type-blocks
branch
from
4a890d0 to
8aa73a1
Compare
|
Please test with following PRs: @swift-ci please test |
|
@compnerd: I might be wrong, but it seems that One of those is XCTest. While compiling it uses https://github.com/apple/swift-corelibs-xctest/blob/master/CMakeLists.txt#L48 to point to the Another is LLBuild. From https://github.com/apple/swift-llbuild/blob/master/products/llbuildSwift/CMakeLists.txt#L19-L24 it points the library search path to I think those might need to be fixed, but I will wait until I can talk with you to confirm. |
build: honour `BUILD_SHARED_LIBS` Signed-off-by: Kim Topley <[email protected]>
This is needed for proper shared linkage to the BlocksRuntime (e.g. when
building SourceKit). We now build BlocksRuntime shared or static as requested.