[FIXUP] cmake: Fix flags handling

1. Use standard CMake's way to pass flags from depends to the main build system using CMAKE_<LANGUAGE>_FLAGS_INIT variables instead of custom DEPENDS_<LANGUAGE>_COMPILER_FLAGS ones. This guaranties using those flags during various checks at the configuration stage. 2. Setting flags is decoupled from setting compilers in the toolchain file. 3. Per-configuration flags are cached properly. 4. No longer needed to set -DCMAKE_BUILD_TYPE=None when building with depends. Fixes cross compilation for macOS. Allows the user to use {C,CXX}FLAGS environment variables for depends and the main build system without drawbacks/workarounds.
hebasto · Mar 23, 2024 · 7129dd7 · 7129dd7
1 parent e8bbd04
commit 7129dd7
Show file tree

Hide file tree

Showing 5 changed files with 166 additions and 64 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -114,6 +114,12 @@ add_library(core_interface INTERFACE)
 # include the warn_interface as subtree's warnings are not fixable
 # in our tree.
 add_library(core_base_interface INTERFACE)
+add_library(core_depends_release_interface INTERFACE)
+add_library(core_depends_debug_interface INTERFACE)
+target_link_libraries(core_base_interface INTERFACE
+  $<$<CONFIG:RelWithDebInfo>:${core_depends_release_interface}>
+  $<$<CONFIG:Debug>:${core_depends_debug_interface}>
+)
 target_link_libraries(core_interface INTERFACE core_base_interface)
 
 if(FUZZ)
@@ -223,12 +229,6 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
   try_append_linker_flag("-Wl,-headerpad_max_install_names" TARGET core_base_interface)
 endif()
 
-if(CMAKE_CROSSCOMPILING)
-  target_compile_definitions(core_base_interface INTERFACE ${DEPENDS_COMPILE_DEFINITIONS})
-  target_compile_options(core_base_interface INTERFACE "$<$<COMPILE_LANGUAGE:C>:${DEPENDS_C_COMPILER_FLAGS}>")
-  target_compile_options(core_base_interface INTERFACE "$<$<COMPILE_LANGUAGE:CXX>:${DEPENDS_CXX_COMPILER_FLAGS}>")
-endif()
-
 include(AddThreadsIfNeeded)
 add_threads_if_needed()
 
@@ -280,10 +280,10 @@ include(cmake/leveldb.cmake)
 include(cmake/minisketch.cmake)
 include(cmake/secp256k1.cmake)
 
-string(STRIP "${CMAKE_CXX_FLAGS}" cxx_flags)
-string(STRIP "${CMAKE_CXX_FLAGS_INIT}" cxx_flags_init)
-if(cxx_flags STREQUAL cxx_flags_init)
-  # CMAKE_CXX_FLAGS are not overridden.
+include(ProcessConfigurations)
+are_flags_overridden(CMAKE_CXX_FLAGS cxx_flags_overridden)
+# TODO: Rework after https://github.com/bitcoin/bitcoin/pull/25972.
+if(NOT cxx_flags_overridden AND NOT CMAKE_CROSSCOMPILING)
   add_library(warn_interface INTERFACE)
   target_link_libraries(core_interface INTERFACE warn_interface)
   if(MSVC)
@@ -331,19 +331,17 @@ if(cxx_flags STREQUAL cxx_flags_init)
     )
   endif()
 endif()
-unset(cxx_flags)
-unset(cxx_flags_init)
+unset(cxx_flags_overridden)
 
-include(ProcessConfigurations)
 set_default_config(RelWithDebInfo)
 
