CMake: update empty CMAKE_BUILD_TYPE value to Release

[ChrisJohnsen]

The build scenario that led me to find #5865 was a non-optimized build via an empty CMAKE_BUILD_TYPE that arose from using a CMake configuration command like this:

cmake -B build-local -G Ninja -DCMAKE_INSTALL_PREFIX=… -DBUILD_DOCS=1

(This "empty" build type generates a build that is close to a Debug build, but (e.g.) lacks the -g compiler option that Debug includes.)

Note

CMAKE_BUILD_TYPE is only used for single-configuration generators (Ninja, Unix Makfiles, etc.).
CMAKE_CONFIGURATION_TYPES is used for multi-configuration generators (Ninja Multi-Configuration, Visual Studio, etc.).

I've since updated my personal "generate a CMake build dir" script to pass a CMAKE_BUILD_TYPE, so it shouldn't happen to me again by accident.

But should this kind of mistake be prevented automatically?

Intent?

It looks like CMakeLists.txt "wants" to update CMAKE_BUILD_TYPE to Release if it is "falsy" (CMake has many falsy values, including the empty string). The history shows that this logic (with slight variations) has been there from the beginning of the DFHack Git history, but I didn't get a feel for the intent from any of the commit messages.

Problem

This code runs into a problem in that set(… CACHE …) (without FORCE) does not update already-cached values and CMAKE_BUILD_TYPE "arrives" pre-cached (whether from the project call or from the command line).

History

The DFHack history shows that the CMAKE_BUILD_TYPE check/update logic has been used either before or after the project call at various points in history. Currently it is after project; this ensuring it always sees a pre-cached CMAKE_BUILD_TYPE and makes the update to the cached value ineffective.

DFHack CMAKE_BUILD_TYPE check/update history

The history shows that the help text has changed over time and the condition has varied (e.g., "not defined or empty" to "falsy"), but the core "set the cached value" has always been the same (never using FORCE).

• git blame origin/develop -L 35,40 CMakeLists.txt -> 76da2c2
• git blame 76da2c2aaf~ -L 9,14 CMakeLists.txt -> 1226919
• git blame 1226919b16~ -L 8,10 CMakeLists.txt -> f600928
• git blame f600928ec1~ -L 4,5 CMakeLists.txt -> e9a04df
• git blame e9a04dfa65~ -L 18,19 CMakeLists.txt -> f2b91d3
• git blame f2b91d3269~ -L 40,41 CMakeLists.txt -> 557d673
• git blame 557d6733e2~ -L 21,23 CMakeLists.txt -> fac8847

The line numbers give a rough idea of whether the code was before or after the project call at the time.

Solutions

The solution in this PR is to leave the check where it is (after project) and add FORCE to it. This will override any falsy (usually just empty), cached value whether it came from project or the command line. The "empty" build type becomes impossible to specify, even from the command line. This check/update logic would continue to work if it was moved back above project() in the future. This use of FORCE parallels the similar handling of CMAKE_CONFIGURATION_TYPES that is done in the same chunk of code.

Alternatively, the check/update logic could be moved before the project call. This would effectively preemptively override the cached value that project will offer, but will not override a value from the command line (-DCMAKE_INSTALL_PREFIX= would still create an "empty" build type). The associated handling of CMAKE_CONFIGURATION_TYPES would continue to work in this position.

Neither solution would prevent a (non-falsy) bogus/undesirable CMAKE_BUILD_TYPE value from being specified on the command line though.

Example CMakeLists.txt

I put this together to investigate CMake variables, caching, and how the DFHack check/update code works on them. It can be used (once put in some temporary directory) with a command like this:

cmake -B a-temp-build-dir -G Ninja -L --fresh

Extra arguments can be used to check the interaction with user-provided values:

• -DCMAKE_BUILD_TYPE=
• -DCMAKE_BUILD_TYPE=Release
• -DCMAKE_BUILD_TYPE=Debug

The code has the new FORCE variation from this PR, but can be modified to try other possibilities.

demo CMakeLists.txt

