Skip to content

Commit

Permalink
Merge #287: cmake: Fix TryAppendCXXFlags module
Browse files Browse the repository at this point in the history 
634b3c7 fixup! cmake: Add `TryAppendCXXFlags` module (Hennadii Stepanov)
ca8a111 fixup! cmake: Add compiler diagnostic flags (Hennadii Stepanov)
9bc8400 fixup! cmake: Add `TryAppendCXXFlags` module (Hennadii Stepanov)

Pull request description:

  This PR fixes a couple of bugs in the `TryAppendCXXFlags` module that I noticed during testing on the staging branch:

  1. Fixes the check of multiple flags. As documented, a space must be used as a separator:https://github.com/hebasto/bitcoin/blob/56450e50f0248969ff4688fc15e4f051c3b8cb65/cmake/module/TryAppendCXXFlags.cmake#L12-L14

  Otherwise, only the first flag is actually being checked.

  Here are excerpts from `build/CMakeFiles/CMakeConfigureLog.yaml`:
  - on the staging branch @ 9256718:
  ```
      kind: "try_compile-v1"
      backtrace:
        - "/usr/share/cmake-3.27/Modules/Internal/CheckSourceCompiles.cmake:101 (try_compile)"
        - "/usr/share/cmake-3.27/Modules/CheckCXXSourceCompiles.cmake:52 (cmake_check_source_compiles)"
        - "cmake/module/TryAppendCXXFlags.cmake:70 (check_cxx_source_compiles)"
        - "CMakeLists.txt:416 (try_append_cxx_flags)"
      checks:
        - "Performing Test CXX_SUPPORTS__WFORMAT__WFORMAT_SECURITY"
      directories:
        source: "/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-8Q9qSw"
        binary: "/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-8Q9qSw"
      cmakeVariables:
        CMAKE_CXX_FLAGS: ""
        CMAKE_CXX_FLAGS_DEBUG: "-g"
        CMAKE_CXX_LINK_NO_PIE_SUPPORTED: "1"
        CMAKE_EXE_LINKER_FLAGS: ""
        CMAKE_MODULE_PATH: "/home/hebasto/git/bitcoin/cmake/module"
        CMAKE_POSITION_INDEPENDENT_CODE: "ON"
      buildResult:
        variable: "CXX_SUPPORTS__WFORMAT__WFORMAT_SECURITY"
        cached: true
        stdout: |
          Change Dir: '/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-8Q9qSw'

          Run Build Command(s): /usr/bin/cmake -E env VERBOSE=1 /usr/bin/gmake -f Makefile cmTC_52a2c/fast
          /usr/bin/gmake  -f CMakeFiles/cmTC_52a2c.dir/build.make CMakeFiles/cmTC_52a2c.dir/build
          gmake[1]: Entering directory '/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-8Q9qSw'
          Building CXX object CMakeFiles/cmTC_52a2c.dir/src.cxx.o
          /usr/bin/c++ -DCXX_SUPPORTS__WFORMAT__WFORMAT_SECURITY  -Wformat -std=c++20 -fPIC -o CMakeFiles/cmTC_52a2c.dir/src.cxx.o -c /home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-8Q9qSw/src.cxx
          Linking CXX static library libcmTC_52a2c.a
          /usr/bin/cmake -P CMakeFiles/cmTC_52a2c.dir/cmake_clean_target.cmake
          /usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_52a2c.dir/link.txt --verbose=1
          /usr/bin/ar qc libcmTC_52a2c.a CMakeFiles/cmTC_52a2c.dir/src.cxx.o
          /usr/bin/ranlib libcmTC_52a2c.a
          gmake[1]: Leaving directory '/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-8Q9qSw'

        exitCode: 0
  ```
  - with this PR:
  ```
      kind: "try_compile-v1"
      backtrace:
        - "/usr/share/cmake-3.27/Modules/Internal/CheckSourceCompiles.cmake:101 (try_compile)"
        - "/usr/share/cmake-3.27/Modules/CheckCXXSourceCompiles.cmake:52 (cmake_check_source_compiles)"
        - "cmake/module/TryAppendCXXFlags.cmake:70 (check_cxx_source_compiles)"
        - "CMakeLists.txt:416 (try_append_cxx_flags)"
      checks:
        - "Performing Test CXX_SUPPORTS__WFORMAT__WFORMAT_SECURITY"
      directories:
        source: "/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-SK4Pzn"
        binary: "/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-SK4Pzn"
      cmakeVariables:
        CMAKE_CXX_FLAGS: ""
        CMAKE_CXX_FLAGS_DEBUG: "-g"
        CMAKE_CXX_LINK_NO_PIE_SUPPORTED: "1"
        CMAKE_EXE_LINKER_FLAGS: ""
        CMAKE_MODULE_PATH: "/home/hebasto/git/bitcoin/cmake/module"
        CMAKE_POSITION_INDEPENDENT_CODE: "ON"
      buildResult:
        variable: "CXX_SUPPORTS__WFORMAT__WFORMAT_SECURITY"
        cached: true
        stdout: |
          Change Dir: '/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-SK4Pzn'

          Run Build Command(s): /usr/bin/cmake -E env VERBOSE=1 /usr/bin/gmake -f Makefile cmTC_ef70a/fast
          /usr/bin/gmake  -f CMakeFiles/cmTC_ef70a.dir/build.make CMakeFiles/cmTC_ef70a.dir/build
          gmake[1]: Entering directory '/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-SK4Pzn'
          Building CXX object CMakeFiles/cmTC_ef70a.dir/src.cxx.o
          /usr/bin/c++ -DCXX_SUPPORTS__WFORMAT__WFORMAT_SECURITY  -Wformat -Wformat-security -Werror -std=c++20 -fPIC -o CMakeFiles/cmTC_ef70a.dir/src.cxx.o -c /home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-SK4Pzn/src.cxx
          Linking CXX static library libcmTC_ef70a.a
          /usr/bin/cmake -P CMakeFiles/cmTC_ef70a.dir/cmake_clean_target.cmake
          /usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_ef70a.dir/link.txt --verbose=1
          /usr/bin/ar qc libcmTC_ef70a.a CMakeFiles/cmTC_ef70a.dir/src.cxx.o
          /usr/bin/ranlib libcmTC_ef70a.a
          gmake[1]: Leaving directory '/home/hebasto/git/bitcoin/build/CMakeFiles/CMakeScratch/TryCompile-SK4Pzn'

        exitCode: 0
  ```

  2. Avoid skipping checks for linking due to caching of the `result` variable. The effect can be observed in the configuration output.

