[stdlib] Drop ICU #40340

Azoy · 2021-11-30T21:56:57Z

Now that the stdlib no longer needs ICU, let's drop the dependency 🙂

This is marked as a draft because right now this only removes the immediate dependency on the stdlib, and isn't touching any of the larger build rules. I wanted to get others opinions on how best to handle that (@compnerd @gottesmm ). I think some of the build rules in this repository are still needed for corelibs-foundation no?

Also, this updates the freestanding symbol list (@kubamracek ).

If there are others that you think I should cc please let me know!

update freestanding deps

compnerd · 2021-11-30T22:06:01Z

I really love this change, but I think that this needs a bit more work. I don't think we need to worry about the dependency in Foundation - the rest of the build will take care of that, it doesn't pull it in from swift itself, but rather through its own build. We can tweak the ordering subsequently, but there is more build cleanup that should be done.

Off the top of my head, delete:
https://github.com/apple/swift/blob/main/CMakeLists.txt#L1021-L1050
https://github.com/apple/swift/blob/main/cmake/modules/SwiftConfigureSDK.cmake#L68-L75
https://github.com/apple/swift/blob/main/stdlib/cmake/modules/AddSwiftStdlib.cmake#L464-L473
https://github.com/apple/swift/blob/main/stdlib/cmake/modules/AddSwiftStdlib.cmake#L1316-L1318
https://github.com/apple/swift/blob/main/cmake/modules/FindICU.cmake

We don't need to worry about the rest of the cleanup (i.e. build-script or build-script-impl) which need to pass the parameters around. That can be done subsequently and will only make a minor difference. The important part here is to clean up the large amount of cruft that is custom.

compnerd · 2021-11-30T21:58:56Z

utils/check_freestanding_dependencies.py

-    "_ubrk_setText", "_ubrk_setUText", "_unorm2_getNFCInstance",
-    "_unorm2_hasBoundaryBefore", "_unorm2_normalize",
-    "_unorm2_spanQuickCheckYes", "_utext_openUChars", "_utext_openUTF8",
-]


❤️ ❤️ ❤️

milseman · 2021-11-30T22:13:45Z

stdlib/public/stubs/UnicodeShims.cpp

-// Force an autolink with ICU
-#if defined(__MACH__)
-asm(".linker_option \"-licucore\"\n");
-#endif // defined(__MACH__)


If the stdlib isn't doing this anymore, would importing Foundation need to?

Foundation would, and already does so by means of injection. The need for this is strictly for static linking. When statically linking, the dependencies must be explicitly listed and this is a horrible workaround to avoid the user from having to specify that and the driver not learning the dependency. This can be safely removed IMO.

https://github.com/apple/swift-corelibs-foundation/blob/main/Sources/Foundation/CMakeLists.txt#L171-L182 for the hackery that I was referring to - the dependency is injected via the swift driver at compile time.

belkadan · 2021-11-30T22:31:16Z

stdlib/public/core/CMakeLists.txt

@@ -238,29 +237,6 @@ set(swift_core_link_flags "${SWIFT_RUNTIME_SWIFT_LINK_FLAGS}")
 set(swift_core_framework_depends)
 set(swift_core_private_link_libraries)
 set(swift_stdlib_compile_flags "${SWIFT_RUNTIME_SWIFT_COMPILE_FLAGS}")
-if(SWIFT_PRIMARY_VARIANT_SDK IN_LIST SWIFT_DARWIN_PLATFORMS)
-  list(APPEND swift_core_link_flags "-all_load")


(from outside) Was ICU the only thing using -all_load? Or is this something that can be removed separately? Or is it still needed?

Yes, ICU is the only think that the standard library links against (outside of system libraries like ... libSystem) explicitly. The rest of the dependencies that need to be linked against will be from the clang triggered auto-link. INCORPORATE_OBJECT_LIBRARIES does properly use the TARGET_OBJECTS generator to unpack the archive and link them so no unintentional DCE should occur.

It was swiftRuntime.a itself I was worried about, but if that's going through INCORPORATE_OBJECT_LIBRARIES then everything should be okay.

Right, that is handled via the incorporate object libraries and so we don't need to worry about the potential DCE.

compnerd · 2021-11-30T23:10:27Z

@swift-ci please test Windows platform

compnerd

Thanks for cleaning this up, I'm super excited to see this finally happening.

Azoy · 2021-12-01T03:04:39Z

@swift-ci please test

gwynne

I love everything about this. Amazing work! ❤️

gwynne · 2021-12-01T03:18:16Z

stdlib/public/core/UnicodeHelpers.swift