-# Redefine configuration flags.
-target_compile_definitions(core_base_interface INTERFACE
-  $<$<CONFIG:Debug>:DEBUG>
-  $<$<CONFIG:Debug>:DEBUG_LOCKORDER>
-  $<$<CONFIG:Debug>:DEBUG_LOCKCONTENTION>
-  $<$<CONFIG:Debug>:RPC_DOC_CHECK>
-  $<$<CONFIG:Debug>:ABORT_ON_FAILED_ASSUME>
+# Redefine/adjust per-configuration flags.
+target_compile_definitions(core_depends_debug_interface INTERFACE
+  DEBUG
+  DEBUG_LOCKORDER
+  DEBUG_LOCKCONTENTION
+  RPC_DOC_CHECK
+  ABORT_ON_FAILED_ASSUME
 )
 # We leave assertions on.
 if(MSVC)
@@ -353,20 +351,52 @@ else()
   remove_c_flag_from_all_configs(-DNDEBUG)
   remove_cxx_flag_from_all_configs(-DNDEBUG)
 
+  # Adjust flags used by the C/CXX compiler during RELEASE builds.
   # Prefer -O2 optimization level. (-O3 is CMake's default for Release for many compilers.)
   replace_c_flag_in_config(Release -O3 -O2)
   replace_cxx_flag_in_config(Release -O3 -O2)
 
-  set(debug_flags)
-  try_append_cxx_flags("-O0" VAR debug_flags)
-  try_append_cxx_flags("-g3" VAR debug_flags RESULT_VAR compiler_supports_g3)
-  if(NOT compiler_supports_g3)
-    try_append_cxx_flags("-g" VAR debug_flags)
+  are_flags_overridden(CMAKE_CXX_FLAGS_DEBUG cxx_flags_debug_overridden)
+  if(NOT cxx_flags_debug_overridden)
+    # Redefine flags used by the CXX compiler during DEBUG builds.
+    try_append_cxx_flags("-g3" RESULT_VAR compiler_supports_g3)
+    if(compiler_supports_g3)
+      replace_cxx_flag_in_config(Debug -g -g3)
+    endif()
+
+    try_append_cxx_flags("-ftrapv" RESULT_VAR compiler_supports_ftrapv)
+    if(compiler_supports_ftrapv)
+      string(PREPEND CMAKE_CXX_FLAGS_DEBUG "-ftrapv ")
+    endif()
+    unset(compiler_supports_ftrapv)
+
+    string(PREPEND CMAKE_CXX_FLAGS_DEBUG "-O0 ")
+
+    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}"
+      CACHE STRING
+      "Flags used by the CXX compiler during DEBUG builds."
+      FORCE
+    )
+  endif()
+  unset(cxx_flags_debug_overridden)
+
+  are_flags_overridden(CMAKE_C_FLAGS_DEBUG c_flags_debug_overridden)
+  if(NOT c_flags_debug_overridden)
+    # Redefine flags used by the C compiler during DEBUG builds.
+    if(compiler_supports_g3)
+      replace_c_flag_in_config(Debug -g -g3)
+    endif()
+
+    string(PREPEND CMAKE_C_FLAGS_DEBUG "-O0 ")
+
+    set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}"
+      CACHE STRING
+      "Flags used by the C compiler during DEBUG builds."
+      FORCE
+    )
   endif()
-  set(CMAKE_C_FLAGS_DEBUG "${debug_flags}")
-  try_append_cxx_flags("-ftrapv" VAR debug_flags)
-  set(CMAKE_CXX_FLAGS_DEBUG "${debug_flags}")
-  unset(debug_flags)
+  unset(compiler_supports_g3)
+  unset(c_flags_debug_overridden)
 endif()
 
 include(cmake/optional_qt.cmake)
@@ -462,9 +492,27 @@ setup_split_debug_script()
 add_maintenance_targets()
 add_windows_deploy_target()
 
+
+if(CMAKE_CROSSCOMPILING)
+  target_compile_definitions(core_base_interface INTERFACE ${DEPENDS_COMPILE_DEFINITIONS})
+  target_compile_definitions(core_depends_release_interface INTERFACE ${DEPENDS_COMPILE_DEFINITIONS_RELWITHDEBINFO})
+  target_compile_definitions(core_depends_debug_interface INTERFACE ${DEPENDS_COMPILE_DEFINITIONS_DEBUG})
+
+  # If {C,CXX}FLAGS variables are defined during building depends and
+  # configuring this build system, their content might be duplicated.
+  if(DEFINED ENV{CFLAGS})
+    deduplicate_flags(CMAKE_C_FLAGS)
+  endif()
+  if(DEFINED ENV{CXXFLAGS})
+    deduplicate_flags(CMAKE_CXX_FLAGS)
+  endif()
+endif()
+
+
 include(GetTargetInterface)
 get_target_interface(definitions core_interface COMPILE_DEFINITIONS)