ACKs for top commit:
  m3dwards:
    ACK 634b3c7

Tree-SHA512: 986f8efcff792f953cafb32d71e1e941303af0f03bc1187f0a371bdc5b950b4cf3764360fcc19fefae6464ac2ed4922b2a57da25d9d6df0d2135369b6c5d8cee
  • Loading branch information
@hebasto
hebasto committed Jul 26, 2024
2 parents 9256718 + 634b3c7 commit e4f7e12
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ else()
try_append_cxx_flags("-Wextra" TARGET warn_interface SKIP_LINK)
try_append_cxx_flags("-Wgnu" TARGET warn_interface SKIP_LINK)
# Some compilers will ignore -Wformat-security without -Wformat, so just combine the two here.
try_append_cxx_flags("-Wformat;-Wformat-security" TARGET warn_interface SKIP_LINK)
try_append_cxx_flags("-Wformat -Wformat-security" TARGET warn_interface SKIP_LINK)
try_append_cxx_flags("-Wvla" TARGET warn_interface SKIP_LINK)
try_append_cxx_flags("-Wshadow-field" TARGET warn_interface SKIP_LINK)
try_append_cxx_flags("-Wthread-safety" TARGET warn_interface SKIP_LINK)
Expand Down
32 changes: 18 additions & 14 deletions cmake/module/TryAppendCXXFlags.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,24 +52,27 @@ function(try_append_cxx_flags flags)
"IF_CHECK_PASSED;IF_CHECK_FAILED" # multi_value_keywords
)

