Skip to content
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

Fix autolink mechanism for static libraries on Linux #2996

Merged
merged 2 commits into from
Aug 7, 2021

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Apr 27, 2021

Resolve: [SR-14536] Importing FoundationNetworking with -static-stdlib is broken with missing symbols on Linux - Swift

Discussion: Autolinking behavior of @_implementationOnly with static linking - Development / Core Libraries - Swift Forums

Blocked by [Frontend] Add -public-autolink-library option by kateinoigakukun · Pull Request #35936 · apple/swift

Overview

Foundation imports CoreFoundation with @_implementationOnly as a private dependency, and it doesn't emit IMPORTED_MODULE in swiftmodule.
Users of Foundation can link libCoreFoundation.so automatically when using dynamic linking because it's already linked into libFoundation.so.
However, this automatic linking doesn't work when static linking, so users need to link CoreFoundation and dependent libraries of CoreFoundation explicitly by adding -lCoreFoundation -lBlocksRuntime -licui18n.

To avoid forcing users to link private dependencies explicitly, this patch changed to merge the dependencies into libFoundation.a. And also to avoid forcing to link dependent libs of CoreFoundation like curl or icui18n, add LINK_LIBRARY entry in Foundation.swiftmodule using -public-autolink-library which is introduced by swiftlang/swift#35936

@kateinoigakukun kateinoigakukun force-pushed the katei/merge-private-libs branch from 9ceb356 to fc0c1c8 Compare April 27, 2021 14:44
@kateinoigakukun kateinoigakukun changed the title Merge private dependencies into single static library Fix autolink mechanism for static libraries on Linux Apr 27, 2021
@spevans
Copy link
Contributor

spevans commented Apr 27, 2021

When you have this working could you add an integration test into the https://github.com/apple/swift-integration-tests repository to catch any future regressions? Thanks.

There are some example tests in
https://github.com/apple/swift-integration-tests/blob/main/test-foundation-package/test-foundation-networking.swift
https://github.com/apple/swift-integration-tests/tree/main/test-static-stdlib

@kateinoigakukun
Copy link
Member Author

@spevans Okey, I'll add a test case in the repo. Thanks for letting me know :)

@xwu
Copy link
Contributor

xwu commented Apr 28, 2021

cc @compnerd

`Foundation` imports `CoreFoundation` with `@_implementationOnly` as a
private dependency, and it doesn't emit `IMPORTED_MODULE` in
swiftmodule.
Users of Foundation can link `libCoreFoundation.so` automatically when
using dynamic linking because it's already linked into
`libFoundation.so`.
However, this automatic linking doesn't work when static linking, so
users need to link CoreFoundation and dependent libraries of
CoreFoundation explicitly by adding `-lCoreFoundation -lBlocksRuntime
-licui18n`.

To avoid forcing users to link private dependencies explicitly, this
patch changed to merge the dependencies into libFoundation.a. And also
to avoid forcing to link dependent libs of CoreFoundation like `curl` or
`icui18n`, add `LINK_LIBRARY` entry in Foundation.swiftmodule using
`-public-autolink-library` which is introduced by
swiftlang/swift#35936
@MaxDesiatov
Copy link
Contributor

Please test with following PR:
swiftlang/swift#35936

@swift-ci Please test

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented Jun 17, 2021

swiftlang/swift#35936 has been merged into main branch, so there is no blocker to merge this. Please proceed review again 🙏

@kateinoigakukun
Copy link
Member Author

Gentle ping :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants