Skip to content

Commit

Permalink
CMake: Fix for #1699 msvc runtime selection issues
Browse files Browse the repository at this point in the history
Previously I eschewed the use of CMAKE_MSVC_RUNTIME_LIBRARY in favour of setting the flags using target_compile_options so that they would propagate to consumers.

However, it has been raised that a dependency( independent to godot-cpp ) that doesn't set any runtime flags, which relies purely on the CMAKE_MSVC_RUNTIME_LIBRARY variable will very likely not have the correct msvc runtime flags set.

Where MSVC documentation states "All modules passed to a given invocation of the linker must have been compiled with the same runtime library compiler option (/MD, /MT, /LD)."

It was also mentioned that messaging as a warning  that we are setting the option was not ideal.

So I have updated the cmake code to use CMAKE_MSVC_RUNTIME_LIBRARY  over target-compile_options. And set it as a CACHE STRING variable so that it can be overridden if desired. We still message consumers, but as a NOTICE.

While performing this work I noticed that the various options code being inside functions was unnecessary, and less flexible.
  • Loading branch information
enetheru committed Feb 5, 2025
1 parent ee2a895 commit 89869d7
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 121 deletions.
2 changes: 0 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ The CMake equivalent is below.

include( cmake/godotcpp.cmake )

godotcpp_options()

#[[ Python is required for code generation ]]
find_package(Python3 3.4 REQUIRED) # pathlib should be present

Expand Down
14 changes: 10 additions & 4 deletions cmake/android.cmake
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
#[=======================================================================[.rst:
Android
-------
This file contains functions for options and configuration for targeting the
Android platform
Because this file is included into the top level CMakelists.txt before the
project directive, it means that
* ``CMAKE_CURRENT_SOURCE_DIR`` is the location of godot-cpp's CMakeLists.txt
* ``CMAKE_SOURCE_DIR`` is the location where any prior ``project(...)``
directive was
Android Toolchain
-----------------
Configuration of the Android toolchain is done using toolchain files,
CMakePresets, or variables on the command line.
Expand All @@ -25,9 +33,7 @@ Android platforms.
There is further information and examples in the doc/cmake.rst file.
]=======================================================================]
function( android_options )
# Android Options
endfunction()


function( android_generate )
target_compile_definitions(${TARGET_NAME}
Expand Down
137 changes: 64 additions & 73 deletions cmake/godotcpp.cmake
Original file line number Diff line number Diff line change
@@ -1,35 +1,25 @@
#[=======================================================================[.rst:
godotcpp.cmake
--------------
Because this file is included into the top level CMakelists.txt before the
project directive, it means that
* ``CMAKE_CURRENT_SOURCE_DIR`` is the location of godot-cpp's CMakeLists.txt
* ``CMAKE_SOURCE_DIR`` is the location where any prior ``project(...)``
directive was
]=======================================================================]

#[[ Silence CMake warning for missing C Compiler
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.
]=======================================================================]
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
* ``CMAKE_CURRENT_SOURCE_DIR`` is the location of godot-cpp's CMakeLists.txt
* ``CMAKE_SOURCE_DIR`` is the location where any prior ``project(...)``
directive was
]=======================================================================]
# Include directives
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/common_compiler_flags.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/android.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/ios.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/linux.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/macos.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/web.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/windows.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/python_callouts.cmake)

# Detect number of processors
Expand Down Expand Up @@ -82,80 +72,81 @@ function( godot_arch_map ALIAS PROC )
endif ()
endfunction()

# Function to define all the options.
function( godotcpp_options )
#NOTE: platform is managed using toolchain files.

# Input from user for GDExtension interface header and the API JSON file
set(GODOT_GDEXTENSION_DIR "gdextension" CACHE PATH
"Path to a custom directory containing GDExtension interface header and API JSON file ( /path/to/gdextension_dir )" )
set(GODOT_CUSTOM_API_FILE "" CACHE FILEPATH
"Path to a custom GDExtension API JSON file (takes precedence over `GODOT_GDEXTENSION_DIR`) ( /path/to/custom_api_file )")
#[================================[ Options ]================================]

