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

[5.8] Re-enable test-static-stdlib.test #118

Merged
merged 1 commit into from
Jun 2, 2023

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented May 31, 2023

Originally submitted to 5.9 as #115

This test was disabled for a while, which led to static linking regressions on Linux. It should be re-enabled for us to make sure that this feature works as intended.

The issue itself is fixed in swiftlang/swift#66246.

Originally submitted to 5.9 as #115

This test was disabled for a while, which led to static linking regressions on Linux. It should be re-enabled for us to make sure that this feature works as intended.
@MaxDesiatov MaxDesiatov requested a review from shahmishal May 31, 2023 14:28
MaxDesiatov added a commit to swiftlang/swift that referenced this pull request Jun 1, 2023
[5.8] CMake: fix missing `SWIFT_CONCURRENCY_GLOBAL_EXECUTOR`

Explanation: Resolves issues with static linking on Linux
Risk: Medium, affects Linux builds and top-level CMake declarations.
Original PRs: #65795 and #64312 for `main`, #65824 and #64633 for `release/5.9`
Reviewed by: @al45tair @drexin @etcwilde 
Resolves: some of the issues reported in #65097, also resolves #58380
Tests: Added in swiftlang/swift-integration-tests#118

`SWIFT_CONCURRENCY_GLOBAL_EXECUTOR` is defined in `stdlib/cmake/modules/StdlibOptions.cmake`, which is not included during the first pass of evaluation of the root `CMakeLists.txt`. It is available on subsequent evaluations after the value is stored in CMake cache. This led to subtle bugs, where `usr/lib/swift_static/linux/static-stdlib-args.lnk` didn't contain certain flags on clean toolchain builds, but did contain them in incremental builds.

Not having these autolinking flags in toolchain builds leads to errors when statically linking executables on Linux.

Additionally, since our trivial lit tests previously didn't link Dispatch statically, they didn't expose a bug where `%import-static-libdispatch` substitution had a missing value. To fix that I had to update `lit.cfg` and clean up some of the related path computations to infer a correct substitution value.
@MaxDesiatov MaxDesiatov merged commit b7af8c0 into release/5.8 Jun 2, 2023
@MaxDesiatov MaxDesiatov deleted the maxd/5.8-reenable-static-linking branch June 2, 2023 02:50
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.

1 participant