Skip to content

Split Swift SDK Overlay #401

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 5, 2018
Merged

Split Swift SDK Overlay #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
71942ed
cmake: update SwiftSupport from XCTest
compnerd Oct 12, 2018
965739e
cmake: support static and shared swift libraries
compnerd Oct 12, 2018
cbe5c1d
build: split out the SDK overlay
compnerd Oct 12, 2018
3526fab
build: correct dependency tracking
compnerd Oct 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 142 additions & 45 deletions cmake/modules/SwiftSupport.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,74 @@

include(CMakeParseArguments)

function(add_swift_library library)
set(options)
function(add_swift_target target)
set(options LIBRARY;SHARED;STATIC)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One concern that I have with the way that the code is written is that the installation code is separate from the add_swift_target. I am worried about that getting out of sync with this code. I am not sure if today it is out of sync... but can you check just in case? In the future IMO add_swift_target should just handle the installation to make sure things stay in sync.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you would like me to verify. I think I would be find with extending this in a follow up to add support for installation of the output as well.

set(single_value_options MODULE_NAME;MODULE_LINK_NAME;MODULE_PATH;MODULE_CACHE_PATH;OUTPUT;TARGET)
set(multiple_value_options SOURCES;SWIFT_FLAGS;CFLAGS;DEPENDS)
set(multiple_value_options CFLAGS;DEPENDS;LINK_FLAGS;SOURCES;SWIFT_FLAGS)

cmake_parse_arguments(ASL "${options}" "${single_value_options}" "${multiple_value_options}" ${ARGN})
cmake_parse_arguments(AST "${options}" "${single_value_options}" "${multiple_value_options}" ${ARGN})

set(flags ${CMAKE_SWIFT_FLAGS})
set(link_flags)

list(APPEND flags -emit-library)

if(ASL_TARGET)
list(APPEND FLAGS -target;${ASL_TARGET})
endif()
if(ASL_MODULE_NAME)
list(APPEND flags -module-name;${ASL_MODULE_NAME})
if(AST_TARGET)
list(APPEND flags -target;${AST_TARGET})
endif()
if(ASL_MODULE_LINK_NAME)
list(APPEND flags -module-link-name;${ASL_MODULE_LINK_NAME})
if(AST_MODULE_NAME)
list(APPEND flags -module-name;${AST_MODULE_NAME})
else()
list(APPEND flags -module-name;${target})
endif()
if(ASL_MODULE_PATH)
list(APPEND flags -emit-module-path;${ASL_MODULE_PATH})
if(AST_MODULE_LINK_NAME)
list(APPEND flags -module-link-name;${AST_MODULE_LINK_NAME})
endif()
if(ASL_MODULE_CACHE_PATH)
list(APPEND flags -module-cache-path;${ASL_MODULE_CACHE_PATH})
if(AST_MODULE_CACHE_PATH)
list(APPEND flags -module-cache-path;${AST_MODULE_CACHE_PATH})
endif()
if(ASL_SWIFT_FLAGS)
foreach(flag ${ASL_SWIFT_FLAGS})
if(AST_SWIFT_FLAGS)
foreach(flag ${AST_SWIFT_FLAGS})
list(APPEND flags ${flag})
endforeach()
endif()
if(ASL_CFLAGS)
foreach(flag ${ASL_CFLAGS})
if(AST_CFLAGS)
foreach(flag ${AST_CFLAGS})
list(APPEND flags -Xcc;${flag})
endforeach()
endif()