#TODO generate_bindings
#NOTE: platform is managed using toolchain files.

option(GODOT_GENERATE_TEMPLATE_GET_NODE
"Generate a template version of the Node class's get_node. (ON|OFF)" ON)
# Input from user for GDExtension interface header and the API JSON file
set(GODOT_GDEXTENSION_DIR "gdextension" CACHE PATH
"Path to a custom directory containing GDExtension interface header and API JSON file ( /path/to/gdextension_dir )" )
set(GODOT_CUSTOM_API_FILE "" CACHE FILEPATH
"Path to a custom GDExtension API JSON file (takes precedence over `GODOT_GDEXTENSION_DIR`) ( /path/to/custom_api_file )")

#TODO build_library
#TODO generate_bindings

set(GODOT_PRECISION "single" CACHE STRING
"Set the floating-point precision level (single|double)")
option(GODOT_GENERATE_TEMPLATE_GET_NODE
"Generate a template version of the Node class's get_node. (ON|OFF)" ON)

# The arch is typically set by the toolchain
# however for Apple multi-arch setting it here will override.
set( GODOT_ARCH "" CACHE STRING "Target CPU Architecture")
set_property( CACHE GODOT_ARCH PROPERTY STRINGS ${ARCH_LIST} )
#TODO build_library

#TODO threads
#TODO compiledb
#TODO compiledb_file
set(GODOT_PRECISION "single" CACHE STRING
"Set the floating-point precision level (single|double)")

set( GODOT_BUILD_PROFILE "" CACHE PATH
"Path to a file containing a feature build profile" )
# The arch is typically set by the toolchain
# however for Apple multi-arch setting it here will override.
set( GODOT_ARCH "" CACHE STRING "Target CPU Architecture")
set_property( CACHE GODOT_ARCH PROPERTY STRINGS ${ARCH_LIST} )

set(GODOT_USE_HOT_RELOAD "" CACHE BOOL
"Enable the extra accounting required to support hot reload. (ON|OFF)")
#TODO threads
#TODO compiledb
#TODO compiledb_file

# Disable exception handling. Godot doesn't use exceptions anywhere, and this
# saves around 20% of binary size and very significant build time (GH-80513).
option(GODOT_DISABLE_EXCEPTIONS "Force disabling exception handling code (ON|OFF)" ON )
set( GODOT_BUILD_PROFILE "" CACHE PATH
"Path to a file containing a feature build profile" )

set( GODOT_SYMBOL_VISIBILITY "hidden" CACHE STRING
"Symbols visibility on GNU platforms. Use 'auto' to apply the default value. (auto|visible|hidden)")
set_property( CACHE GODOT_SYMBOL_VISIBILITY PROPERTY STRINGS "auto;visible;hidden" )
set(GODOT_USE_HOT_RELOAD "" CACHE BOOL
"Enable the extra accounting required to support hot reload. (ON|OFF)")

#TODO optimize
# Disable exception handling. Godot doesn't use exceptions anywhere, and this
# saves around 20% of binary size and very significant build time (GH-80513).
option(GODOT_DISABLE_EXCEPTIONS "Force disabling exception handling code (ON|OFF)" ON )

option( GODOT_DEV_BUILD "Developer build with dev-only debugging code (DEV_ENABLED)" OFF )
set( GODOT_SYMBOL_VISIBILITY "hidden" CACHE STRING
"Symbols visibility on GNU platforms. Use 'auto' to apply the default value. (auto|visible|hidden)")
set_property( CACHE GODOT_SYMBOL_VISIBILITY PROPERTY STRINGS "auto;visible;hidden" )

#[[ debug_symbols
Debug symbols are enabled by using the Debug or RelWithDebInfo build configurations.
Single Config Generator is set at configure time
#TODO optimize

cmake ../ -DCMAKE_BUILD_TYPE=Debug
option( GODOT_DEV_BUILD "Developer build with dev-only debugging code (DEV_ENABLED)" OFF )

