From 542ab19a21535e489ab0b69cbdcd7e1e44f3febc Mon Sep 17 00:00:00 2001
From: Samuel Nicholas <nicholas.samuel@gmail.com>
Date: Tue, 26 Nov 2024 19:44:37 +1030
Subject: [PATCH] CMake: Handle GODOT_DEV_BUILD flag correctly

.dev is added to output artifacts
Warn users for specifying dev_build and Release build config
Update documentation with deviations to SCons
Update debug_symbols handling, its rolled into build config
Cleanup helper variables and comments
---
 CMakeLists.txt                    | 52 ++-------------------
 cmake/common_compiler_flags.cmake | 28 ++++--------
 cmake/godotcpp.cmake              | 76 +++++++++++++++++++++----------
 cmake/windows.cmake               |  4 --
 doc/cmake.rst                     | 14 ++++++
 test/CMakeLists.txt               |  5 +-
 6 files changed, 85 insertions(+), 94 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 926946d98..fb59a9990 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -12,8 +12,12 @@ CMAKE_PROJECT_<PROJECT-NAME>_INCLUDE which was introduced in version 3.17
 Scons Compatibility
 -------------------
 
+There is an understandable conflict between build systems as they define
+similar concepts in different ways. When there isn't a 1:1 relationship,
+compromises need to be made to resolve those differences.
+
 As we are attempting to maintain feature parity, and ease of maintenance, these
-CMake scripts are built to resemble the SCons build system.
+CMake scripts are built to resemble the SCons build system wherever possible.
 
 The file structure and file content are made to match, if not in content then
 in spirit. The closer the two build systems look the easier they will be to
@@ -30,58 +34,12 @@ function is run.
     cpp_tool = Tool("godotcpp", toolpath=["tools"])
     cpp_tool.options(opts, env)
 
-
 The CMake equivalent is below.
 ]=======================================================================]
 
 include( cmake/godotcpp.cmake )
 godotcpp_options()
 
-#[=======================================================================[.rst:
-
-Configurations
---------------
-
-There are two build main configurations, 'Debug' and 'Release', these are not
-related to godot's DEBUG_FEATURES flag. Build configurations change the default
-compiler and linker flags present when building the library, things like debug
-symbols, optimization.
-
-The Scons build scripts don't have this concept, you can think of it like the
-SCons solution has a single default configuration. In both cases overriding the
-defaults is controlled by options on the command line, or in preset files.
-
-Because of this added configuration and that it can be undefined, it becomes
-important to set a default, considering the SCons solution that does not enable
-debug symbols by default, it seemed appropriate to set the default to 'Release'
-if unspecified. This can always be overridden like below.
-
-.. highlight:: shell
-
-    cmake <source> -DCMAKE_BUILD_TYPE:STRING=Debug
-
-.. caution::
-
-A complication arises from `Multi-Config Generators`_ that cannot have
-their configuration set at configure time. This means that the configuration
-must be set on the build command. This is especially important for Visual
-Studio Generators which default to 'Debug'
-
-.. highlight:: shell
-
-    cmake --build . --config Release
-
-.. _Multi-Config Generators:https://cmake.org/cmake/help/latest/prop_gbl/GENERATOR_IS_MULTI_CONFIG.html
-]=======================================================================]
-get_property( IS_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG )
-if( NOT IS_MULTI_CONFIG AND NOT CMAKE_BUILD_TYPE )
-    if( GODOT_DEV_BUILD )
-        set( CMAKE_BUILD_TYPE Debug )
-    else ()
-        set( CMAKE_BUILD_TYPE Release )
-    endif ()
-endif ()
-
 #[[ Python is required for code generation ]]
 find_package(Python3 3.4 REQUIRED) # pathlib should be present
 
diff --git a/cmake/common_compiler_flags.cmake b/cmake/common_compiler_flags.cmake
index 1b185fe2c..4d1bdb95e 100644
--- a/cmake/common_compiler_flags.cmake
+++ b/cmake/common_compiler_flags.cmake
@@ -7,7 +7,11 @@ flags like optimization levels, warnings, and features. For platform specific
 flags look to each of the ``cmake/<platform>.cmake`` files.
 
 ]=======================================================================]
-#Generator Expression Helpers
+
+#[[ Compiler Configuration, not to be confused with build targets ]]
+set( DEBUG_SYMBOLS "$<OR:$<CONFIG:Debug>,$<CONFIG:RelWithDebInfo>>" )
+
+#[[ Compiler Identification ]]
 set( IS_CLANG "$<CXX_COMPILER_ID:Clang>" )
 set( IS_APPLECLANG "$<CXX_COMPILER_ID:AppleClang>" )
 set( IS_GNU "$<CXX_COMPILER_ID:GNU>" )
