Skip to content

Introduce CMake support for SwiftFoundationICU #22

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

Merged
merged 1 commit into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.DS_Store
/.swiftpm
/.build
/build
/Packages
/*.xcodeproj
xcuserdata/
Expand Down
85 changes: 85 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
##===----------------------------------------------------------------------===##
##
## This source file is part of the Swift open source project
##
## Copyright (c) 2024 Apple Inc. and the Swift project authors
## Licensed under Apache License v2.0
##
## See LICENSE.txt for license information
## See CONTRIBUTORS.md for the list of Swift project authors
##
## SPDX-License-Identifier: Apache-2.0
##
##===----------------------------------------------------------------------===##

cmake_minimum_required(VERSION 3.24)
Copy link
Member

Choose a reason for hiding this comment

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

Is this for CI? It might be worth trying this build across a few versions of CMake and doing something like this

cmake_minimum_required(VERSION 3.24...3.29)

That way we pick up CMP0156 and CMP0157. We definitely want these two if folks have a new enough CMake since it will fix the linker warning about duplicate libraries and will result in faster incremental builds due to the better Swift build graphs in CMake 3.29.

At the very least, between the cmake_minimum_required and project invocations, can you enable those policies? Something like this:

cmake_minimum_required(VERSION 3.24)

if(POLICY CMP0156)
    # Deduplicate linked libraries where appropriate
    cmake_policy(SET CMP0156 NEW)
endif()

if(POLICY CMP0157)
    # New Swift build model: improved incremental build performance and LSP support
    cmake_policy(SET CMP0157 NEW)
endif()

project(SwiftFoundationICU
    LANGUAGES CXX Swift)
...


project(SwiftFoundationICU
LANGUAGES CXX Swift)

set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD_REQUIRED ON)

list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/modules)
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
set(CMAKE_Swift_MODULE_DIRECTORY ${CMAKE_BINARY_DIR}/swift)

# Build flags
add_compile_definitions(
$<$<COMPILE_LANGUAGE:C,CXX>:U_ATTRIBUTE_DEPRECATED="">
$<$<COMPILE_LANGUAGE:C,CXX>:U_SHOW_CPLUSPLUS_API=1>
$<$<COMPILE_LANGUAGE:C,CXX>:U_SHOW_INTERNAL_API=1>
$<$<COMPILE_LANGUAGE:C,CXX>:U_STATIC_IMPLEMENTATION>
$<$<COMPILE_LANGUAGE:C,CXX>:U_TIMEZONE_PACKAGE="icutz44l">
$<$<COMPILE_LANGUAGE:C,CXX>:FORTIFY_SOURCE=2>
$<$<COMPILE_LANGUAGE:C,CXX>:STD_INSPIRED>
$<$<COMPILE_LANGUAGE:C,CXX>:MAC_OS_X_VERSION_MIN_REQUIRED=101500>
$<$<COMPILE_LANGUAGE:C,CXX>:U_HAVE_STRTOD_L=1>
$<$<COMPILE_LANGUAGE:C,CXX>:U_HAVE_XLOCALE_H=1>
$<$<COMPILE_LANGUAGE:C,CXX>:U_HAVE_STRING_VIEW=1>
$<$<COMPILE_LANGUAGE:C,CXX>:U_DISABLE_RENAMING=1>
$<$<COMPILE_LANGUAGE:C,CXX>:U_COMMON_IMPLEMENTATION>
$<$<COMPILE_LANGUAGE:C,CXX>:ICU_DATA_DIR="/usr/share/icu">
$<$<COMPILE_LANGUAGE:C,CXX>:USE_PACKAGE_DATA=1>
$<$<COMPILE_LANGUAGE:C,CXX>:APPLE_ICU_CHANGES=1>)

# Window specific settings
if(CMAKE_SYSTEM_NAME STREQUAL Windows)
add_compile_definitions(
$<$<COMPILE_LANGUAGE:C,CXX>:U_TIMEZONE=_timezone>
$<$<COMPILE_LANGUAGE:C,CXX>:_CRT_SECURE_NO_DEPRECATE>
$<$<COMPILE_LANGUAGE:C,CXX>:U_PLATFORM_USES_ONLY_WIN32_API>)
else()
add_compile_definitions(
$<$<COMPILE_LANGUAGE:C,CXX>:U_TIMEZONE=timezone>)
endif()
# WASI specific settings
if(CMAKE_SYSTEM_NAME STREQUAL WASI)
add_compile_definitions(
$<$<COMPILE_LANGUAGE:C,CXX>:U_HAVE_TZSET=0>
$<$<COMPILE_LANGUAGE:C,CXX>:U_HAVE_TZNAME=0>
$<$<COMPILE_LANGUAGE:C,CXX>:U_HAVE_TIMEZONE=0>
$<$<COMPILE_LANGUAGE:C,CXX>:_WASI_EMULATED_SIGNAL>
$<$<COMPILE_LANGUAGE:C,CXX>:_WASI_EMULATED_MMAN>)
add_link_options("-Lwasi-emulated-signal")
add_link_options("-Lwasi-emulated-mman")
endif()

if(BUILD_SHARED_LIBS)
set(install_directory swift)
else()
set(install_directory swift_static)
endif()

if(NOT SWIFT_SYSTEM_NAME)
if(CMAKE_SYSTEM_NAME STREQUAL Darwin)
set(SWIFT_SYSTEM_NAME macosx)
else()
set(SWIFT_SYSTEM_NAME "$<LOWER_CASE:${CMAKE_SYSTEM_NAME}>")
endif()
endif()

add_subdirectory(icuSources)
add_subdirectory(cmake/modules)
Copy link
Member

Choose a reason for hiding this comment

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

I don't love jumping into the cmake/modules like this. I'd prefer to configure the exports file from here, but I'm not strongly opinionated on this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm following the pattern from all of there other packages.. they mostly seem to use this way (unless I'm mistaken)

Copy link
Member

Choose a reason for hiding this comment

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

Eh, we can leave it for now.

95 changes: 18 additions & 77 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,15 @@ var buildSettings: [CXXSetting] = [
// Where data are stored
.define("ICU_DATA_DIR", to: "\"/usr/share/icu/\""),
.define("USE_PACKAGE_DATA", to: "1"),
.define("APPLE_ICU_CHANGES", to: "1")
.define("APPLE_ICU_CHANGES", to: "1"),

.headerSearchPath("common"),
.headerSearchPath("io"),
.headerSearchPath("i18n"),
.headerSearchPath("include"),
.define("U_I18N_IMPLEMENTATION"),
.define("SWIFT_PACKAGE", to: "1", .when(platforms: [.linux])),
.define("U_IO_IMPLEMENTATION"),
]

#if os(Windows)
Expand All @@ -48,30 +56,6 @@ buildSettings.append(contentsOf: [
])
#endif

let commonBuildSettings: [CXXSetting] = buildSettings.appending([
.headerSearchPath("."),
])

let i18nBuildSettings: [CXXSetting] = buildSettings.appending([
.define("U_I18N_IMPLEMENTATION"),
.define("SWIFT_PACKAGE", to: "1", .when(platforms: [.linux])),
.headerSearchPath("../common"),
.headerSearchPath("."),
])

let ioBuildSettings: [CXXSetting] = buildSettings.appending([
.define("U_IO_IMPLEMENTATION"),
.headerSearchPath("../common"),
.headerSearchPath("../i18n"),
.headerSearchPath("."),
])

let stubDataBuildSettings: [CXXSetting] = buildSettings.appending([
.headerSearchPath("../common"),
.headerSearchPath("../i18n"),
.headerSearchPath("."),
])

let linkerSettings: [LinkerSetting] = [
.linkedLibrary("wasi-emulated-signal", .when(platforms: [.wasi])),
.linkedLibrary("wasi-emulated-mman", .when(platforms: [.wasi])),
Expand All @@ -82,62 +66,19 @@ let package = Package(
products: [
.library(
name: "_FoundationICU",
targets: ["_FoundationICU"]),
.library(
name: "_FoundationICUCommon",
targets: ["_FoundationICUCommon"]),
.library(
name: "_FoundationICUI18N",
targets: ["_FoundationICUI18N"]),
.library(
name: "_FoundationICUIO",
targets: ["_FoundationICUIO"]),
targets: [
"_FoundationICU"
]
)
],
targets: [
.target(
name: "_FoundationICU",
dependencies: [
"_FoundationICUCommon",
"_FoundationICUI18N",
"_FoundationICUIO",
"_FoundationICUStubData"
],
path: "swift/FoundationICU"),
.target(
name: "_FoundationICUCommon",
path: "icuSources/common",
publicHeadersPath: "include",
cxxSettings: commonBuildSettings),
.target(
name: "_FoundationICUI18N",
dependencies: ["_FoundationICUCommon"],
path: "icuSources/i18n",
publicHeadersPath: "include",
cxxSettings: i18nBuildSettings),
.target(
name: "_FoundationICUIO",
dependencies: ["_FoundationICUCommon", "_FoundationICUI18N"],
path: "icuSources/io",
publicHeadersPath: "include",
cxxSettings: ioBuildSettings),
.target(
name: "_FoundationICUStubData",
dependencies: ["_FoundationICUCommon"],
path: "icuSources/stubdata",
publicHeadersPath: ".",
cxxSettings: stubDataBuildSettings),
path: "icuSources",
exclude: ["stubdata"],
cxxSettings: buildSettings,
linkerSettings: linkerSettings
)
],
cxxLanguageStandard: .cxx14
)

for target in package.targets {
target.linkerSettings = linkerSettings
}

fileprivate extension Array {
func appending(_ other: Self) -> Self {
var me = self
me.append(contentsOf: other)
return me
}
}
24 changes: 24 additions & 0 deletions cmake/modules/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
##===----------------------------------------------------------------------===##
##
## This source file is part of the Swift open source project
##
## Copyright (c) 2024 Apple Inc. and the Swift project authors
## Licensed under Apache License v2.0
##
## See LICENSE.txt for license information
## See CONTRIBUTORS.md for the list of Swift project authors
##
## SPDX-License-Identifier: Apache-2.0
##
##===----------------------------------------------------------------------===##

set(SWIFT_FOUNDATION_ICU_EXPORTS_FILE ${CMAKE_CURRENT_BINARY_DIR}/SwiftFoundationICUExports.cmake)

configure_file(SwiftFoundationICUConfig.cmake.in
${CMAKE_CURRENT_BINARY_DIR}/SwiftFoundationICUConfig.cmake)

get_property(SWIFT_FOUNDATION_ICU_EXPORTS GLOBAL PROPERTY SWIFT_FOUNDATION_ICU_EXPORTS)
export(TARGETS ${SWIFT_FOUNDATION_ICU_EXPORTS}
Copy link
Member

Choose a reason for hiding this comment

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

Do you only want to export these targets, or do you want to install them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I only want to install FoundationICU because that's the actual "exported" target.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I think we can leave the install story up in the air for the time being. If we need to figure it out, we can come up with how that should work later.

NAMESPACE SwiftFoundationICU::
FILE ${SWIFT_FOUNDATION_ICU_EXPORTS_FILE}
EXPORT_LINK_INTERFACE_LIBRARIES)
17 changes: 17 additions & 0 deletions cmake/modules/SwiftFoundationICUConfig.cmake.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
##===----------------------------------------------------------------------===##
##
## This source file is part of the Swift open source project
##
## Copyright (c) 2024 Apple Inc. and the Swift project authors
## Licensed under Apache License v2.0
##
## See LICENSE.txt for license information
## See CONTRIBUTORS.md for the list of Swift project authors
##
## SPDX-License-Identifier: Apache-2.0
##
##===----------------------------------------------------------------------===##

if(NOT TARGET SwiftFoundationICU)
include(@SWIFT_FOUNDATION_ICU_EXPORTS_FILE@)
endif()
45 changes: 45 additions & 0 deletions icuSources/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
##===----------------------------------------------------------------------===##
##
## This source file is part of the SwiftFoundation open source project
##
## Copyright (c) 2024 Apple Inc. and the SwiftFoundation project authors
## Licensed under Apache License v2.0
##
## See LICENSE.txt for license information
## See CONTRIBUTORS.md for the list of SwiftFoundation project authors
##
## SPDX-License-Identifier: Apache-2.0
##
##===----------------------------------------------------------------------===##

add_library(_FoundationICU)

target_include_directories(_FoundationICU
PUBLIC
include/)

add_subdirectory(common)
add_subdirectory(i18n)
add_subdirectory(io)

target_link_libraries(_FoundationICU PRIVATE stdc++)
Copy link
Member

Choose a reason for hiding this comment

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

This is an odd one since we have to guess what flavor of C++ folks are using since the Swift driver decided to not help us out and tell us what it's using (actually, I think part of the problem is that it doesn't know what flavor of C++ we're using either leaving it as an exercise to the user to pass all the right flags).
I know this isn't going to work for Android (uses libcxx) nor for Windows (which can use the Windows C++ runtime, but I think libcxx might also work there too so it could go either way). I think it should be customizable at distribution time since the person putting together the SDK will likely have the best chance of knowing which C++ runtime they're building the SDK with. At least until the Swift compiler learns how to specify what C++ runtime you're working with.

In theory, we could do something like this to provide reasonable defaults:

if(Linux)
set(SwiftFoundation_CXX_RUNTIME_default stdc++)
elseif(Android)
set(SwiftFoundation_CXX_RUNTIME_default c++)
elseif(WIN32)
set(SwiftFoundation_CXX_RUNTIME_default WhateverWindowsUses)
endif()
set(SwiftFoundation_CXX_RUNTIME "${SwiftFoundation_CXX_RUNTIME_default}" CACHE STRING
    "C++ runtime that SwiftFoundation is built against")

@compnerd, do you have a better idea?

Copy link
Member

Choose a reason for hiding this comment

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

Why not leave it to the user? If we set the linker language to C++, the C++ driver will be used. The user can add in -stdlib=libc++ or -stdlib=libstdc++ if they are using clang. If they are using cl, that will default to the MS STL, as would clang-cl.

Copy link
Member

Choose a reason for hiding this comment

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

This is related to using c++ interop at link time. The one thing that we should be careful of is if that gets serialized into the module.

Copy link
Contributor Author

@iCharlesHu iCharlesHu Jun 10, 2024

Choose a reason for hiding this comment

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

It turns out we don't actually need it... I deleted it.

target_compile_definitions(_FoundationICU PRIVATE U_I18N_IMPLEMENTATION U_IO_IMPLEMENTATION)
if(CMAKE_SYSTEM_NAME STREQUAL Linux)
target_compile_definitions(_FoundationICU PRIVATE SWIFT_PACKAGE=1)
endif()
set_target_properties(_FoundationICU PROPERTIES
INSTALL_RPATH "$ORIGIN")

# Install binary
install(TARGETS _FoundationICU
ARCHIVE DESTINATION lib/${install_directory}/${SWIFT_SYSTEM_NAME}
LIBRARY DESTINATION lib/${install_directory}/${SWIFT_SYSTEM_NAME}
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})

# Install headers
install(DIRECTORY
include/
DESTINATION
lib/${install_directory})

set_property(GLOBAL APPEND PROPERTY SWIFT_FOUNDATION_ICU_EXPORTS _FoundationICU)
Loading