@@ -388,7 +388,7 @@ extension _StringGuts {
    _internalInvariant(self.isForeign)

    // Both a fast-path for single-code-unit graphemes and validation:
-    //   ICU treats isolated surrogates as isolated graphemes
+    //    treat isolated surrogates as isolated graphemes


Suggested change

// treat isolated surrogates as isolated graphemes

// Treat isolated surrogates as isolated graphemes.

swift-ci · 2021-12-01T05:05:49Z

Build failed
Swift Test Linux Platform
Git Sha - 86fec40

finagolfin · 2021-12-01T05:30:42Z

Looks like the linux CI was unable to run the Foundation tests because of a missing dependency on ICU, looks like you'll need to fix Foundation too.

vgorloff · 2021-12-02T21:00:09Z

Am I properly understand that stdlib (from git repository https://github.com/apple/swift.git) and libFoundation (from git repository https://github.com/apple/swift-corelibs-foundation.git) will not have build time and runtime dependencies on ICU?

Or this PR is only about build time dependencies, thus, assuming that concrete system will have shared library for runtime execution e.g. libicui18n.so?

For example when targeting Android, the ICU needs to be renamed (e.g. libicui18n.so -> libicui18nswift.so) to avoid conflicts with system provided version of ICU (which might be too old).

Few Android examples:

finagolfin · 2021-12-03T08:52:35Z

@vgorloff, my understanding is that a series of pulls by @Azoy have already removed all external ICU dependencies from this repo, but that swift-corelibs-foundation still requires it: see comments above for more info. That's probably why this pull currently fails on the linux CI, because the foundation tests inadvertently relied on this CMake configuration.

stevapple · 2021-12-03T11:50:08Z

I’m curious about why it passed all the test on Windows😂 It seems Windows and Linux CI shared the same way of detecting and linking ICU.

finagolfin · 2021-12-03T14:48:44Z

Both the macOS and Windows CI don't run the swift-corelibs-foundation tests, only the linux CI does.

Azoy · 2021-12-04T02:18:30Z

swiftlang/swift-corelibs-foundation#3118

@swift-ci please test

swift-ci · 2021-12-04T03:11:42Z

Build failed
Swift Test Linux Platform
Git Sha - 4fbbd0c054669d3b2ab08fddb751eba1cc116dc1

finagolfin · 2021-12-04T06:26:11Z

The linux CI couldn't check out your new foundation pull for some reason, maybe spurious.

Azoy · 2021-12-04T07:07:08Z

swiftlang/swift-corelibs-foundation#3118

@swift-ci please clean test Linux

finagolfin

Nice work.

Azoy · 2021-12-06T23:19:40Z

@swift-ci please clean test Linux

Azoy · 2021-12-07T05:47:42Z

@compnerd @buttaface this look good to go?

compnerd · 2021-12-07T05:51:07Z

LGTM, but please also let @buttaface have another look.

finagolfin · 2021-12-07T07:43:23Z

LGTM

…swiftlang#40340

[android] Remove ICU build flags since that requirement was dropped in #40340

…swiftlang#40340

…ilt separately This was dropped in #40340.

…ilt separately This was dropped in swiftlang#40340. (cherry picked from commit abe9225)

…ilt separately (#79274) This was dropped in #40340. (cherry picked from commit abe9225) Co-authored-by: Finagolfin <finagolfin@tuta.io>

Drop ICU

21ee3a5

update freestanding deps

Azoy requested review from compnerd, gottesmm, kubamracek, milseman, Catfish-Man and kylemacomber November 30, 2021 21:56

compnerd requested changes Nov 30, 2021

View reviewed changes

milseman reviewed Nov 30, 2021

View reviewed changes

belkadan reviewed Nov 30, 2021

View reviewed changes

Cleanup more of the build from Saleem's comments

86fec40

compnerd approved these changes Nov 30, 2021

View reviewed changes

gwynne approved these changes Dec 1, 2021

View reviewed changes

finagolfin mentioned this pull request Dec 2, 2021

[build] rewrite #21707 so that it doesn't break cross-compilation #30170

Open

Azoy mentioned this pull request Dec 4, 2021

[build] Explicitly link against icudata swiftlang/swift-corelibs-foundation#3118

Closed

finagolfin approved these changes Dec 4, 2021

View reviewed changes

Azoy marked this pull request as ready for review December 4, 2021 11:06

stevapple mentioned this pull request Dec 7, 2021

[200] Issue #200 - December 16, 2021 SwiftWeekly/swiftweekly.github.io#708

Closed

Azoy merged commit 8a5f728 into swiftlang:main Dec 7, 2021

Azoy deleted the drop-icu branch December 7, 2021 09:44

finagolfin added a commit to finagolfin/swift that referenced this pull request Dec 20, 2021

[android] Remove ICU build flags since that requirement was dropped in …

a3ea28f

…swiftlang#40340

finagolfin mentioned this pull request Dec 21, 2021

[build] Remove last vestiges of ICU for anything other than Foundation #40659

Merged

compnerd added a commit that referenced this pull request Dec 23, 2021

Merge pull request #40625 from buttaface/droid-remove-icu

d1bb98b

[android] Remove ICU build flags since that requirement was dropped in #40340

filip-sakel mentioned this pull request Dec 24, 2021

Allow WASI-less builds with --triple wasm32-unknown-unknown swiftwasm/swift#2412

Closed

finagolfin added a commit to finagolfin/swift that referenced this pull request Jan 12, 2022

[android] Remove ICU build flags since that requirement was dropped in …

ef821d5

…swiftlang#40340

finagolfin mentioned this pull request Jan 12, 2022

[5.6][build] Clean up ICU detritus that is no longer used to build the stdlib #40817

Merged

Azoy mentioned this pull request Dec 7, 2021

[SR-10535] [stdlib] Drop ICU dependency #52935

Closed

This was referenced Jan 27, 2022

[SR-11744] Decrease Swift Runtime Size (for use on embedded devices) swiftlang/swift-corelibs-foundation#3376

Open

[SR-12394] plutil executable is broken on linux swiftlang/swift-corelibs-foundation#3262

Closed

ferologics mentioned this pull request Jun 14, 2022

build wasm file is very big??? swiftwasm/swift#7

Open

finagolfin mentioned this pull request Sep 1, 2022

Debian packaging support #60690

Open

finagolfin mentioned this pull request Mar 14, 2023

[build] Remove runpath from build host from shared ICU libraries on linux #64365

Merged

finagolfin mentioned this pull request May 5, 2023

[5.8][build] Remove runpath from build host from shared ICU libraries on linux #65692

Merged

finagolfin mentioned this pull request Jan 16, 2025

Linux: don't link libicu in static executables, as it is no longer built separately #78678

Merged

finagolfin added a commit that referenced this pull request Jan 17, 2025

Linux: don't link libicu in static executables, as it is no longer bu…

abe9225

…ilt separately This was dropped in #40340.

rjmansfield pushed a commit to rjmansfield/swift that referenced this pull request Feb 10, 2025

Linux: don't link libicu in static executables, as it is no longer bu…

b0b7a66

…ilt separately This was dropped in swiftlang#40340. (cherry picked from commit abe9225)

rjmansfield mentioned this pull request Feb 10, 2025

🍒 Linux: don't link libicu in static executables, as it is no longer built separately #79274

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[stdlib] Drop ICU #40340

[stdlib] Drop ICU #40340

Azoy commented Nov 30, 2021

compnerd commented Nov 30, 2021 •

edited

Loading

compnerd Nov 30, 2021

kubamracek Nov 30, 2021

finagolfin Dec 1, 2021

milseman Nov 30, 2021

compnerd Nov 30, 2021 •

edited

Loading

compnerd Nov 30, 2021

belkadan Nov 30, 2021

compnerd Nov 30, 2021

belkadan Nov 30, 2021

compnerd Nov 30, 2021

compnerd commented Nov 30, 2021

compnerd left a comment

Azoy commented Dec 1, 2021

gwynne left a comment

gwynne Dec 1, 2021 •

edited

Loading

swift-ci commented Dec 1, 2021

finagolfin commented Dec 1, 2021

vgorloff commented Dec 2, 2021

finagolfin commented Dec 3, 2021

stevapple commented Dec 3, 2021

finagolfin commented Dec 3, 2021

Azoy commented Dec 4, 2021

swift-ci commented Dec 4, 2021

finagolfin commented Dec 4, 2021

Azoy commented Dec 4, 2021

finagolfin left a comment

Azoy commented Dec 6, 2021

Azoy commented Dec 7, 2021

compnerd commented Dec 7, 2021

finagolfin commented Dec 7, 2021

	// treat isolated surrogates as isolated graphemes
	// Treat isolated surrogates as isolated graphemes.

[stdlib] Drop ICU #40340

[stdlib] Drop ICU #40340

Conversation

Azoy commented Nov 30, 2021

compnerd commented Nov 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compnerd Nov 30, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compnerd commented Nov 30, 2021

compnerd left a comment

Choose a reason for hiding this comment

Azoy commented Dec 1, 2021

gwynne left a comment

Choose a reason for hiding this comment

gwynne Dec 1, 2021 • edited Loading

Choose a reason for hiding this comment

swift-ci commented Dec 1, 2021

finagolfin commented Dec 1, 2021

vgorloff commented Dec 2, 2021

finagolfin commented Dec 3, 2021

stevapple commented Dec 3, 2021

finagolfin commented Dec 3, 2021

Azoy commented Dec 4, 2021

swift-ci commented Dec 4, 2021

finagolfin commented Dec 4, 2021

Azoy commented Dec 4, 2021

finagolfin left a comment

Choose a reason for hiding this comment

Azoy commented Dec 6, 2021

Azoy commented Dec 7, 2021

compnerd commented Dec 7, 2021

finagolfin commented Dec 7, 2021

compnerd commented Nov 30, 2021 •

edited

Loading

compnerd Nov 30, 2021 •

edited

Loading

gwynne Dec 1, 2021 •

edited

Loading