-separate_by_configs(definitions)
+get_target_interface(definitions_RELWITHDEBINFO core_depends_release_interface COMPILE_DEFINITIONS)
+get_target_interface(definitions_DEBUG core_depends_debug_interface COMPILE_DEFINITIONS)
 
 message("\n")
 message("Configure summary")
@@ -500,15 +548,11 @@ else()
   set(cross_status "FALSE")
 endif()
 message("Cross compiling ....................... ${cross_status}")
-message("Preprocessor defined macros ........... ${definitions_ALL}")
+message("Preprocessor defined macros ........... ${definitions}")
 message("C compiler ............................ ${CMAKE_C_COMPILER}")
-list(JOIN DEPENDS_C_COMPILER_FLAGS " " depends_c_flags)
-string(STRIP "${CMAKE_C_FLAGS} ${depends_c_flags}" combined_c_flags)
-message("CFLAGS ................................ ${combined_c_flags}")
+message("CFLAGS ................................ ${CMAKE_C_FLAGS}")
 message("C++ compiler .......................... ${CMAKE_CXX_COMPILER}")
-list(JOIN DEPENDS_CXX_COMPILER_FLAGS " " depends_cxx_flags)
-string(STRIP "${CMAKE_CXX_FLAGS} ${depends_cxx_flags}" combined_cxx_flags)
-message("CXXFLAGS .............................. ${combined_cxx_flags}")
+message("CXXFLAGS .............................. ${CMAKE_CXX_FLAGS}")
 get_target_interface(common_compile_options core_interface COMPILE_OPTIONS)
 message("Common compile options ................ ${common_compile_options}")
 get_target_interface(common_link_options core_interface LINK_OPTIONS)

diff --git a/cmake/module/GetTargetInterface.cmake b/cmake/module/GetTargetInterface.cmake
@@ -8,9 +8,7 @@ include_guard(GLOBAL)
 function(get_target_interface var target property)
   get_target_property(result ${target} INTERFACE_${property})
   if(result)
-    if(NOT ${property} STREQUAL "COMPILE_DEFINITIONS")
-      string(GENEX_STRIP "${result}" result)
-    endif()
+    string(GENEX_STRIP "${result}" result)
     list(JOIN result " " result)
   else()
     set(result)

diff --git a/cmake/module/ProcessConfigurations.cmake b/cmake/module/ProcessConfigurations.cmake
@@ -2,6 +2,36 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or https://opensource.org/license/mit/.
 
