build: perform parallel builds of BlocksRuntime #395
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
|
|
||
| cmake_minimum_required(VERSION 3.4.3) | ||
| project(BlocksRuntime | ||
| LANGUAGES C) | ||
|
|
||
| include(CheckIncludeFiles) | ||
| include(GNUInstallDirs) | ||
|
|
||
| if(CMAKE_C_SIMULATE_ID STREQUAL MSVC) | ||
| # clang-cl interprets paths starting with /U as macro undefines, so we need to | ||
| # put a -- before the input file path to force it to be treated as a path. | ||
| string(REPLACE "-c <SOURCE>" "-c -- <SOURCE>" CMAKE_C_COMPILE_OBJECT "${CMAKE_C_COMPILE_OBJECT}") | ||
| endif() | ||
|
|
||
| set(CMAKE_C_STANDARD 11) | ||
| set(CMAKE_C_STANDARD_REQUIRED YES) | ||
|
|
||
| set(CMAKE_C_VISIBILITY_PRESET hidden) | ||
| set(CMAKE_C_VISIBILITY_INLINES_HIDDEN YES) | ||
|
|
||
| check_include_files("objc/objc-internal.h" HAVE_OBJC) | ||
|
|
||
| option(INSTALL_PRIVATE_HEADERS "install private headers in the same location as the public ones" NO) | ||
|
There was a problem hiding this comment. By placing this inside an ExternalProject there's no way to trigger this option anymore. There was a problem hiding this comment. Oh, but there is! There is the still the option in libdispatch, which just passes it through :-) However, note that the installation for the swift side is done in the libdispatch side, so that we can install both variants into the install'ed image for libdispatch. |
||
|
|
||
| add_library(BlocksRuntime | ||
| ${PROJECT_SOURCE_DIR}/data.c | ||
| ${PROJECT_SOURCE_DIR}/runtime.c) | ||
| set_target_properties(BlocksRuntime | ||
| PROPERTIES | ||
| POSITION_INDEPENDENT_CODE TRUE) | ||
| if(HAVE_OBJC AND CMAKE_DL_LIBS) | ||
| target_link_libraries(BlocksRuntime | ||
| PUBLIC | ||
| ${CMAKE_DL_LIBS}) | ||
| endif() | ||
|
|
||
| install(FILES | ||
| ${PROJECT_SOURCE_DIR}/Block.h | ||
| DESTINATION | ||
| ${CMAKE_INSTALL_INCLUDEDIR}) | ||
| if(INSTALL_PRIVATE_HEADERS) | ||
| install(FILES | ||
| ${PROJECT_SOURCE_DIR}/Block_private.h | ||
| DESTINATION | ||
| ${CMAKE_INSTALL_INCLUDEDIR}) | ||
| endif() | ||
| install(TARGETS | ||
| BlocksRuntime | ||
| DESTINATION | ||
| ${CMAKE_INSTALL_LIBDIR}) | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
My experience using
ExternalProjecthasn't been great compared to creating library targets that are directly addressable in the build system and I strongly suggest we don't use it in dispatch.CMAKE_ARGS. For example here you don't propagateCMAKE_C_FLAGS,CMAKE_ARorCMAKE_LIBRARY_PATHwhich could cause the downstream libraries to be built differently. There are too many influential variables to enumerate here.BUILD_BYPRODUCTSmanually and it's easy to under or over-depend. ExternalProjects also don't have great IDE integration as they appear opaque.IMPORTEDprevents dependency tracking and prevents it being exported from the build system (viainstall(EXPORT)for example).Nis the number of cores your build machine has). Console output is also broken.Having said all that, have you considered making
BlocksRuntimeandBlocksRuntime_statictargets usingadd_libraryinstead?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.
Well, we already are using ExternalProject for libdispatch in swift, and it has been working quite well :-). That said, there is interest in moving to the LLVM wrapper. Doing so will automatically propagate those other variables implicitly.
The dependency tracking issue that you portray is not entirely correct. There is tracking of the dependencies in the sense that the external project's byproducts are linked into the imported library target. So if the sub-library changes, CMake/ninja realize that and rebuild the dependent targets.
Yes, I did consider that. I suppose that if we want to just duplicate the rules, thats certainly an option. I really am strongly opposed to creating the custom wrappers for both variants. I suppose that we could always extract the BlocksRuntime rules into a separate CMakeList and include it in this one to make this easier to work with.