```cmake cmake_minimum_required(VERSION 3.18 FATAL_ERROR)

macro(show_status expr)
block(SCOPE_FOR VARIABLES)
set(indent " ")
list(APPEND CMAKE_MESSAGE_INDENT "${indent}")

    message("${expr} true conditionals:")

    list(APPEND CMAKE_MESSAGE_INDENT "${indent}")

    if(${expr})
        message("«${expr}»")
    endif()

    if(NOT ${expr})
        message("«NOT ${expr}»")
    endif()

    if(DEFINED ${expr})
        message("«DEFINED ${expr}»")
    endif()

    if(NOT DEFINED ${expr})
        message("«NOT DEFINED ${expr}»")
    endif()

    if(DEFINED CACHE{${expr}})
        message("«DEFINED CACHE{${expr}}»")
    endif()

    if(NOT DEFINED CACHE{${expr}})
        message("«NOT DEFINED CACHE{${expr}}»")
    endif()

    if(DEFINED ENV{${expr}})
        message("«DEFINED ENV{${expr}}»")
    endif()

    if(${expr} STREQUAL "")
        message("«${expr} STREQUAL \"\"»")
    endif()

    list(POP_BACK CMAKE_MESSAGE_INDENT)

    if(DEFINED ${expr})
        message("${expr} value:")
        list(APPEND CMAKE_MESSAGE_INDENT "${indent}")

        if(DEFINED CACHE{${expr}})
            block(SCOPE_FOR VARIABLES)
                set(value ${${expr}})
                set(cache-value $CACHE{${expr}})
                # get_property(cache-value CACHE ${expr} PROPERTY VALUE)
                get_property(cache-type CACHE ${expr} PROPERTY TYPE)
                string(LENGTH "${cache-type}" type-spacer)
                string(REPEAT " " ${type-spacer} type-spacer)

                if(NOT "${value}" STREQUAL "${cache-value}")
                    message("# distinct SET and CACHE values")
                    message(
                        "   SET "
                        "${expr} ${type-spacer} = «${value}»"
                    )
                    message(
                        "CACHEd "
                        "${expr}:${cache-type} = «${cache-value}»"
                    )
                else()
                    # directly set cache value (does not affect set value)
                    set_property(
                        CACHE ${expr} PROPERTY VALUE
                        "XXX${value}XXX"
                    )

                    if(${expr} STREQUAL "${value}")
                        message("# identical SET and CACHE values")
                        message(
                            "   SET "
                            "${expr} ${type-spacer} = «${value}»"
                        )
                        message(
                            "CACHEd "
                            "${expr}:${cache-type} = «${cache-value}»"
                        )
                    else()
                        message(
                            "CACHEd-only "
                            "${expr}:${cache-type} = «${cache-value}»"
                        )
                    endif()

                    # restore cached value (block doesn't restore it)
                    set_property(
                        CACHE ${expr} PROPERTY VALUE
                        "${cache-value}"
                    )
                endif()
            endblock()
        else()
            message(
                "SET-only "
                "${expr} = «${${expr}}»"
            )
        endif()

        list(POP_BACK CMAKE_MESSAGE_INDENT)
    endif()

    list(POP_BACK CMAKE_MESSAGE_INDENT)
endblock()

endmacro()

message("pre-project() CMAKE_CONFIGURATION_TYPES")
show_status(CMAKE_CONFIGURATION_TYPES)
message("pre-project() CMAKE_BUILD_TYPE")
show_status(CMAKE_BUILD_TYPE)

message("project()")
project(var-test)

#######

DFHack-like CMAKE_BUILD_TYPE demonstration

CMAKE_CONFIGURATION_TYPES and CMAKE_BUILD_TYPE have a special relationship

the first is for multi-config generators

the second for single-config generators

if(CMAKE_CONFIGURATION_TYPES)
message("initial CMAKE_CONFIGURATION_TYPES")
show_status(CMAKE_CONFIGURATION_TYPES)
else()
message("initial CMAKE_BUILD_TYPE")
show_status(CMAKE_BUILD_TYPE)
endif()

message("check/update CMAKE_BUILD_TYPE")
if(CMAKE_CONFIGURATION_TYPES)
set(CMAKE_CONFIGURATION_TYPES "Release;RelWithDebInfo"
CACHE STRING "List of supported configuration types" FORCE)
else()
set(DFHACK_TYPE_HELP
"Choose the type of build, options are: Release and RelWithDebInfo")
# Prevent cmake C module attempts to overwrite our help string
if(NOT CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE "Release" CACHE STRING "${DFHACK_TYPE_HELP}" FORCE)
else()
set(CMAKE_BUILD_TYPE "${CMAKE_BUILD_TYPE}"
CACHE STRING "${DFHACK_TYPE_HELP}")
endif()
set_property(CACHE CMAKE_BUILD_TYPE
PROPERTY STRINGS "Release;RelWithDebInfo")
endif()

if(CMAKE_CONFIGURATION_TYPES)
message("final CMAKE_CONFIGURATION_TYPES")
show_status(CMAKE_CONFIGURATION_TYPES)
else()
message("final CMAKE_BUILD_TYPE")
show_status(CMAKE_BUILD_TYPE)
endif()

#######

### show_status demonstrations

unset(test_unset)

set(test_empty "")

set(test_full "value")

show_status(test_unset)

show_status(test_empty)

show_status(test_full)

message("prior to set(CACHE) of test_cache_var")

show_status(test_cache_var)

set(test_cache_var "The value." CACHE STRING "A custom cache variable.")

message("subsequent to set(CACHE) of test_cache_var")

show_status(test_cache_var)

set(set-only "set only")

set(cached-only "cached-only cache value" CACHE STRING "help")

if(NOT CMAKE_VERSION VERSION_LESS "3.21")

block(SCOPE_FOR POLICIES)

# set then set-cache requires CMP0126 (or directly setting properties)

cmake_policy(SET CMP0126 NEW)

set(set-cache "set-cache set value")

set(set-cache "set-cache cache value" CACHE STRING "help")

endblock()

endif()

set(cache-set "cache-set cache value" CACHE STRING "help")

set(cache-set "cache-set set value")

set(cache-set-same "cache-set-same same value" CACHE STRING "help")

set(cache-set-same "cache-set-same same value")

set(cache-cache "cache-cache A value" CACHE STRING "help")

set(cache-cache "cache-cache B value" CACHE STRING "help")

show_status(set-only)

show_status(cached-only)

if(NOT CMAKE_VERSION VERSION_LESS "3.21")

show_status(set-cache)

endif()

show_status(cache-set)

show_status(cache-set-same)

show_status(cache-cache)

</details>