Use FoundationEssentials if it's available #674

0xTim · 2024-10-24T11:52:56Z

Use FoundationEssentials if it's available. This stops all of Foundation being linked when not needed and reduces binary sizes

Checklist

I've added at least one test that validates that my change is working, if appropriate
I've followed the code style of the rest of the project
I've read the Contribution Guidelines
I've updated the documentation if necessary

MaxDesiatov · 2024-10-24T12:48:55Z

@swift-ci test

MaxDesiatov · 2024-10-24T14:07:04Z

@swift-ci test

lhoward · 2025-01-03T22:51:31Z

This feature will also be very useful to me. After applying the patch locally I still see libFoundationInternationalization.so being pulled in, though:

lukeh@liebling:~/CVSRoot/apple/swift-argument-parser$ ldd .build/debug/color
	linux-vdso.so.1 (0x0000fe8b15062000)
	libFoundationInternationalization.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libFoundationInternationalization.so (0x0000fe8b14800000)
	libFoundationEssentials.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libFoundationEssentials.so (0x0000fe8b14200000)
	libswiftSwiftOnoneSupport.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libswiftSwiftOnoneSupport.so (0x0000fe8b14fd0000)
	libswiftCore.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libswiftCore.so (0x0000fe8b13a00000)
	libswift_Concurrency.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libswift_Concurrency.so (0x0000fe8b14f20000)
	libswift_StringProcessing.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libswift_StringProcessing.so (0x0000fe8b14b10000)
	libswift_RegexParser.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libswift_RegexParser.so (0x0000fe8b140e0000)
	libswiftGlibc.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libswiftGlibc.so (0x0000fe8b14ef0000)
	libBlocksRuntime.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libBlocksRuntime.so (0x0000fe8b14ec0000)
	libdispatch.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libdispatch.so (0x0000fe8b14a90000)
	libswiftDispatch.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libswiftDispatch.so (0x0000fe8b14e70000)
	libFoundation.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libFoundation.so (0x0000fe8b13200000)
	libm.so.6 => /lib/aarch64-linux-gnu/libm.so.6 (0x0000fe8b13950000)
	libc.so.6 => /lib/aarch64-linux-gnu/libc.so.6 (0x0000fe8b13040000)
	/lib/ld-linux-aarch64.so.1 (0x0000fe8b15025000)
	lib_FoundationICU.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/lib_FoundationICU.so (0x0000fe8b10a00000)
	libgcc_s.so.1 => /lib/aarch64-linux-gnu/libgcc_s.so.1 (0x0000fe8b14a40000)
	libswiftSynchronization.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libswiftSynchronization.so (0x0000fe8b14a10000)
	libstdc++.so.6 => /lib/aarch64-linux-gnu/libstdc++.so.6 (0x0000fe8b10600000)

This is my last Foundation dependency for a particular daemon which will shrink the stripped/release/static stdlib size from 62MB to 14MB.

lhoward · 2025-01-04T05:28:22Z

After applying the patch:

lukeh@liebling:~/CVSRoot/apple/swift-argument-parser/.build/debug/ArgumentParser.build$ nm *.o|grep Foundation|grep -v FoundationEssentials|sort|uniq|grep "U "|awk '{print $2}'|xargs swift-demangle 
$s10Foundation7NSErrorCMa ---> type metadata accessor for Foundation.NSError
$ss5ErrorP10FoundationE20localizedDescriptionSSvg ---> (extension in Foundation):Swift.Error.localizedDescription.getter : Swift.String
$sSy10FoundationE20replacingOccurrences2of4with7options5rangeSSqd___qd_0_SS0A10EssentialsE14CompareOptionsVSnySS5IndexVGSgtSyRd__SyRd_0_r0_lF ---> (extension in Foundation):Swift.StringProtocol.replacingOccurrences<A, B where A1: Swift.StringProtocol, B1: Swift.StringProtocol>(of: A1, with: B1, options: (extension in FoundationEssentials):Swift.String.CompareOptions, range: Swift.Range<Swift.String.Index>?) -> Swift.String
$sSy10FoundationE5range2of7optionsAB6localeSnySS5IndexVGSgqd___SS0A10EssentialsE14CompareOptionsVAiJ6LocaleVSgtSyRd__lF ---> (extension in Foundation):Swift.StringProtocol.range<A where A1: Swift.StringProtocol>(of: A1, options: (extension in FoundationEssentials):Swift.String.CompareOptions, range: Swift.Range<Swift.String.Index>?, locale: FoundationEssentials.Locale?) -> Swift.Range<Swift.String.Index>?
$sSy10FoundationE8containsySbqd__SyRd__lF ---> (extension in Foundation):Swift.StringProtocol.contains<A where A1: Swift.StringProtocol>(A1) -> Swift.Bool

