cmake: Do not modify build types when integrating by downstream project #1543

The CMAKE_BUILD_TYPE and CMAKE_CONFIGURATION_TYPES must be managed by the downstream project.

Suggesting to review with git diff -w.

Fixes std::out_of_range exception from CMake in https://github.com/hebasto/bitcoin/pull/192 when running configuration step using "Ninja Multi-Config" generator:

$ cmake -B build -G "Ninja Multi-Config"
...
-- Configuring done (17.1s)
terminate called after throwing an instance of 'std::out_of_range'
  what():  map::at
Aborted (core dumped)

Here are related discussions:

• https://discourse.cmake.org/t/uncaught-exception-when-trying-to-generate-a-project-using-ninja-multi-config/11051
• https://gitlab.kitware.com/cmake/cmake/-/issues/26064

Fixes std::out_of_range exception in https://github.com/hebasto/bitcoin/pull/192 when using "Ninja Multi-Config" generator.

Can you at least elaborate some amount? How is changing something CMake related in this C project, the solution to some C++ issue in a different project? Theres no details or CI failure etc in the linked PR, so it's not even clear what's broken there.

Yeah, I tend to agree that we shouldn't touch these variables. But it's a bit unclear how this fixes a std::out_of_range exception.

Yeah, I tend to agree that we shouldn't touch these variables.

Hm, on a second thought, I'm not sure. I believe we want to build libsecp256k1 with the default RelWithDebInfo (-O2 -g in our case) even when we build it as part of Core. Because (almost) all our testing uses this config.

edit: What does the current autotools build do?

By the way, this is one of these issues that makes me wonder if the GNU build system was actually that bad. The amount of "custom" logic that is needed to convince CMake to do the right thing is still large.

How is changing something CMake related in this C project, the solution to some C++ issue in a different project?

After offline discussion, it's been clarified that the std::out_of_range exception is actually coming from CMake itself, with it being uncertain as to whether the CMake developers consider it a bug or just "misuse". @hebasto if you have a link to upstream discussion, it'd be good to have it here as well.

Fixes std::out_of_range exception in hebasto/bitcoin#192 when using "Ninja Multi-Config" generator.

Can you at least elaborate some amount? How is changing something CMake related in this C project, the solution to some C++ issue in a different project? Theres no details or CI failure etc in the linked PR, so it's not even clear what's broken there.

I've updated the PR description trying to make it more descriptive and providing more details.

@hebasto if you have a link to upstream discussion, it'd be good to have it here as well.

Done.

I've updated the PR description trying to make it more descriptive and providing more details.

@hebasto if you have a link to upstream discussion, it'd be good to have it here as well.

Done.

There's also an GitLab issue now: https://gitlab.kitware.com/cmake/cmake/-/issues/26064. I took the freedom to add the link to the PR description.

The CMake's bug has been fixed in https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9618.

I tested CMake version 3.30.0-rc4 and no exceptions happen.

Nevertheless, this PR's changes are still meaningful and required.

ACK 158f9e5eae583b1520af9ee727db342e7408dab1

Tested that this change (combined with https://github.com/hebasto/bitcoin/pull/192) no-longers results in CMake segfaulting when configuring. I assume there's nothing else we need to do, even if CMake also changes its docs here: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9618#note_1538910 ?

The CMAKE_BUILD_TYPE and CMAKE_CONFIGURATION_TYPES must be managed by the downstream project.

All the CMake issues mention only CMAKE_CONFIGURATION_TYPES? Have you a reference for the claim that also CMAKE_BUILD_TYPE should be managed by the top-level project? (Well, it's probably also just a good idea for consistency?)

The CMAKE_BUILD_TYPE and CMAKE_CONFIGURATION_TYPES must be managed by the downstream project.

All the CMake issues mention only CMAKE_CONFIGURATION_TYPES? Have you a reference for the claim that also CMAKE_BUILD_TYPE should be managed by the top-level project? (Well, it's probably also just a good idea for consistency?)

The CMAKE_BUILD_TYPE being a cache variable is dedicated to be modifiable externally and it should be managed by the top-level project for consistency.

The same technique is suggested in the book Professional CMake: A Practical Guide, section 15.3. Custom Build Types.