Skip to content

Serialization: avoid pushing static libraries into autolink#63814

Open
compnerd wants to merge 1 commit into
swiftlang:mainfrom
compnerd:double-static
Open

Serialization: avoid pushing static libraries into autolink#63814
compnerd wants to merge 1 commit into
swiftlang:mainfrom
compnerd:double-static

Conversation

@compnerd

Copy link
Copy Markdown
Member

Remove the static libraries from autolinking. This should be safe to do with SPM builds which does not respect the static linking and instead treats all targets as object libraries for static linking. With CMake this should also not be a problem as it already requires dependencies be spelt out and then correctly forms the linker invocation. This prevents us from double linking a library (both into a potential intermediate shared library and any consumers of any public interfaces).

Remove the static libraries from autolinking.  This should be safe to do
with SPM builds which does not respect the static linking and instead
treats all targets as object libraries for static linking.  With CMake
this should also not be a problem as it already requires dependencies be
spelt out and then correctly forms the linker invocation.  This prevents
us from double linking a library (both into a potential intermediate
shared library and any consumers of any public interfaces).
@compnerd compnerd marked this pull request as ready for review February 21, 2023 23:17
@compnerd

Copy link
Copy Markdown
Member Author

CC: @bnbarham @etcwilde @neonichu

@compnerd compnerd marked this pull request as draft February 22, 2023 01:34
@compnerd

Copy link
Copy Markdown
Member Author

This exposes an interesting issue that I was afraid of. There are symbols that may be part of a public interface that cannot be fully internalised.

@compnerd

Copy link
Copy Markdown
Member Author

@swift-ci please build toolchain Windows platform

@compnerd

Copy link
Copy Markdown
Member Author

Please test with following PRs:
apple/swift-argument-parser#579

@swift-ci please build toolchain Windows platform

@compnerd

Copy link
Copy Markdown
Member Author

Please test with following PRs:
swiftlang/swift-package-manager#6815
apple/swift-argument-parser#579

@swift-ci please build toolchain Windows platform

@compnerd compnerd marked this pull request as ready for review August 15, 2023 16:13
@compnerd

Copy link
Copy Markdown
Member Author

Please test with following PRs:
swiftlang/swift-package-manager#6815
apple/swift-argument-parser#579

@swift-ci please test

@compnerd

Copy link
Copy Markdown
Member Author

@swift-ci please test Linux platform

@compnerd

Copy link
Copy Markdown
Member Author

@swift-ci please test

@artemcm artemcm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems reasonable to me.

@etcwilde etcwilde left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems reasonable

@compnerd

Copy link
Copy Markdown
Member Author

@swift-ci please test Windows platform

@compnerd

Copy link
Copy Markdown
Member Author

@swift-ci please test Linux platform

1 similar comment
@compnerd

Copy link
Copy Markdown
Member Author

@swift-ci please test Linux platform

@compnerd

compnerd commented Dec 7, 2023

Copy link
Copy Markdown
Member Author

@swift-ci please test

@compnerd

Copy link
Copy Markdown
Member Author

@swift-ci please test Linux platform

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.

3 participants