string(MAKE_C_IDENTIFIER "${flags}" result)
string(TOUPPER "${result}" result)
string(PREPEND result CXX_SUPPORTS_)
set(flags_as_string "${flags}")
separate_arguments(flags)

string(MAKE_C_IDENTIFIER "${flags_as_string}" id_string)
string(TOUPPER "${id_string}" id_string)

set(source "int main() { return 0; }")
if(DEFINED TACXXF_SOURCE AND NOT TACXXF_SOURCE STREQUAL source)
set(source "${TACXXF_SOURCE}")
string(SHA256 source_hash "${source}")
string(SUBSTRING ${source_hash} 0 4 source_hash_head)
string(APPEND result _${source_hash_head})
string(APPEND id_string _${source_hash_head})
endif()

# This avoids running a linker.
set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
set(CMAKE_REQUIRED_FLAGS "${flags} ${working_compiler_werror_flag}")
check_cxx_source_compiles("${source}" ${result})
set(CMAKE_REQUIRED_FLAGS "${flags_as_string} ${working_compiler_werror_flag}")
set(compiler_result CXX_SUPPORTS_${id_string})
check_cxx_source_compiles("${source}" ${compiler_result})

if(${result})
if(${compiler_result})
if(DEFINED TACXXF_IF_CHECK_PASSED)
if(DEFINED TACXXF_TARGET)
target_compile_options(${TACXXF_TARGET} INTERFACE ${TACXXF_IF_CHECK_PASSED})
Expand All @@ -82,7 +85,7 @@ function(try_append_cxx_flags flags)
target_compile_options(${TACXXF_TARGET} INTERFACE ${flags})
endif()
if(DEFINED TACXXF_VAR)
string(STRIP "${${TACXXF_VAR}} ${flags}" ${TACXXF_VAR})
string(STRIP "${${TACXXF_VAR}} ${flags_as_string}" ${TACXXF_VAR})
endif()
endif()
elseif(DEFINED TACXXF_IF_CHECK_FAILED)
Expand All @@ -99,19 +102,20 @@ function(try_append_cxx_flags flags)
endif()

if(DEFINED TACXXF_RESULT_VAR)
set(${TACXXF_RESULT_VAR} "${${result}}" PARENT_SCOPE)
set(${TACXXF_RESULT_VAR} "${${compiler_result}}" PARENT_SCOPE)
endif()

if(NOT ${result} OR TACXXF_SKIP_LINK)
if(NOT ${compiler_result} OR TACXXF_SKIP_LINK)
return()
endif()

# This forces running a linker.
set(CMAKE_TRY_COMPILE_TARGET_TYPE EXECUTABLE)
set(CMAKE_REQUIRED_FLAGS "${flags} ${working_linker_werror_flag}")
check_cxx_source_compiles("${source}" ${result})
set(CMAKE_REQUIRED_FLAGS "${flags_as_string} ${working_linker_werror_flag}")
set(linker_result LINKER_SUPPORTS_${id_string})
check_cxx_source_compiles("${source}" ${linker_result})

if(${result})
if(${linker_result})
if(DEFINED TACXXF_IF_CHECK_PASSED)
if(DEFINED TACXXF_TARGET)
target_link_options(${TACXXF_TARGET} INTERFACE ${TACXXF_IF_CHECK_PASSED})
Expand All @@ -122,7 +126,7 @@ function(try_append_cxx_flags flags)
endif()
endif()
else()
message(WARNING "The ${flags} fail(s) to link.")
message(WARNING "'${flags_as_string}' fail(s) to link.")
endif()
endfunction()

Expand Down

0 comments on commit e4f7e12

Please sign in to comment.