Skip to content

[cmake] Bump minimum cmake to version 3.24 and place -- after cmake -E env's options are passed.#90039

Open
gottesmm wants to merge 1 commit into
swiftlang:mainfrom
gottesmm:pr-728937accf75af175f3239e3a3c525d74d6f9a33
Open

[cmake] Bump minimum cmake to version 3.24 and place -- after cmake -E env's options are passed.#90039
gottesmm wants to merge 1 commit into
swiftlang:mainfrom
gottesmm:pr-728937accf75af175f3239e3a3c525d74d6f9a33

Conversation

@gottesmm

Copy link
Copy Markdown
Contributor

NOTE: This is just for @etcwilde when he gets around to it.


cmake -E env is CMake's helper for running a child command with extra environment variables. Its argument grammar is:

cmake -E env [options...] [--] <command> [<args>...]

The options it accepts include NAME=VALUE assignments and --unset=NAME. Without an explicit -- separator, CMake has to decide where its own options end and the child command begins, and parses tokens greedily as options until it sees one that doesn't look like one. That heuristic breaks as soon as the "command" we want to run looks option-shaped, e.g. starts with -, contains =, or is a path/generator-expression that CMake can't unambiguously classify. Symptoms range from "the variable assignment gets dropped" to "the wrong binary gets executed" to outright parse errors, depending on CMake version.

The -- terminator removes the ambiguity: everything before -- is env's own options, everything after is the child command and its arguments. Support for -- in cmake -E env was added in CMake 3.24, which is why this commit does two things together:

  1. Bump cmake_minimum_required to 3.24 in CMakeLists.txt, benchmark/CMakeLists.txt, and Runtimes/Resync.cmake so the new syntax is guaranteed to exist.

  2. Insert -- before the child command in every cmake -E env call site: - gyb invocations (Runtimes/{Core,Overlay,Supplemental}/cmake/modules/gyb.cmake, cmake/modules/SwiftHandleGybSources.cmake, cmake/modules/AddSwift.cmake) - libdispatch's install step (cmake/modules/Libdispatch.cmake) - LTO archiver wrappers (cmake/modules/UnixCompileRules.cmake) - lit test driver (test/CMakeLists.txt)

The lit driver case is the most fragile pre-fix: ${SWIFT_LIT_ENVIRONMENT} is an arbitrary list of KEY=VAL pairs assembled at configure time, and the very next argument is a generator-expression resolving to the Python binary path. Without --, CMake had to guess where the env list ended; with --, the boundary is explicit.

…E env's options are passed.

`cmake -E env` is CMake's helper for running a child command with extra
environment variables. Its argument grammar is:

    cmake -E env [options...] [--] <command> [<args>...]

The options it accepts include `NAME=VALUE` assignments and
`--unset=NAME`. Without an explicit `--` separator, CMake has to decide
where its own options end and the child command begins, and parses
tokens greedily as options until it sees one that doesn't look like
one. That heuristic breaks as soon as the "command" we want to run
looks option-shaped, e.g. starts with `-`, contains `=`, or is a
path/generator-expression that CMake can't unambiguously classify.
Symptoms range from "the variable assignment gets dropped" to "the
wrong binary gets executed" to outright parse errors, depending on
CMake version.

The `--` terminator removes the ambiguity: everything before `--` is
`env`'s own options, everything after is the child command and its
arguments. Support for `--` in `cmake -E env` was added in CMake 3.24,
which is why this commit does two things together:

1. Bump `cmake_minimum_required` to 3.24 in `CMakeLists.txt`,
   `benchmark/CMakeLists.txt`, and `Runtimes/Resync.cmake` so the new
   syntax is guaranteed to exist.

2. Insert `--` before the child command in every `cmake -E env` call
   site:
     - gyb invocations
       (`Runtimes/{Core,Overlay,Supplemental}/cmake/modules/gyb.cmake`,
       `cmake/modules/SwiftHandleGybSources.cmake`,
       `cmake/modules/AddSwift.cmake`)
     - libdispatch's install step (`cmake/modules/Libdispatch.cmake`)
     - LTO archiver wrappers (`cmake/modules/UnixCompileRules.cmake`)
     - lit test driver (`test/CMakeLists.txt`)

The lit driver case is the most fragile pre-fix: `${SWIFT_LIT_ENVIRONMENT}`
is an arbitrary list of `KEY=VAL` pairs assembled at configure time,
and the very next argument is a generator-expression resolving to the
Python binary path. Without `--`, CMake had to guess where the env
list ended; with `--`, the boundary is explicit.
Comment thread CMakeLists.txt
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.19.6)
cmake_minimum_required(VERSION 3.24)

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.

Good to see this finally raised, maybe we could go even higher, as in #88609?

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.

2 participants