Multi-Config Generator is set at build time
#[[ debug_symbols
Debug symbols are enabled by using the Debug or RelWithDebInfo build configurations.
Single Config Generator is set at configure time
cmake --build . --config Debug
cmake ../ -DCMAKE_BUILD_TYPE=Debug
]]
Multi-Config Generator is set at build time
# 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 )
cmake --build . --config Debug
# Enable Testing
option( GODOT_ENABLE_TESTING "Enable the godot-cpp.test.<target> integration testing targets" OFF )
]]

# 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 )

# Enable Testing
option( GODOT_ENABLE_TESTING "Enable the godot-cpp.test.<target> integration testing targets" OFF )

#[[ Target Platforms]]
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/android.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/ios.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/linux.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/macos.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/web.cmake)
include( ${CMAKE_CURRENT_SOURCE_DIR}/cmake/windows.cmake)

#[[ Target Platform Options ]]
android_options()
ios_options()
linux_options()
macos_options()
web_options()
windows_options()
endfunction()

# Function to configure and generate the targets
function( godotcpp_generate )
Expand Down
12 changes: 8 additions & 4 deletions cmake/ios.cmake
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
#[=======================================================================[.rst:
Ios
---
This file contains functions for options and configuration for targeting the
Ios platform
Because this file is included into the top level CMakelists.txt before the
project directive, it means that
* ``CMAKE_CURRENT_SOURCE_DIR`` is the location of godot-cpp's CMakeLists.txt
* ``CMAKE_SOURCE_DIR`` is the location where any prior ``project(...)``
directive was
]=======================================================================]
function(ios_options)
# iOS options
endfunction()


function(ios_generate)
target_compile_definitions(${TARGET_NAME}
Expand Down
12 changes: 8 additions & 4 deletions cmake/linux.cmake
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
#[=======================================================================[.rst:
Linux
-----
This file contains functions for options and configuration for targeting the
Linux platform
Because this file is included into the top level CMakelists.txt before the
project directive, it means that
* ``CMAKE_CURRENT_SOURCE_DIR`` is the location of godot-cpp's CMakeLists.txt
* ``CMAKE_SOURCE_DIR`` is the location where any prior ``project(...)``
directive was
]=======================================================================]
function( linux_options )
# Linux Options
endfunction()


function( linux_generate )
target_compile_definitions( ${TARGET_NAME}
Expand Down
13 changes: 7 additions & 6 deletions cmake/macos.cmake
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
#[=======================================================================[.rst:
MacOS
-----
This file contains functions for options and configuration for targeting the
MacOS platform
Because this file is included into the top level CMakelists.txt before the
project directive, it means that
* ``CMAKE_CURRENT_SOURCE_DIR`` is the location of godot-cpp's CMakeLists.txt
* ``CMAKE_SOURCE_DIR`` is the location where any prior ``project(...)``
directive was
]=======================================================================]

# Find Requirements
Expand All @@ -18,11 +24,6 @@ IF(APPLE)
ENDIF (APPLE)


function( macos_options )
# macos options here
endfunction()


function( macos_generate )

# OSX_ARCHITECTURES does not support generator expressions.
Expand Down
12 changes: 7 additions & 5 deletions cmake/web.cmake
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
#[=======================================================================[.rst:
Web
---
This file contains functions for options and configuration for targeting the
Web platform
Because this file is included into the top level CMakelists.txt before the
project directive, it means that
* ``CMAKE_CURRENT_SOURCE_DIR`` is the location of godot-cpp's CMakeLists.txt
* ``CMAKE_SOURCE_DIR`` is the location where any prior ``project(...)``
directive was
]=======================================================================]

# Emscripten requires this hack for use of the SHARED option
set( CMAKE_PROJECT_godot-cpp_INCLUDE cmake/emsdkHack.cmake )

function( web_options )
# web options
endfunction()


function( web_generate )
target_compile_definitions(${TARGET_NAME}
Expand Down
Loading

0 comments on commit 89869d7

Please # to comment.