# FIXME: We shouldn't /have/ to build things in a single process.
# <rdar://problem/15972329>
list(APPEND flags -force-single-frontend-invocation)
if(AST_LINK_FLAGS)
foreach(flag ${AST_LINK_FLAGS})
list(APPEND link_flags ${flag})
endforeach()
endif()
if(AST_LIBRARY)
if(AST_STATIC AND AST_SHARED)
message(SEND_ERROR "add_swift_target asked to create library as STATIC and SHARED")
elseif(AST_STATIC OR NOT BUILD_SHARED_LIBS)
set(library_kind STATIC)
elseif(AST_SHARED OR BUILD_SHARED_LIBS)
set(library_kind SHARED)
endif()
else()
if(AST_STATIC OR AST_SHARED)
message(SEND_ERROR "add_swift_target asked to create executable as STATIC or SHARED")
endif()
endif()
if(NOT AST_OUTPUT)
if(AST_LIBRARY)
if(AST_SHARED OR BUILD_SHARED_LIBS)
set(AST_OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${target}.dir/${CMAKE_SHARED_LIBRARY_PREFIX}${target}${CMAKE_SHARED_LIBRARY_SUFFIX})
else()
# NOTE(compnerd) this is a hack for the computation of the
# basename/dirname below for the static path.
set(AST_OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${target}.dir/${target})
endif()
else()
set(AST_OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${target}.dir/${target}${CMAKE_EXECUTABLE_SUFFIX})
endif()
endif()

set(sources)
foreach(source ${ASL_SOURCES})
foreach(source ${AST_SOURCES})
get_filename_component(location ${source} PATH)
if(IS_ABSOLUTE ${location})
list(APPEND sources ${source})
Expand All @@ -52,25 +77,97 @@ function(add_swift_library library)
endif()
endforeach()

get_filename_component(module_directory ${ASL_MODULE_PATH} DIRECTORY)

add_custom_command(OUTPUT
${ASL_OUTPUT}
${ASL_MODULE_PATH}
${module_directory}/${ASL_MODULE_NAME}.swiftdoc
DEPENDS
${ASL_SOURCES}
${CMAKE_SWIFT_COMPILER}
${ASL_DEPENDS}
COMMAND
${CMAKE_COMMAND} -E make_directory ${module_directory}
COMMAND
${CMAKE_SWIFT_COMPILER} ${flags} -c ${sources} -o ${ASL_OUTPUT})
add_custom_target(${library}
DEPENDS
${ASL_OUTPUT}
${ASL_MODULE_PATH}
${module_directory}/${ASL_MODULE_NAME}.swiftdoc)
set(objs)
set(mods)
set(docs)
set(i 0)
foreach(source ${sources})
get_filename_component(name ${source} NAME)

set(obj ${CMAKE_CURRENT_BINARY_DIR}/${target}.dir/${name}${CMAKE_C_OUTPUT_EXTENSION})
set(mod ${CMAKE_CURRENT_BINARY_DIR}/${target}.dir/${name}.swiftmodule)
set(doc ${CMAKE_CURRENT_BINARY_DIR}/${target}.dir/${name}.swiftdoc)

set(all_sources ${sources})
list(INSERT all_sources ${i} -primary-file)

add_custom_command(OUTPUT
${obj}
${mod}
${doc}
DEPENDS
${source}
${AST_DEPENDS}
COMMAND
${CMAKE_SWIFT_COMPILER} -frontend ${flags} -emit-module-path ${mod} -emit-module-doc-path ${doc} -o ${obj} -c ${all_sources})

list(APPEND objs ${obj})
list(APPEND mods ${mod})
list(APPEND docs ${doc})

math(EXPR i "${i}+1")
endforeach()

if(AST_LIBRARY)
get_filename_component(module_directory ${AST_MODULE_PATH} DIRECTORY)

set(module ${AST_MODULE_PATH})
set(documentation ${module_directory}/${AST_MODULE_NAME}.swiftdoc)

add_custom_command(OUTPUT
${module}
${documentation}
DEPENDS
${mods}
${docs}
${AST_DEPENDS}
COMMAND
${CMAKE_SWIFT_COMPILER} -frontend ${flags} -sil-merge-partial-modules -emit-module ${mods} -o ${module} -emit-module-doc-path ${documentation})
endif()