@@ -20,16 +24,7 @@ set( GNU_GT_V11 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,11>" )
 set( GNU_LT_V11 "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,11>" )
 set( GNU_GE_V12 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,12>" )
 
-set( HOT_RELOAD-UNSET "$<STREQUAL:${GODOT_USE_HOT_RELOAD},>")
-
-set( DISABLE_EXCEPTIONS "$<BOOL:${GODOT_DISABLE_EXCEPTIONS}>")
-
-
 function( common_compiler_flags TARGET_NAME )
-    set( IS_RELEASE "$<STREQUAL:${TARGET_NAME},template_release>")
-    set( DEBUG_FEATURES "$<OR:$<STREQUAL:${TARGET_NAME},template_debug>,$<STREQUAL:${TARGET_NAME},editor>>" )
-    set( HOT_RELOAD "$<IF:${HOT_RELOAD-UNSET},$<NOT:${IS_RELEASE}>,$<BOOL:${GODOT_USE_HOT_RELOAD}>>" )
-    set( DEBUG_SYMBOLS "$<BOOL:${GODOT_DEBUG_SYMBOLS}>" )
 
     target_compile_features(${TARGET_NAME}
             PUBLIC
@@ -44,24 +39,19 @@ function( common_compiler_flags TARGET_NAME )
             $<${DISABLE_EXCEPTIONS}:
                 $<${NOT_MSVC}:-fno-exceptions>
             >
-            $<$<NOT:${DISABLE_EXCEPTIONS}>:
-                $<${IS_MSVC}:/EHsc>
-            >
 
             # Enabling Debug Symbols
             $<${DEBUG_SYMBOLS}:
-                $<${IS_MSVC}: /Zi /FS>
-
                 # Adding dwarf-4 explicitly makes stacktraces work with clang builds,
                 # otherwise addr2line doesn't understand them.
                 $<${NOT_MSVC}:
                     -gdwarf-4
-                    $<IF:${IS_DEV},-g3,-g2>
+                    $<IF:${IS_DEV_BUILD},-g3,-g2>
                 >
             >
 
-            $<${IS_DEV}:
-                $<${NOT_MSVC}:-fno-omit-frame-pointer -O0 -g>
+            $<${IS_DEV_BUILD}:
+                $<${NOT_MSVC}:-fno-omit-frame-pointer -O0>
             >
 
             $<${HOT_RELOAD}:
@@ -137,6 +127,8 @@ function( common_compiler_flags TARGET_NAME )
             # features
             $<${DEBUG_FEATURES}:DEBUG_ENABLED DEBUG_METHODS_ENABLED>
 
+            $<${IS_DEV_BUILD}:DEV_ENABLED>
+
             $<${HOT_RELOAD}:HOT_RELOAD_ENABLED>
 
             $<$<STREQUAL:${GODOT_PRECISION},double>:REAL_T_IS_DOUBLE>
diff --git a/cmake/godotcpp.cmake b/cmake/godotcpp.cmake
index 13f1bfdb1..9bad5ae0d 100644
--- a/cmake/godotcpp.cmake
+++ b/cmake/godotcpp.cmake
@@ -2,6 +2,19 @@
 godotcpp.cmake
 --------------
 
+As godot-cpp is a C++ project, there are no C files, and detection of a C
+compiler is unnecessary. When CMake performs the configure process, if a
+C compiler is specified, like in a toolchain, or from an IDE, then it will
+print a warning stating that the CMAKE_C_COMPILER compiler is unused.
+This if statement simply silences that warning.
+]=======================================================================]
+if( CMAKE_C_COMPILER )
+endif ()
+
+#[=======================================================================[.rst:
+Include Platform Files
+----------------------
+
 Because these files are included into the top level CMakelists.txt before the
 project directive, it means that
 
@@ -18,10 +31,7 @@ include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/macos.cmake)
 include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/web.cmake)
 include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/windows.cmake)
 
-#Silence warning from unused CMAKE_C_COMPILER from toolchain
-if( CMAKE_C_COMPILER )
-endif ()
-
+# Detect number of processors
 include(ProcessorCount)
 ProcessorCount(PROC_MAX)
 message( "Auto-detected ${PROC_MAX} CPU cores available for build parallelism." )
@@ -99,8 +109,7 @@ function( godotcpp_options )
     #TODO threads
     #TODO compiledb
     #TODO compiledb_file
-
-    #NOTE: build_profile's equivalent in cmake is CMakePresets.json
+    #TODO build_profile
 
     set(GODOT_USE_HOT_RELOAD "" CACHE BOOL
             "Enable the extra accounting required to support hot reload. (ON|OFF)")
@@ -114,17 +123,26 @@ function( godotcpp_options )
     set_property( CACHE GODOT_SYMBOL_VISIBILITY PROPERTY STRINGS "auto;visible;hidden" )
 
     #TODO optimize
