bootstrap: make cmake executable configurable with config.toml

[nodakai]

This is a spiritual successor to #71262

Fixes #71227

TODO

• Add a template section to config.toml.example
• Official CI runs the build with a weird setup; it is trivial to add if-else just to avoid crashing under the CI but its implications should be reviewed
• With this change, CMAKE env var to control cmake crate will no longer be a pass-through field; bootstrap will (almost) always overwrite it. Its implications should be reviewed

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[jyn514]

Why is this useful, compared to only searching for cmake3 -> cmake?

[Mark-Simulacrum]

Additionally, can you say what uses the CMAKE env var? Marking as waiting-on-author for now.

[nodakai]

Updated the PR description

@jyn514 "the same reason why config.toml supports specifying command names for python, nodejs, npm, and gdb"

To be more specific, I'm aware of

1. people who have CMake 2.x as cmake and a sufficiently new CMake 3.x as cmake3 and want to invoke the latter, and
2. people who have a very old CMake 3.x from Yum as cmake3 and the latest CMake 3.x from https://cmake.org/download/ as cmake and want to invoke the latter

@Mark-Simulacrum https://docs.rs/cmake/0.1.45/src/cmake/lib.rs.html#456

[bors]

☔ The latest upstream changes (presumably #87413) made this pull request unmergeable. Please resolve the merge conflicts.

[Mark-Simulacrum]

This looks like a good start. As you noted, we'll need to fix CI; unfortunately those logs have expired now so I'm not sure what the failure looked like exactly.

[Mark-Simulacrum]

(Once you're ready for a review, you can use @rustbot ready to switch the status such that I'll take a look).

[Dylan-DPC]

@nodakai any updates on this?

[nodakai]

@rustbot ready

[Mark-Simulacrum]

Most of this logic probably belongs in config.rs, not here. We should always have build.config.cmake as the source of truth for the cmake path, and support setting it only via config.toml. (If it is set in the environment and not equal to the config.toml value, it should be a panic! at config parse time).

Here, we should just have the if need_cmake { cmd_finder.must_have(&build.config.cmake); }, with that defaulting to just "cmake" if nothing is set in the config.toml.

[nodakai]

I have a couple of comments, let me start with minor ones (since major ones are going to take more time to write)

1. (If it is set in the environment and not equal to the config.toml value, it should be a panic! at config parse time)

   Last time, you wrote
   bootstrap: make cmake executable configurable with config.toml #85728 (comment)

   If it is set and different, I think we should probably prefer config.toml still but issue a warning (just eprintln! is fine).

   Warn or panic? Let me know of your decision.

2. We should always have build.config.cmake as the source of truth for the cmake path, and support setting it only via config.toml.

   Are you suggesting to set CMAKE env var unconditionally based on what's in config.toml (or its default value defined in config.rs > define_config!)? Is that your opinion? You don't mind breaking a hypothetical existing build env which relies on a custom CMAKE env var (which is passed through to cmake crate as of today)? Let's say someone has CMAKE=mycmake as of today, expecting it to be passed through to cmake crate, and your suggestion is that they should set cmake = "mycmake" instead, and if they don't, their build should fail with a panic saying sth like "cmake from config.toml wants to use cmake but you have a contradictory env var CMAKE=mycmake`"?

   Caveat: from what I see, cmake crate accepts a very wide variety of env vars,
   https://docs.rs/cmake/0.1.48/src/cmake/lib.rs.html#857-861
   like CMAKE_x86_64-unknown-linux-gnu, and I don't think it's realistic for bootstrap to cover all of them.

I'll find time to add more

[nodakai]

@Mark-Simulacrum

[Mark-Simulacrum]

Warn or panic? Let me know of your decision.

I think panic! is probably best.

[...] cmake env

Looking into our use of cmake a little more, it looks like we only ever actually invoke it through the cmake crate, so if users are supplying configuration to us via env variables, the cmake crate would already be picking that up appropriately. In sanity checking, we do use "cmake" without doing the same env-variable lookup that the cmake crate does. (Target-specific cmake binaries do not feel particularly likely to be actually necessary, as it's really a host tool).

I'm not too worried about env-variables overriding bootstrap's configuration in the cmake crate - that seems unlikely to be an issue in practice, given how bootstrap itself isn't using cmake directly (except for sanity checks). Given that CMAKE isn't a "standard" env variable (or at least I've seen no evidence of that), I'm not really worried about supporting setting it as an alternative to editing config.toml, either.

So I think the value of config.cmake probably comes down to:

• config.toml, if set
• cmake, if not

After we load that, we want to put it in our own environment (set_var("CMAKE")) so that the cmake crate picks it up, at least as things stand today. Just before doing that I think it makes sense to check that it's not set to a different value (mostly just as a sanity check, but it doesn't make sense for this to be in sanity.rs).

Pretty much all of that logic should go into config.rs.

[bors]

☔ The latest upstream changes (presumably #96659) made this pull request unmergeable. Please resolve the merge conflicts.

[JohnCSimon]

ping from triage:
@nodakai can you please address the merge conflicts?

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

[nodakai]

😵‍💫

[Dylan-DPC]

Closing this as inactive. Feel free to open a new PR when you have the time for it. Thanks for contributing