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] CMake: fix missing SWIFT_CONCURRENCY_GLOBAL_EXECUTOR #66246

Merged
merged 11 commits into from
Jun 1, 2023

Conversation

MaxDesiatov
Copy link
Contributor

@MaxDesiatov MaxDesiatov commented May 31, 2023

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 and others added 5 commits May 31, 2023 15:18
`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.
It is only used in the stdlib build, so really has no business being set in the root `CMakeLists.txt`.
@MaxDesiatov MaxDesiatov added concurrency Feature: umbrella label for concurrency language features cmake swift 5.8 labels May 31, 2023
@MaxDesiatov MaxDesiatov requested a review from a team as a code owner May 31, 2023 14:29
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

swiftlang/swift-integration-tests#118

@swift-ci build toolchain

@MaxDesiatov MaxDesiatov added 🍒 release cherry pick Flag: Release branch cherry picks bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. Linux Platform: Linux build-script Area → utils: The build script labels May 31, 2023
@MaxDesiatov MaxDesiatov marked this pull request as draft May 31, 2023 16:32
@MaxDesiatov
Copy link
Contributor Author

@swift-ci test

@MaxDesiatov
Copy link
Contributor Author

swiftlang/swift-integration-tests#118

@swift-ci build toolchain

@tomerd
Copy link
Contributor

tomerd commented May 31, 2023

cc @shahmishal

@MaxDesiatov MaxDesiatov marked this pull request as ready for review May 31, 2023 18:02
@MaxDesiatov MaxDesiatov merged commit 3f5029f into release/5.8 Jun 1, 2023
@MaxDesiatov MaxDesiatov deleted the maxd/5.8-cmake-dispatch-fix branch June 1, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. build-script Area → utils: The build script cmake concurrency Feature: umbrella label for concurrency language features Linux Platform: Linux 🍒 release cherry pick Flag: Release branch cherry picks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants