build: add support for nesting the Swift subdirectory #860
Conversation
These can be controlled by the user external to the project. Remove this to simplify the rules.
CMake 3.19.0 fixed the compiler invocation requiring the local workaround. Match the runtimes CMake version requirement and drop the workaround.
Reduce the verbosity in the generator expression to apply the compile flags to C or C++ code only.
`HAVE_STRLCPY` was added to the static configuration but not the dynamic configuration. Update the template for this.
CMake 3.25 introduced a more consistent way to check what system is being built for. Adopt that uniformly through the build to make it easier to understand.
|
@swift-ci please test |
|
@swift-ci please test |
|
@swift-ci please test Windows platform |
This introduces the new option to control installation to a nested subdirectory. It also converts the current installation into a thick module format.
|
@swift-ci please test |
The DispatchStubs are now used only for the ObjC codepath and no longer need to be included in the regular build. Simply remove the extra library unless Objective-C support is enabled.
This should now be enabled due to the required version specification being updated.
Specify RPATH and POSITION_INDEPENDENT globally rather than on a per-target basis.
|
@swift-ci please test Linux platform |
|
Please test with following PRs: @swift-ci please test Linux platform |
|
@swift-ci please test Windows platform |
1 similar comment
|
@swift-ci please test Windows platform |
|
Please test with following PRs: @swift-ci please test Windows platform |
etcwilde
left a comment
etcwilde
left a comment
There was a problem hiding this comment.
For next time, can you split the CMake updates and the module installation changes into separate PRs. There's a lot going on in this PR.
|
|
||
| set(CMAKE_POSITION_INDEPENDENT_CODE YES) | ||
| if(NOT APPLE) | ||
| set(CMAKE_INSTALL_RPATH "$ORIGIN") |
There was a problem hiding this comment.
What if I want to override the RPATH?
There was a problem hiding this comment.
Good question - how would you do that today? I don't think that this was something that was previously available.
There was a problem hiding this comment.
Maybe something like this?
if(NOT APPLE AND NOT DEFINED CMAKE_INSTALL_RPATH)
set(CMAKE_INSTALL_RPATH "$ORIGIN")
endif()What is the CMake default? Nothing?
Alternatively, maybe something like this?
if(NOT APPLE)
list(APPEND CMAKE_INSTALL_RPATH "$ORIGIN")
endif()I think I prefer the former though.
No, it wasn't previously available, but as long as we're fixing all the things, might as well fix this too.
| INSTALL(FILES $<TARGET_PROPERTY:swiftDispatch,Swift_MODULE_DIRECTORY>/$<TARGET_PROPERTY:swiftDispatch,Swift_MODULE_NAME>.swiftdoc | ||
| DESTINATION ${Dispatch_INSTALL_SWIFTMODULEDIR}/$<TARGET_PROPERTY:swiftDispatch,Swift_MODULE_NAME>.swiftmodule | ||
| RENAME ${Dispatch_MODULE_TRIPLE}.swiftdoc) | ||
| # INSTALL(FILES $<TARGET_PROPERTY:swiftDispatch,Swift_MODULE_DIRECTORY>/$<TARGET_PROPERTY:swiftDispatch,Swift_MODULE_NAME>.swiftinterface |
There was a problem hiding this comment.
Is this commented out for a reason? Do we want a TODO here to enable installing the interface once interfaces work without stability, or should this code be deleted?
There was a problem hiding this comment.
Yeah, this is something that we want to enable in the future. It requires adding new options, and I was trying to keep the flags the same as they were, just changing the install location.
There was a problem hiding this comment.
Okay, maybe leave a FIXME/TODO comment so that we know what was intended.
| export * | ||
| link "dispatch" | ||
| link "BlocksRuntime" | ||
| link "DispatchStubs" |
There was a problem hiding this comment.
Not a request for changes, but by removing this line, this file is identical to https://github.com/swiftlang/swift-corelibs-libdispatch/blob/main/dispatch/generic/module.modulemap, so we might be able to remove the static case from the vfs overlay generation
swift-corelibs-libdispatch/dispatch/CMakeLists.txt
Lines 6 to 7 in 84ec97c
3 participants
Prepare dispatch to permit installation into a nested subdirectory. Prior to doing that change, try to simplify some of the CMake rules to bring them into line with more modern CMake rules.