From cbf5ab385db9e596fa4c904c95a9bf553039e9f6 Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Fri, 1 Jul 2022 16:09:11 +0100 Subject: [PATCH] CMake: Clean up option definitions (#4771) 1. Make the vcpkg hack minimal by only defining the options it needs before `include(Platforms)`. Every dependent option needed by vcpkg has to be recalculated after `include(Platforms)`. This brings the count of such options down to 1 (`PACKET_ENCRYPTION`). 2. `include(Platforms)` before the rest of the options. 3. Group related options together into sections and add comments. 4. Fix `--gc-sections` for C files. Also adds `-Wl,--as-needed`. Note: `PIE` can now be `ON` even if `BUILD_TESTING` is `OFF`. --- CMake/functions/genex.cmake | 21 +++- CMakeLists.txt | 190 +++++++++++++++++++----------------- 2 files changed, 120 insertions(+), 91 deletions(-) diff --git a/CMake/functions/genex.cmake b/CMake/functions/genex.cmake index 09ba1dd37..876af7f24 100644 --- a/CMake/functions/genex.cmake +++ b/CMake/functions/genex.cmake @@ -1,8 +1,25 @@ # Generator expression helpers +# If "NEW", `set(CACHE ...)` does not override non-cache variables +if(POLICY CMP0126) + cmake_policy(GET CMP0126 _cache_does_not_override_normal_vars_policy) +else() + set(_cache_does_not_override_normal_vars_policy "OLD") +endif() + macro(GENEX_OPTION name default description) - set(${name} ${default} CACHE STRING ${description}) - set_property(CACHE ${name} PROPERTY STRINGS FOR_DEBUG FOR_RELEASE ON OFF) + if(_cache_does_not_override_normal_vars_policy STREQUAL "NEW") + set(_define_cache_var TRUE) + elseif(DEFINED ${name}) + get_property(_define_cache_var CACHE ${name} PROPERTY TYPE) + endif() + + if(_define_cache_var) + set(${name} ${default} CACHE STRING ${description}) + set_property(CACHE ${name} PROPERTY STRINGS FOR_DEBUG FOR_RELEASE ON OFF) + else() + message("Skipping `set(CACHE ${name} ...)`: CMake is < 3.21 and a non-cache variable with the same name is already set (${name}=${${name}})") + endif() endmacro() # Provide an option that defaults to ON in debug builds. diff --git a/CMakeLists.txt b/CMakeLists.txt index 8380f6113..560917f78 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,6 +9,9 @@ endif() if(POLICY CMP0111) cmake_policy(SET CMP0111 NEW) endif() +if(POLICY CMP0126) + cmake_policy(SET CMP0126 NEW) +endif() # Projects added via `add_subdirectory` or `FetchContent` may have a lower # `cmake_minimum_required` than we set here. Set policies that we require @@ -32,69 +35,24 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMake") list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMake/finders") include(functions/genex) -DEBUG_OPTION(ASAN "Enable address sanitizer") -DEBUG_OPTION(UBSAN "Enable undefined behaviour sanitizer") -option(TSAN "Enable thread sanitizer (not compatible with ASAN=ON)" OFF) -DEBUG_OPTION(DEBUG "Enable debug mode in engine") -option(GPERF "Build with GPerfTools profiler" OFF) -cmake_dependent_option(GPERF_HEAP_FIRST_GAME_ITERATION "Save heap profile of the first game iteration" OFF "GPERF" OFF) -option(BUILD_TESTING "Build tests." ON) -option(DISABLE_LTO "Disable link-time optimization (by default enabled in release mode)" OFF) -cmake_dependent_option(PIE "Generate position-independent code" OFF "BUILD_TESTING" ON) -option(MACOSX_STANDALONE_APP_BUNDLE "Generate a portable app bundle to use on other devices (requires sudo)" OFF) +# Options required by `VcPkgManifestFeatures`, which must be included before the `project` call. option(USE_SDL1 "Use SDL1.2 instead of SDL2" OFF) option(NONET "Disable network support" OFF) -RELEASE_OPTION(DEVILUTIONX_STATIC_CXX_STDLIB "Link C++ standard library statically (if available)") -cmake_dependent_option(DISABLE_TCP "Disable TCP multiplayer option" OFF "NOT NONET" ON) -cmake_dependent_option(DISABLE_ZERO_TIER "Disable ZeroTier multiplayer option" OFF "NOT NONET" ON) cmake_dependent_option(PACKET_ENCRYPTION "Encrypt network packets" ON "NOT NONET" OFF) -option(NOSOUND "Disable sound support" OFF) -option(ENABLE_CODECOVERAGE "Instrument code for code coverage (only enabled with BUILD_TESTING)" OFF) -option(DISCORD_INTEGRATION "Build with Discord SDK for rich presence support" OFF) - -option(DISABLE_STREAMING_MUSIC "Disable streaming music (to work around broken platform implementations)" OFF) -mark_as_advanced(DISABLE_STREAMING_MUSIC) -option(DISABLE_STREAMING_SOUNDS "Disable streaming sounds (to work around broken platform implementations)" OFF) -mark_as_advanced(DISABLE_STREAMING_SOUNDS) -option(STREAM_ALL_AUDIO "Stream all the audio. For extremely RAM-constrained platforms.") -mark_as_advanced(STREAM_ALL_AUDIO) - -option(DEVILUTIONX_RESAMPLER_SPEEX "Build with Speex resampler" ON) -option(DEVILUTIONX_RESAMPLER_SDL "Build with SDL resampler" ON) - -option(DEVILUTIONX_PALETTE_TRANSPARENCY_BLACK_16_LUT "Whether to use a lookup table for transparency blending with black. This improves performance of blending transparent black overlays, such as quest dialog background, at the cost of 128 KiB of RAM." ON) - -cmake_dependent_option(DEVILUTIONX_DISABLE_RTTI "Disable RTTI" ON "NONET" OFF) -cmake_dependent_option(DEVILUTIONX_DISABLE_EXCEPTIONS "Disable exceptions" ON "NONET" OFF) - -if(TSAN) - set(ASAN OFF) -endif() - -# By default, devilutionx.mpq is built only if smpq is installed. -if(NOT DEFINED BUILD_ASSETS_MPQ AND NOT SRC_DIST) - find_program(SMPQ smpq) -elseif(BUILD_ASSETS_MPQ) - find_program(SMPQ smpq REQUIRED) -endif() -if(SMPQ) - set(_has_smpq ON) -else() - set(_has_smpq OFF) -endif() -option(BUILD_ASSETS_MPQ "If true, assets are packaged into devilutionx.mpq." ${_has_smpq}) - # The gettext[tools] package takes a very long time to install if(CMAKE_TOOLCHAIN_FILE MATCHES "vcpkg.cmake$") option(USE_GETTEXT_FROM_VCPKG "Add vcpkg dependency for gettext[tools] for compiling translations" OFF) endif() - -RELEASE_OPTION(CPACK "Configure CPack") +option(BUILD_TESTING "Build tests." ON) # These must be included after the options above but before the `project` call. include(VcPkgManifestFeatures) -# Set up the `project` early so that properties such as `TARGET_SUPPORTS_SHARED_LIBS` are defined. +# Set up the `project` before the rest of the options so that: +# +# 1. Properties such as `TARGET_SUPPORTS_SHARED_LIBS` are defined. +# 2. Toolchain file is evaluated, required for `Platforms.cmake`, +# which can override the options. if(NOT VERSION_NUM) include(functions/git) get_git_tag(VERSION_NUM) @@ -118,12 +76,9 @@ else() endif() set(PROJECT_VERSION_WITH_SUFFIX "${PROJECT_VERSION}$<$:-${VERSION_SUFFIX}>") -if(MSVC AND NOT CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT DISABLE_LTO) - # Work around MSVC + CMake bug when LTO is enabled. - # See https://github.com/diasurgical/devilutionX/issues/3778 - # and https://gitlab.kitware.com/cmake/cmake/-/issues/23035 - set(BUILD_TESTING OFF) -endif() +# Platform definitions can override options and we want `cmake_dependent_option` to see the effects. +# Note that a few options are still defined before this because they're needed by `VcPkgManifestFeatures.cmake`. +include(Platforms) # This built-in CMake module adds a BUILD_TESTING option (ON by default). # Must be included in the top-level `CMakeLists.txt` after calling `project`. @@ -131,51 +86,105 @@ endif() # we add a BUILD_TESTING option ourselves above as well. include(CTest) -# Platform definitions can override options and we want `cmake_dependent_option` to see the effects, -# so ideally we would include Platforms.cmake before definining the options. -# -# However, `Platforms` require `project` to have been called (to get access to toolchain defs), -# but `project` must be called after `VcPkgManifestFeatures`, and `VcPkgManifestFeatures` need -# to be after the options. -include(Platforms) +# Debugging / profiling options +DEBUG_OPTION(ASAN "Enable address sanitizer") +DEBUG_OPTION(UBSAN "Enable undefined behaviour sanitizer") +option(TSAN "Enable thread sanitizer (not compatible with ASAN=ON)" OFF) +DEBUG_OPTION(DEBUG "Enable debug mode in engine") +option(GPERF "Build with GPerfTools profiler" OFF) +cmake_dependent_option(GPERF_HEAP_FIRST_GAME_ITERATION "Save heap profile of the first game iteration" OFF "GPERF" OFF) +option(ENABLE_CODECOVERAGE "Instrument code for code coverage (only enabled with BUILD_TESTING)" OFF) + +# Packaging options +RELEASE_OPTION(CPACK "Configure CPack") +option(MACOSX_STANDALONE_APP_BUNDLE "Generate a portable app bundle to use on other devices (requires sudo)" OFF) + +# Network options +cmake_dependent_option(DISABLE_TCP "Disable TCP multiplayer option" OFF "NOT NONET" ON) +cmake_dependent_option(DISABLE_ZERO_TIER "Disable ZeroTier multiplayer option" OFF "NOT NONET" ON) + +# Sound options +option(NOSOUND "Disable sound support" OFF) +option(DEVILUTIONX_RESAMPLER_SPEEX "Build with Speex resampler" ON) +cmake_dependent_option(DEVILUTIONX_RESAMPLER_SDL "Build with SDL resampler" ON "NOT USE_SDL1" OFF) +if(DEVILUTIONX_RESAMPLER_SPEEX) + list(APPEND _resamplers Speex) +endif() +if(DEVILUTIONX_RESAMPLER_SDL) + list(APPEND _resamplers SDL) +endif() +list(GET _resamplers 0 _default_resampler) +set(DEVILUTIONX_DEFAULT_RESAMPLER ${_default_resampler} CACHE STRING "Default resampler") +set_property(CACHE DEVILUTIONX_DEFAULT_RESAMPLER PROPERTY STRINGS ${_resamplers}) + +# Optimization / link options +option(DISABLE_LTO "Disable link-time optimization (by default enabled in release mode)" OFF) +option(PIE "Generate position-independent code" OFF) +cmake_dependent_option(DEVILUTIONX_DISABLE_RTTI "Disable RTTI" ON "NONET" OFF) +cmake_dependent_option(DEVILUTIONX_DISABLE_EXCEPTIONS "Disable exceptions" ON "NONET" OFF) +RELEASE_OPTION(DEVILUTIONX_STATIC_CXX_STDLIB "Link C++ standard library statically (if available)") + +# Memory / performance trade-off options +option(DISABLE_STREAMING_MUSIC "Disable streaming music (to work around broken platform implementations)" OFF) +mark_as_advanced(DISABLE_STREAMING_MUSIC) +option(DISABLE_STREAMING_SOUNDS "Disable streaming sounds (to work around broken platform implementations)" OFF) +mark_as_advanced(DISABLE_STREAMING_SOUNDS) +option(STREAM_ALL_AUDIO "Stream all the audio. For extremely RAM-constrained platforms.") +mark_as_advanced(STREAM_ALL_AUDIO) +option(DEVILUTIONX_PALETTE_TRANSPARENCY_BLACK_16_LUT "Whether to use a lookup table for transparency blending with black. This improves performance of blending transparent black overlays, such as quest dialog background, at the cost of 128 KiB of RAM." ON) +mark_as_advanced(DEVILUTIONX_PALETTE_TRANSPARENCY_BLACK_16_LUT) + +# Additional features +option(DISCORD_INTEGRATION "Build with Discord SDK for rich presence support" OFF) + +# By default, devilutionx.mpq is built only if smpq is installed. +if(NOT DEFINED BUILD_ASSETS_MPQ AND NOT SRC_DIST) + find_program(SMPQ smpq) +elseif(BUILD_ASSETS_MPQ) + find_program(SMPQ smpq REQUIRED) +endif() +if(SMPQ) + set(_has_smpq ON) +else() + set(_has_smpq OFF) +endif() +option(BUILD_ASSETS_MPQ "If true, assets are packaged into devilutionx.mpq." ${_has_smpq}) + +# === Option overrides === +# TSAN is not compatible with ASAN. +if(TSAN) + set(ASAN OFF) +endif() + +if(MSVC AND NOT CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT DISABLE_LTO) + # Work around MSVC + CMake bug when LTO is enabled. + # See https://github.com/diasurgical/devilutionX/issues/3778 + # and https://gitlab.kitware.com/cmake/cmake/-/issues/23035 + set(BUILD_TESTING OFF) +endif() # Note: `CMAKE_CROSSCOMPILING` is only available after the `project` call. if(CMAKE_CROSSCOMPILING) set(BUILD_TESTING OFF) endif() -# Recalculate the dependent options after including the Platforms: if(BUILD_TESTING) - # For tests, we build a libdevilutionx.so shared library. - # When this libdevilutionx.so is linked against certain static libraries, - # they must be compiled with `-fPIC`. + # When tests are enabled, we build a shared devilutionx_so library, which needs to be PIC to link. set(PIE ON) endif() -if(PIE) - set(CMAKE_POSITION_INDEPENDENT_CODE TRUE) -endif() - +# Recalculate the dependent options that are defined before `include(Platforms)`: if(NONET) - set(DISABLE_TCP ON) - set(DISABLE_ZERO_TIER ON) - set(DISABLE_RTTI ON) - set(DISABLE_EXCEPTIONS ON) + # PACKET_ENCRYPTION is defined before `Platforms.cmake` is included. + # This means that if a `Platforms.cmake` sets NONET to OFF, PACKET_ENCRYPTION will not automatically + # reflect that. set(PACKET_ENCRYPTION OFF) endif() +# === End of option overrides === -if(USE_SDL1) - set(DEVILUTIONX_RESAMPLER_SDL OFF) -endif() -if(DEVILUTIONX_RESAMPLER_SPEEX) - list(APPEND _resamplers Speex) -endif() -if(DEVILUTIONX_RESAMPLER_SDL) - list(APPEND _resamplers SDL) +if(PIE) + set(CMAKE_POSITION_INDEPENDENT_CODE TRUE) endif() -list(GET _resamplers 0 _default_resampler) -set(DEVILUTIONX_DEFAULT_RESAMPLER ${_default_resampler} CACHE STRING "Default resampler") -set_property(CACHE DEVILUTIONX_DEFAULT_RESAMPLER PROPERTY STRINGS ${_resamplers}) find_program(CCACHE_PROGRAM ccache) if(CCACHE_PROGRAM) @@ -203,11 +212,14 @@ endif() if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") # For some reason, adding to CMAKE_CXX_FLAGS results in a slightly smaller # binary than using `add_compile/link_options` - set(_extra_flags "-ffunction-sections -fdata-sections -Wl,--gc-sections") + set(_extra_flags "-ffunction-sections -fdata-sections -Wl,--gc-sections,--as-needed") set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} ${_extra_flags}") + set(CMAKE_C_FLAGS_RELEASE "${CMAKE_C_FLAGS_RELEASE} ${_extra_flags}") set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO} ${_extra_flags}") + set(CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO} ${_extra_flags}") set(CMAKE_CXX_FLAGS_MINSIZEREL "${CMAKE_CXX_FLAGS_MINSIZEREL} ${_extra_flags}") + set(CMAKE_C_FLAGS_MINSIZEREL "${CMAKE_C_FLAGS_MINSIZEREL} ${_extra_flags}") endif() # Not a genexp because CMake doesn't support it