[5.9] build: fix accidental cmake expansions #67624
Conversation
|
@MaxDesiatov, would you run the CI on this? |
|
@egorzhdan, please run the CI on this. |
|
@swift-ci please test |
|
Sigh, linux CI crashed when checking out the source. |
|
@swift-ci please test Linux |
|
Windows CI failed when building libdispatch after the Swift compiler is installed, appears unrelated to this pull. |
|
@swift-ci test windows |
|
@DougGregor, ready for final approval and merge. |
|
@compnerd, does anybody else need to sign off on this, or can this go in now? Please merge if the latter. |
|
Looks like this missed the |
|
Pinging @DougGregor as release manager again. |
|
@stephank, looks like this needs a rebase. |
As of CMake 3.25, there are now global variables `LINUX=1`, `ANDROID=1`,
etc. These conflict with expressions that used these names as unquoted
strings in positions where CMake accepts 'variable|string', for example:
- `if(sdk STREQUAL LINUX)` would fail, because `LINUX` is now defined and
expands to 1, where it would previously coerce to a string.
- `if(${sdk} STREQUAL "LINUX")` would fail if `sdk=LINUX`, because the
left-hand side expands twice.
In this patch, I looked for a number of patterns to fix up, sometimes a
little defensively:
- Quoted right-hand side of `STREQUAL` where I was confident it was
intended to be a string literal.
- Removed manual variable expansion on left-hand side of `STREQUAL`,
`MATCHES` and `IN_LIST` where I was confident it was unintended.
Fixes swiftlang#65028.
stephank
force-pushed
the
fix/5.9-cmake-3.25
branch
from
e4b7797 to
1197648
Compare
|
@bnbarham, need another CI run here. |
|
@swift-ci please test |
|
Pinging @DougGregor as 5.9 release manager for this repo, wdyt? |
|
Yea, this is great. Thank you! |
|
Hi, can we get another cherry-pick for release/5.9.1? I'm not familiar with the release process of the swift, but it seems that release/5.9 and release/5.9.1 are different branches. |
|
@bc-lee, anyone can propose getting this into 5.9.1, feel free to open a pull similar to this. I was surprised this wasn't already in, but it appears the 5.9 releases for this repo aren't actually being cut from the release/5.9 branch. |
Explanation: CMake 3.25 broke the build because it introduced new global variables
LINUX,ANDROID, etc., and 'string or variable' is ambiguous in certain syntax. Fixing these in 5.9 should help packaging on platforms that are already on newer CMake versions, e.g. NixOS 23.05, Ubuntu 23.04, Fedora 39 (rawhide).Issue: [CMake 3.25+]: Swift 5.8: Fails to compile bootstrap1 modules #65028
Risk: Few merge conflicts in the backport, but otherwise this is hard for me to assess as a first-time contributer and CMake novice.
Testing: I believe CI already covers the important platforms / codepaths here?
Reviewer: Unsure, there's no specific owner for the build system it seems? Tagging previous reviewers: @finagolfin, @etcwilde, @compnerd
Main branch PR: build: fix accidental cmake expansions #65534