if(AST_LIBRARY)
set(emit_library -emit-library)
endif()
if(library_kind STREQUAL SHARED)
add_custom_command(OUTPUT
${AST_OUTPUT}
DEPENDS
${objs}
${AST_DEPENDS}
COMMAND
${CMAKE_SWIFT_COMPILER} ${emit_library} ${link_flags} -o ${AST_OUTPUT} ${objs}
COMMAND
${CMAKE_COMMAND} -E copy ${AST_OUTPUT} ${CMAKE_CURRENT_BINARY_DIR})
add_custom_target(${target}
ALL
DEPENDS
${AST_OUTPUT}
${module}
${documentation})
else()
add_library(${target}-static STATIC ${objs})
get_filename_component(ast_output_bn ${AST_OUTPUT} NAME)
get_filename_component(ast_output_dn ${AST_OUTPUT} DIRECTORY)
set_target_properties(${target}-static
PROPERTIES
LINKER_LANGUAGE C
OUTPUT_DIRECTORY ${ast_output_dn}
OUTPUT_NAME ${ast_output_bn})
add_custom_target(${target}
ALL
DEPENDS
${target}-static
${module}
${documentation})
endif()
endfunction()

function(add_swift_library library)
add_swift_target(${library} LIBRARY ${ARGN})
endfunction()

function(add_swift_executable executable)
add_swift_target(${executable} ${ARGN})
endfunction()

# Returns the current achitecture name in a variable
Expand Down
83 changes: 54 additions & 29 deletions src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,25 +72,60 @@ target_sources(dispatch
PRIVATE
block.cpp)
if(HAVE_OBJC)
# TODO(compnerd) split DispatchStubs.cc into a separate component for the ObjC
# registration and a separate component for the swift compiler's emission of a
# call to the ObjC autorelease elision entry point.
target_sources(dispatch
PRIVATE
data.m
object.m)
object.m
swift/DispatchStubs.cc)
endif()
if(ENABLE_SWIFT)
set(swift_optimization_flags)
if(NOT CMAKE_BUILD_TYPE MATCHES Debug)
set(swift_optimization_flags -O)
endif()

# NOTE(compnerd) Today regardless of whether or not ObjC interop is enabled,
# swift will use an autoreleased return value convention for certain CF
# functions (including some that are used/related to dispatch). This means
# that the swift compiler in callers to such functions will call the function,
# and then pass the result of the function to
# objc_retainAutoreleasedReturnValue. In a context where we have ObjC interop
# disabled, we do not have access to the objc runtime so an implementation of
# objc_retainAutoreleasedReturnValue is not available. To work around this, we
# provide a shim for objc_retainAutoreleasedReturnValue in DispatchStubs.cc
# that just calls retain on the object. Once we fix the swift compiler to
# switch to a different model for handling these arguments with objc-interop
# disabled these shims can be eliminated.
add_library(DispatchStubs
STATIC
swift/DispatchStubs.cc)
target_include_directories(DispatchStubs
PRIVATE
${PROJECT_SOURCE_DIR})
set_target_properties(DispatchStubs
PROPERTIES
POSITION_INDEPENDENT_CODE YES)

add_swift_library(swiftDispatch
CFLAGS
-fblocks
-fmodule-map-file=${PROJECT_SOURCE_DIR}/dispatch/module.modulemap
DEPENDS
${PROJECT_SOURCE_DIR}/dispatch/module.modulemap
DispatchStubs
LINK_FLAGS
-lDispatchStubs
-L $<TARGET_LINKER_FILE_DIR:dispatch>
-ldispatch
MODULE_NAME
Dispatch
MODULE_LINK_NAME
dispatch
swiftDispatch
MODULE_PATH
${CMAKE_CURRENT_BINARY_DIR}/swift/Dispatch.swiftmodule
OUTPUT
${CMAKE_CURRENT_BINARY_DIR}/swiftDispatch.o
SOURCES
swift/Block.swift
swift/Data.swift
Expand All @@ -101,32 +136,12 @@ if(ENABLE_SWIFT)
swift/Source.swift
swift/Time.swift
swift/Wrapper.swift
TARGET
${CMAKE_C_COMPILER_TARGET}
CFLAGS
-fblocks
-fmodule-map-file=${PROJECT_SOURCE_DIR}/dispatch/module.modulemap
SWIFT_FLAGS
-I ${PROJECT_SOURCE_DIR}
-I/usr/include
${swift_optimization_flags}
DEPENDS
${PROJECT_SOURCE_DIR}/dispatch/module.modulemap)