-    #TODO debug_symbols
-    option( GODOT_DEBUG_SYMBOLS "" OFF )
+
     option( GODOT_DEV_BUILD "Developer build with dev-only debugging code (DEV_ENABLED)" OFF )
 
+    #[[ debug_symbols
+    Debug symbols are enabled by using the Debug or RelWithDebInfo build configurations.
+    Single Config Generator is set at configure time
+
+        cmake ../ -DCMAKE_BUILD_TYPE=Debug
+
+    Multi-Config Generator is set at build time
+
+        cmake --build . --config Debug
+
+    ]]
+
     # FIXME These options are not present in SCons, and perhaps should be added there.
     option( GODOT_SYSTEM_HEADERS "Expose headers as SYSTEM." OFF )
     option( GODOT_WARNING_AS_ERROR "Treat warnings as errors" OFF )
 
-    # Run options commands on the following to populate cache for all
-    # platforms. This type of thing is typically done conditionally But as
-    # scons shows all options so shall we.
+    #[[ Target Platform Options ]]
     android_options()
     ios_options()
     linux_options()
@@ -136,7 +154,6 @@ endfunction()
 # Function to configure and generate the targets
 function( godotcpp_generate )
     #[[ Multi-Threaded MSVC Compilation
-
     When using the MSVC compiler the build command -j <n> only specifies
     parallel jobs or targets, and not multi-threaded compilation To speed up
     compile times on msvc, the /MP <n> flag can be set. But we need to set it
@@ -157,13 +174,11 @@ function( godotcpp_generate )
 
     #[[ GODOT_SYMBOL_VISIBLITY
     To match the SCons options, the allowed values are "auto", "visible", and "hidden"
-    This effects the compiler flag -fvisibility=[default|internal|hidden|protected]
-    The corresponding CMake option CXX_VISIBILITY_PRESET accepts the compiler values.
+    This effects the compiler flag_ -fvisibility=[default|internal|hidden|protected]
+    The corresponding target option CXX_VISIBILITY_PRESET accepts the compiler values.
 
     TODO: It is probably worth a pull request which changes both to use the compiler values
-    https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#index-fvisibility
-
-    This performs the necessary conversion
+    .. _flag:https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html#index-fvisibility
     ]]
     if( ${GODOT_SYMBOL_VISIBILITY} STREQUAL "auto" OR ${GODOT_SYMBOL_VISIBILITY} STREQUAL "visible" )
         set( GODOT_SYMBOL_VISIBILITY "default" )
@@ -230,14 +245,29 @@ function( godotcpp_generate )
         godot_arch_map( SYSTEM_ARCH ${CMAKE_SYSTEM_PROCESSOR} )
     endif()
 
+    # Transform options into generator expressions
+    set( HOT_RELOAD-UNSET "$<STREQUAL:${GODOT_USE_HOT_RELOAD},>")
+
+    set( DISABLE_EXCEPTIONS "$<BOOL:${GODOT_DISABLE_EXCEPTIONS}>")
+
+    # GODOT_DEV_BUILD
+    set( RELEASE_TYPES "Release;MinSizeRel")
+    get_property( IS_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG )
+    if( IS_MULTI_CONFIG )
+        message( NOTICE "=> Default build type is Debug. For other build types add --config <type> to build command")
+    elseif( GODOT_DEV_BUILD AND CMAKE_BUILD_TYPE IN_LIST RELEASE_TYPES )
+        message( WARNING "=> GODOT_DEV_BUILD implies a Debug-like build but CMAKE_BUILD_TYPE is '${CMAKE_BUILD_TYPE}'")
+    endif ()
+    set( IS_DEV_BUILD "$<BOOL:${GODOT_DEV_BUILD}>")
+    # The .dev portion of the name if GODOT_DEV_BUILD is true.
+    set( DEV_TAG "$<${IS_DEV_BUILD}:.dev>" )
+
     ### Define our godot-cpp library targets
     foreach ( TARGET_NAME template_debug template_release editor )
 
-        # Useful genex snippits used in subsequent genex's
-        set( IS_RELEASE "$<STREQUAL:${TARGET_NAME},template_release>")
-        set( IS_DEV "$<BOOL:${GODOT_DEV_BUILD}>")
-        set( DEBUG_FEATURES "$<OR:$<STREQUAL:${TARGET_NAME},template_debug>,$<STREQUAL:${TARGET_NAME},editor>>" )
-        set( HOT_RELOAD "$<IF:${HOT_RELOAD-UNSET},$<NOT:${IS_RELEASE}>,$<BOOL:${GODOT_USE_HOT_RELOAD}>>" )
+        # Generator Expressions that rely on the target
+        set( DEBUG_FEATURES "$<NOT:$<STREQUAL:${TARGET_NAME},template_release>>" )
+        set( HOT_RELOAD "$<IF:${HOT_RELOAD-UNSET},${DEBUG_FEATURES},$<BOOL:${GODOT_USE_HOT_RELOAD}>>" )
 
         # the godot-cpp.* library targets
         add_library( ${TARGET_NAME} STATIC EXCLUDE_FROM_ALL )