It turns out we need to enable Swift 6 language mode for the #if swift(>=6.0) guards to work. If we remove those:

/home/lukeh/CVSRoot/apple/swift-argument-parser/Sources/ArgumentParser/Usage/MessageInfo.swift: 0:
/home/lukeh/CVSRoot/apple/swift-argument-parser/Sources/ArgumentParser/Usage/MessageInfo.swift:14:23: error: class 'NSError' does not exist in module 'FoundationEssentials'
 12 | #if canImport(FoundationEssentials)
 13 | internal import protocol FoundationEssentials.LocalizedError
 14 | internal import class FoundationEssentials.NSError
    |                       `- error: class 'NSError' does not exist in module 'FoundationEssentials'

What we could do is only use LocalisedError and localizedDescription on Darwin.

That still leaves replacingOccurrences(of:with:), range(of:) and contains(). I suppose these shouldn't be difficult to reimplement.

natecook1000 · 2025-01-22T06:03:39Z

@swift-ci Please test

lhoward · 2025-01-22T06:09:10Z

Sources/ArgumentParser/Usage/MessageInfo.swift

@@ -9,9 +9,14 @@
 //
 //===----------------------------------------------------------------------===//

-#if swift(>=5.11)
+#if swift(>=6.0)


See previous comment that Swift 6 language mode needs to be enabled for these guards to work (which it is not in Package.swift).

Should be addressed by using #if compiler instead, as in provided suggestions.

Good catch, thanks @MaxDesiatov!

MaxDesiatov · 2025-01-22T12:22:40Z

Sources/ArgumentParser/Usage/MessageInfo.swift

@@ -9,9 +9,14 @@
 //
 //===----------------------------------------------------------------------===//

-#if swift(>=5.11)
+#if swift(>=6.0)


Suggested change

#if swift(>=6.0)

#if compiler(>=6.0)

MaxDesiatov · 2025-01-22T12:23:20Z

Sources/ArgumentParser/Usage/MessageInfo.swift

 internal import protocol Foundation.LocalizedError
 internal import class Foundation.NSError
+#endif
 #elseif swift(>=5.10)


Suggested change

#elseif swift(>=5.10)

#elseif compiler(>=5.10)

Ok this actually exposes an issue - NSError isn't part of Foundation Essentials. Are folks happy to remove that check?

0xTim · 2025-01-23T11:22:20Z

Sources/ArgumentParser/Usage/MessageInfo.swift

@@ -119,11 +120,7 @@ enum MessageInfo {
      case let error as LocalizedError where error.errorDescription != nil:
        self = .other(message: error.errorDescription!, exitCode: .failure)
      default:
-        if Swift.type(of: error) is NSError.Type {


NSError does not exist in Foundation Essentials so I've removed this check and defaulted to String(describing:). What are folks thoughts on this? Seems excessive to bring in all of Foundation just for this

If we really want to preserve the behavior we can use reflection (ArgumentParser already depends on it heavily), but that's really overkill.

@0xTim - what's the difference in behavior? Can you try throwing an NSError so we can make sure it isn't a quality regression?

lhoward · 2025-01-23T22:29:49Z

This still isn't working for me on Linux.

lukeh@liebling:~/CVSRoot/apple/swift-argument-parser$ swift --version
Swift version 6.0.2 (swift-6.0.2-RELEASE)
Target: aarch64-unknown-linux-gnu
lukeh@liebling:~/CVSRoot/apple/swift-argument-parser$ ldd .build/debug/color|grep libFoundation
	libFoundationInternationalization.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libFoundationInternationalization.so (0x0000f81b2a400000)
	libFoundationEssentials.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libFoundationEssentials.so (0x0000f81b29e00000)
	libFoundation.so => /opt/swift-6.0.2-RELEASE-ubuntu24.04-aarch64/usr/lib/swift/linux/libFoundation.so (0x0000f81b28e00000)

The issue is the one I described before:

lukeh@liebling:~/CVSRoot/apple/swift-argument-parser/.build/debug$ nm color|grep ' U $sSy10Foundation'
                 U $sSy10FoundationE20replacingOccurrences2of4with7options5rangeSSqd___qd_0_SS0A10EssentialsE14CompareOptionsVSnySS5IndexVGSgtSyRd__SyRd_0_r0_lF
                 U $sSy10FoundationE5range2of7optionsAB6localeSnySS5IndexVGSgqd___SS0A10EssentialsE14CompareOptionsVAiJ6LocaleVSgtSyRd__lF
                 U $sSy10FoundationE8containsySbqd__SyRd__lF

Use FoundationEssentials if it's available

4710201

MaxDesiatov requested a review from rauhul October 24, 2024 12:47

MaxDesiatov approved these changes Oct 24, 2024

View reviewed changes

0xTim added 2 commits October 24, 2024 13:26

Fix Foundation imports

7bbd82e

Fix test warning

cc24c58

parkera requested a review from natecook1000 October 25, 2024 22:48

Merge branch 'main' into foundation-essentials

af61872

lhoward reviewed Jan 22, 2025

View reviewed changes

MaxDesiatov reviewed Jan 22, 2025

View reviewed changes

Correct Swift check

87a00bb

0xTim commented Jan 23, 2025

View reviewed changes

t089 mentioned this pull request Mar 14, 2025

[Linux] Adding swift-argument-parser as a dependency increases binary size to 50+MB (10x larger than without) #748

Open

2 tasks

t089 mentioned this pull request Mar 25, 2025

Only use FoundationEssentials if available #749

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use FoundationEssentials if it's available #674

Use FoundationEssentials if it's available #674

0xTim commented Oct 24, 2024 •

edited

Loading

MaxDesiatov commented Oct 24, 2024

MaxDesiatov commented Oct 24, 2024

lhoward commented Jan 3, 2025 •

edited

Loading

lhoward commented Jan 4, 2025 •

edited

Loading

natecook1000 commented Jan 22, 2025

lhoward Jan 22, 2025

MaxDesiatov Jan 22, 2025

natecook1000 Jan 22, 2025

MaxDesiatov Jan 22, 2025

MaxDesiatov Jan 22, 2025

0xTim Jan 23, 2025

0xTim Jan 23, 2025

stevapple Jan 23, 2025

natecook1000 Jan 23, 2025

lhoward commented Jan 23, 2025

Use FoundationEssentials if it's available #674

Are you sure you want to change the base?

Use FoundationEssentials if it's available #674

Conversation

0xTim commented Oct 24, 2024 • edited Loading

Checklist

MaxDesiatov commented Oct 24, 2024

MaxDesiatov commented Oct 24, 2024

lhoward commented Jan 3, 2025 • edited Loading

lhoward commented Jan 4, 2025 • edited Loading

natecook1000 commented Jan 22, 2025

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhoward commented Jan 23, 2025

0xTim commented Oct 24, 2024 •

edited

Loading

lhoward commented Jan 3, 2025 •

edited

Loading

lhoward commented Jan 4, 2025 •

edited

Loading