get_filename_component(swift_toolchain ${CMAKE_SWIFT_COMPILER} DIRECTORY)
get_filename_component(swift_toolchain ${swift_toolchain} DIRECTORY)
set(swift_runtime_libdir ${swift_toolchain}/lib/${swift_dir}/${swift_os}/${swift_arch})

target_sources(dispatch
PRIVATE
swift/DispatchStubs.cc
${CMAKE_CURRENT_BINARY_DIR}/swiftDispatch.o
${swift_runtime_libdir}/swiftrt.o)
if(CMAKE_BUILD_TYPE MATCHES Debug)
target_link_libraries(dispatch
PRIVATE
swiftSwiftOnoneSupport)
endif()
TARGET
${CMAKE_C_COMPILER_TARGET})
endif()
if(ENABLE_DTRACE)
dtrace_usdt_probe(${CMAKE_CURRENT_SOURCE_DIR}/provider.d
Expand Down Expand Up @@ -231,8 +246,6 @@ add_custom_command(TARGET dispatch POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy $<TARGET_FILE:dispatch> .libs
COMMENT "Copying libdispatch to .libs")

get_swift_host_arch(SWIFT_HOST_ARCH)

install(TARGETS
dispatch
DESTINATION
Expand All @@ -242,6 +255,18 @@ if(ENABLE_SWIFT)
${CMAKE_CURRENT_BINARY_DIR}/swift/Dispatch.swiftmodule
${CMAKE_CURRENT_BINARY_DIR}/swift/Dispatch.swiftdoc
DESTINATION
"${INSTALL_TARGET_DIR}/${SWIFT_HOST_ARCH}")
${INSTALL_TARGET_DIR}/${swift_arch})

if(BUILD_SHARED_LIBS)
set(library_kind SHARED)
else()
set(library_kind STATIC)
endif()
set(swiftDispatch_OUTPUT_FILE
${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_${library_kind}_LIBRARY_PREFIX}swiftDispatch${CMAKE_${library_kind}_LIBRARY_SUFFIX})
install(FILES
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the reason why I wrote in the review of the other commit that it would be better to just have this be taken care of by add_swift_target. It would ensure that these sorts of things are in sync.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Yes, adding an install option is reasonable.

${swiftDispatch_OUTPUT_FILE}
DESTINATION
${INSTALL_TARGET_DIR})
endif()

16 changes: 0 additions & 16 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,6 @@ if(BSD_OVERLAY_FOUND)
PRIVATE
${BSD_OVERLAY_LDFLAGS})
endif()
if(ENABLE_SWIFT)
target_link_libraries(bsdtestharness
PRIVATE
swiftCore-${swift_os}-${swift_arch}
swiftSwiftOnoneSupport-${swift_os}-${swift_arch})
endif()

function(add_unit_test name)
set(options DISABLED_TEST;NO_BSD_OVERLAY)
Expand All @@ -83,10 +77,6 @@ function(add_unit_test name)
# For testing in swift.org CI system; make deadlines lenient by default
# to reduce probability of test failures due to machine load.
target_compile_options(${name} PRIVATE -DLENIENT_DEADLINES=1)
target_link_libraries(${name}
PRIVATE
swiftCore-${swift_os}-${swift_arch}
swiftSwiftOnoneSupport-${swift_os}-${swift_arch})
endif()
target_include_directories(${name}
SYSTEM BEFORE PRIVATE
Expand Down Expand Up @@ -114,12 +104,6 @@ function(add_unit_test name)
PRIVATE
${BSD_OVERLAY_LDFLAGS})
endif()
if(ENABLE_SWIFT)
target_link_libraries(${name}
PRIVATE
swiftCore-${swift_os}-${swift_arch}
swiftSwiftOnoneSupport-${swift_os}-${swift_arch})
endif()
target_link_libraries(${name} PRIVATE bsdtests)
add_test(NAME ${name}
COMMAND bsdtestharness $<TARGET_FILE:${name}>)
Expand Down