@@ -268,7 +298,7 @@ function( godotcpp_generate )
                 BUILD_RPATH_USE_ORIGIN ON
 
                 PREFIX lib
-                OUTPUT_NAME "${PROJECT_NAME}.${SYSTEM_NAME}.${TARGET_NAME}.${SYSTEM_ARCH}"
+                OUTPUT_NAME "${PROJECT_NAME}.${SYSTEM_NAME}.${TARGET_NAME}${DEV_TAG}.${SYSTEM_ARCH}"
                 ARCHIVE_OUTPUT_DIRECTORY "$<1:${CMAKE_BINARY_DIR}/bin>"
 
                 # Things that are handy to know for dependent targets
diff --git a/cmake/windows.cmake b/cmake/windows.cmake
index c209bdee8..fd07bd188 100644
--- a/cmake/windows.cmake
+++ b/cmake/windows.cmake
@@ -59,11 +59,7 @@ endfunction()
 
 #[===========================[ Target Generation ]===========================]
 function( windows_generate TARGET_NAME )
-    set( IS_MSVC "$<CXX_COMPILER_ID:MSVC>" )
-    set( IS_CLANG "$<OR:$<CXX_COMPILER_ID:AppleClang>,$<CXX_COMPILER_ID:Clang>>" )
-    set( NOT_MSVC "$<NOT:${IS_MSVC}>" )
     set( STATIC_CPP "$<BOOL:${GODOT_USE_STATIC_CPP}>")
-    set( DISABLE_EXCEPTIONS "$<BOOL:${GODOT_DISABLE_EXCEPTIONS}>")
     set( DEBUG_CRT "$<BOOL:${GODOT_DEBUG_CRT}>" )
 
     set_target_properties( ${TARGET_NAME}
diff --git a/doc/cmake.rst b/doc/cmake.rst
index d47dadd87..f604e78a6 100644
--- a/doc/cmake.rst
+++ b/doc/cmake.rst
@@ -24,6 +24,20 @@ Configuration examples are listed at the bottom of the page.
 
 .. _godot-cpp-template: https://github.com/godotengine/godot-cpp-template
 
+SCons Deviations
+----------------
+
+Not everything from SCons can be perfectly representable in CMake, here are
+the notable differences.
+
+- debug_symbols
+    No longer has an explicit option, and is enabled via Debug-like CMake
+    build configurations; Debug, RelWithDebInfo.
+
+- dev_build
+    Does not define NDEBUG when disabled, NDEBUG is set via Release-like
+    CMake build configurations; Release, MinSizeRel.
+
 Basic walkthrough
 -----------------
 
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index ab7397fa2..12e44f163 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -26,6 +26,7 @@ get_target_property( GODOT_TARGET godot-cpp::${TEST_TARGET} GODOT_TARGET )
 get_target_property( GODOT_ARCH godot-cpp::${TEST_TARGET} GODOT_ARCH )
 
 set( OUTPUT_DIR "${CMAKE_CURRENT_SOURCE_DIR}/project/bin/" )
+set( DEV_TAG "$<$<BOOL:${GODOT_DEV_BUILD}>:.dev>" )
 
 set_target_properties( godot-cpp-test
         PROPERTIES
@@ -45,7 +46,7 @@ set_target_properties( godot-cpp-test
         PDB_OUTPUT_DIRECTORY     "$<1:${OUTPUT_DIR}>" #MSVC Only, ignored on other platforms
 
         PREFIX "lib"
-        OUTPUT_NAME "gdexample.${GODOT_PLATFORM}.${GODOT_TARGET}.${GODOT_ARCH}"
+        OUTPUT_NAME "gdexample.${GODOT_PLATFORM}.${GODOT_TARGET}${DEV_TAG}.${GODOT_ARCH}"
 )
 
 if( CMAKE_SYSTEM_NAME STREQUAL Darwin )
@@ -58,7 +59,7 @@ if( CMAKE_SYSTEM_NAME STREQUAL Darwin )
             LIBRARY_OUTPUT_DIRECTORY "$<1:${OUTPUT_DIR}>"
             RUNTIME_OUTPUT_DIRECTORY "$<1:${OUTPUT_DIR}>"
 
-            OUTPUT_NAME "gdexample.macos.${TEST_TARGET}"
+            OUTPUT_NAME "gdexample.macos.${TEST_TARGET}${DEV_TAG}"
             SUFFIX ""
 
             #macos options