+include_guard(GLOBAL)
+
+macro(normalize_string string)
+  string(REGEX REPLACE " +" " " ${string} "${${string}}")
+  string(STRIP "${${string}}" ${string})
+endmacro()
+
+function(are_flags_overridden flags_var result_var)
+  normalize_string(${flags_var})
+  normalize_string(${flags_var}_INIT)
+  if(${flags_var} STREQUAL ${flags_var}_INIT)
+    set(${result_var} FALSE PARENT_SCOPE)
+  else()
+    set(${result_var} TRUE PARENT_SCOPE)
+  endif()
+endfunction()
+
+
+# Removes duplicated flags. The relative order of flags is preserved.
+# If duplicates are encountered, only the last instance is preserved.
+function(deduplicate_flags flags)
+  separate_arguments(${flags})
+  list(REVERSE ${flags})
+  list(REMOVE_DUPLICATES ${flags})
+  list(REVERSE ${flags})
+  list(JOIN ${flags} " " result)
+  set(${flags} "${result}" PARENT_SCOPE)
+endfunction()
+
+
 function(get_all_configs output)
   get_property(is_multi_config GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
   if(is_multi_config)
@@ -63,6 +93,11 @@ function(remove_c_flag_from_all_configs flag)
     list(FILTER flags EXCLUDE REGEX "${flag}")
     list(JOIN flags " " new_flags)
     set(CMAKE_C_FLAGS_${config_uppercase} "${new_flags}" PARENT_SCOPE)
+    set(CMAKE_C_FLAGS_${config_uppercase} "${new_flags}"
+      CACHE STRING
+      "Flags used by the C compiler during ${config_uppercase} builds."
+      FORCE
+    )
   endforeach()
 endfunction()
 
@@ -75,36 +110,34 @@ function(remove_cxx_flag_from_all_configs flag)
     list(FILTER flags EXCLUDE REGEX "${flag}")
     list(JOIN flags " " new_flags)
     set(CMAKE_CXX_FLAGS_${config_uppercase} "${new_flags}" PARENT_SCOPE)
+    set(CMAKE_CXX_FLAGS_${config_uppercase} "${new_flags}"
+      CACHE STRING
+      "Flags used by the CXX compiler during ${config_uppercase} builds."
+      FORCE
+    )
   endforeach()
 endfunction()
 
 function(replace_c_flag_in_config config old_flag new_flag)
   string(TOUPPER "${config}" config_uppercase)
   string(REGEX REPLACE "(^| )${old_flag}( |$)" "\\1${new_flag}\\2" new_flags "${CMAKE_C_FLAGS_${config_uppercase}}")
   set(CMAKE_C_FLAGS_${config_uppercase} "${new_flags}" PARENT_SCOPE)
+  set(CMAKE_C_FLAGS_${config_uppercase} "${new_flags}"
+    CACHE STRING
+    "Flags used by the C compiler during ${config_uppercase} builds."
+    FORCE
+  )
 endfunction()
 
 function(replace_cxx_flag_in_config config old_flag new_flag)
   string(TOUPPER "${config}" config_uppercase)
   string(REGEX REPLACE "(^| )${old_flag}( |$)" "\\1${new_flag}\\2" new_flags "${CMAKE_CXX_FLAGS_${config_uppercase}}")
   set(CMAKE_CXX_FLAGS_${config_uppercase} "${new_flags}" PARENT_SCOPE)
-endfunction()
-
-function(separate_by_configs options)
-  list(JOIN ${options} " " ${options}_ALL)
-  string(GENEX_STRIP "${${options}_ALL}" ${options}_ALL)
-  string(STRIP "${${options}_ALL}" ${options}_ALL)
-  set(${options}_ALL "${${options}_ALL}" PARENT_SCOPE)
-
-  get_all_configs(all_configs)
-  foreach(config IN LISTS all_configs)
-    string(REGEX MATCHALL "\\$<\\$<CONFIG:${config}>:[^<>\n]*>" match "${${options}}")
-    list(JOIN match " " match)
-    string(REPLACE "\$<\$<CONFIG:${config}>:" "" match "${match}")
-    string(REPLACE ">" "" match "${match}")
-    string(TOUPPER "${config}" conf_upper)
-    set(${options}_${conf_upper} "${match}" PARENT_SCOPE)
-  endforeach()
+  set(CMAKE_CXX_FLAGS_${config_uppercase} "${new_flags}"
+    CACHE STRING
+    "Flags used by the CXX compiler during ${config_uppercase} builds."
+    FORCE
+  )
 endfunction()
 
 function(print_config_flags)

diff --git a/depends/Makefile b/depends/Makefile
@@ -273,9 +273,15 @@ $(host_prefix)/share/toolchain.cmake : toolchain.cmake.in $(host_prefix)/.stamp_
             -e 's|@INSTALL_NAME_TOOL@|$(host_INSTALL_NAME_TOOL)|' \
             -e 's|@OTOOL@|$(host_OTOOL)|' \
             -e 's|@depends_prefix@|$(host_prefix)|' \
-            -e 's|@CFLAGS@|$(strip $(host_CFLAGS) $(host_$(release_type)_CFLAGS))|' \
-            -e 's|@CXXFLAGS@|$(strip $(host_CXXFLAGS) $(host_$(release_type)_CXXFLAGS))|' \
-            -e 's|@CPPFLAGS@|$(strip $(host_CPPFLAGS) $(host_$(release_type)_CPPFLAGS))|' \
+            -e 's|@CFLAGS@|$(strip $(host_CFLAGS))|' \
+            -e 's|@CFLAGS_RELEASE@|$(strip $(host_release_CFLAGS))|' \
+            -e 's|@CFLAGS_DEBUG@|$(strip $(host_debug_CFLAGS))|' \
+            -e 's|@CXXFLAGS@|$(strip $(host_CXXFLAGS))|' \
+            -e 's|@CXXFLAGS_RELEASE@|$(strip $(host_release_CXXFLAGS))|' \
+            -e 's|@CXXFLAGS_DEBUG@|$(strip $(host_debug_CXXFLAGS))|' \
+            -e 's|@CPPFLAGS@|$(strip $(host_CPPFLAGS))|' \
+            -e 's|@CPPFLAGS_RELEASE@|$(strip $(host_release_CPPFLAGS))|' \
+            -e 's|@CPPFLAGS_DEBUG@|$(strip $(host_debug_CPPFLAGS))|' \
             -e 's|@no_qt@|$(NO_QT)|' \
             -e 's|@no_qr@|$(NO_QR)|' \
             -e 's|@no_zmq@|$(NO_ZMQ)|' \

diff --git a/depends/toolchain.cmake.in b/depends/toolchain.cmake.in
@@ -32,25 +32,46 @@ function(split_compiler_launcher env_compiler launcher compiler)
   set(${compiler} ${${compiler}} PARENT_SCOPE)
 endfunction()
 
-if(NOT CMAKE_C_COMPILER)
+if(NOT DEFINED CMAKE_C_FLAGS_INIT)
+  set(CMAKE_C_FLAGS_INIT "@CFLAGS@")
+endif()
+if(NOT DEFINED CMAKE_C_FLAGS_RELWITHDEBINFO_INIT)
+  set(CMAKE_C_FLAGS_RELWITHDEBINFO_INIT "@CFLAGS_RELEASE@")
+endif()
+if(NOT DEFINED CMAKE_C_FLAGS_DEBUG_INIT)
+  set(CMAKE_C_FLAGS_DEBUG_INIT "@CFLAGS_DEBUG@")
+endif()
+
+if(NOT DEFINED CMAKE_C_COMPILER)
   set(DEPENDS_C_COMPILER_WITH_LAUNCHER @CC@)
   split_compiler_launcher(DEPENDS_C_COMPILER_WITH_LAUNCHER CMAKE_C_COMPILER_LAUNCHER CMAKE_C_COMPILER)
   set(CMAKE_C_LINKER_LAUNCHER ${CMAKE_C_COMPILER_LAUNCHER})
-  set(DEPENDS_C_COMPILER_FLAGS @CFLAGS@)
 endif()
 
-if(NOT CMAKE_CXX_COMPILER)
+if(NOT DEFINED CMAKE_CXX_FLAGS_INIT)
+  set(CMAKE_CXX_FLAGS_INIT "@CXXFLAGS@")
+endif()
+if(NOT DEFINED CMAKE_CXX_FLAGS_RELWITHDEBINFO_INIT)
+  set(CMAKE_CXX_FLAGS_RELWITHDEBINFO_INIT "@CXXFLAGS_RELEASE@")
+endif()
+if(NOT DEFINED CMAKE_CXX_FLAGS_DEBUG_INIT)
+  set(CMAKE_CXX_FLAGS_DEBUG_INIT "@CXXFLAGS_DEBUG@")
+endif()
+
+if(NOT DEFINED CMAKE_CXX_COMPILER)
   set(DEPENDS_CXX_COMPILER_WITH_LAUNCHER @CXX@)
   split_compiler_launcher(DEPENDS_CXX_COMPILER_WITH_LAUNCHER CMAKE_CXX_COMPILER_LAUNCHER CMAKE_CXX_COMPILER)
   set(CMAKE_CXX_LINKER_LAUNCHER ${CMAKE_CXX_COMPILER_LAUNCHER})
-  set(DEPENDS_CXX_COMPILER_FLAGS @CXXFLAGS@)
 
   set(CMAKE_OBJCXX_COMPILER ${CMAKE_CXX_COMPILER})
   set(CMAKE_OBJCXX_COMPILER_LAUNCHER ${CMAKE_CXX_COMPILER_LAUNCHER})
   set(CMAKE_OBJCXX_LINKER_LAUNCHER ${CMAKE_CXX_LINKER_LAUNCHER})
 endif()
 
+# The DEPENDS_COMPILE_DEFINITIONS* variables are to be treated as lists.
 set(DEPENDS_COMPILE_DEFINITIONS @CPPFLAGS@)
+set(DEPENDS_COMPILE_DEFINITIONS_RELWITHDEBINFO @CPPFLAGS_RELEASE@)
+set(DEPENDS_COMPILE_DEFINITIONS_DEBUG @CPPFLAGS_DEBUG@)
 
 set(CMAKE_AR "@AR@")
 set(CMAKE_RANLIB "@RANLIB@")