Merge bitcoin/bitcoin#31662: cmake: Do not modify CMAKE_TRY_COMPILE_TARGET_TYPE globally

2c4b229c90 cmake: Introduce `FUZZ_LIBS` (Hennadii Stepanov)
ea929c0848 scripted-diff: Rename CMake helper module (Hennadii Stepanov)
8d238c1dfd cmake: Delete `check_cxx_source_links*` macros (Hennadii Stepanov)
71bf8294a9 cmake: Convert `check_cxx_source_compiles_with_flags` to a function (Hennadii Stepanov)
88ee6800c9 cmake: Delete `check_cxx_source_links_with_flags` macro (Hennadii Stepanov)
09e8fd25b1 build: Don't override CMake's default try_compile target (Hennadii Stepanov)

Pull request description:

  This was requested in https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2515287092.

  From https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2511246212:
  > (Almost?) every CMake check internally uses the [`try_compile()`](https://cmake.org/cmake/help/latest/command/try_compile.html) command, whose behaviour, in turn, depends on the [`CMAKE_TRY_COMPILE_TARGET_TYPE`](https://cmake.org/cmake/help/latest/variable/CMAKE_TRY_COMPILE_TARGET_TYPE.html) variable:
  >
  >    1. The default value, `EXECUTABLE`, enables both compiler and linker checks.
  >
  >    2. The `STATIC_LIBRARY` value enables only compiler checks.
  >
  >
  > To mimic Autotools' behaviour, we [disabled](d3f42fa08f/cmake/module/CheckSourceCompilesAndLinks.cmake (L9-L10)) linker checks by setting `CMAKE_TRY_COMPILE_TARGET_TYPE` to `STATIC_LIBRARY` globally (perhaps not the best design). This effectively separates the entire CMake script into regions where `CMAKE_TRY_COMPILE_TARGET_TYPE` is:
  >
  >    * unset
  >
  >    * set to `STATIC_LIBRARY`
  >
  >    * set to `EXECUTABLE`

  From https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2515287092:
  > > This seems very fragile and unintuitive, and the fact that this could silently break at any point is not documented in any way. I don't think other bad design decisions should lead to us having to write even more boilerplate code to fix things that should "just work" (minus the upstream bugs).
  >
  > Agreed. I forgot that we set `CMAKE_TRY_COMPILE_TARGET_TYPE` globally. And even worse, it's buried in a module. If that upsets CMake internal tests, I think we should undo that.

  This PR ensures that `CMAKE_TRY_COMPILE_TARGET_TYPE` is modified only within local scopes.

  Additionally, the `FUZZ_LIBS` variable has been introduced to handle additional libraries required for linking, rather than link options, in certain build environment, such as OSS-Fuzz.

ACKs for top commit:
  TheCharlatan:
    Re-ACK 2c4b229c90
  theuni:
    utACK 2c4b229c90

Tree-SHA512: f72ffa8f50f216fc1a2f8027ba8ddfd4acd42b94ff6c1cb2138f2da51eb8f945660e97d3c247d7f3f7ec8dfebbccec3ab84347d6ae2e3f8a40f3d7aa8b14cde9
This commit is contained in:
merge-script
2025-02-20 15:07:03 -05:00
7 changed files with 76 additions and 64 deletions

View File

@@ -1,39 +0,0 @@
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.
include_guard(GLOBAL)
include(CheckCXXSourceCompiles)
include(CMakePushCheckState)
# This avoids running the linker.
set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
macro(check_cxx_source_links source)
set(CMAKE_TRY_COMPILE_TARGET_TYPE EXECUTABLE)
check_cxx_source_compiles("${source}" ${ARGN})
set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
endmacro()
macro(check_cxx_source_compiles_with_flags flags source)
cmake_push_check_state(RESET)
set(CMAKE_REQUIRED_FLAGS ${flags})
list(JOIN CMAKE_REQUIRED_FLAGS " " CMAKE_REQUIRED_FLAGS)
check_cxx_source_compiles("${source}" ${ARGN})
cmake_pop_check_state()
endmacro()
macro(check_cxx_source_links_with_flags flags source)
cmake_push_check_state(RESET)
set(CMAKE_REQUIRED_FLAGS ${flags})
list(JOIN CMAKE_REQUIRED_FLAGS " " CMAKE_REQUIRED_FLAGS)
check_cxx_source_links("${source}" ${ARGN})
cmake_pop_check_state()
endmacro()
macro(check_cxx_source_links_with_libs libs source)
cmake_push_check_state(RESET)
set(CMAKE_REQUIRED_LIBRARIES "${libs}")
check_cxx_source_links("${source}" ${ARGN})
cmake_pop_check_state()
endmacro()

View File

@@ -0,0 +1,34 @@
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.
include_guard(GLOBAL)
#[=[
Check once if C++ source code can be compiled.
Options:
CXXFLAGS - A list of additional flags to pass to the compiler.
LDFLAGS - A list of additional flags to pass to the linker.
LINK_LIBRARIES - A list of libraries to add to the link command.
For historical reasons, among the CMake `CMAKE_REQUIRED_*` variables that influence
`check_cxx_source_compiles()`, only `CMAKE_REQUIRED_FLAGS` is a string rather than
a list. Additionally, `target_compile_options()` also expects a list of options.
The `check_cxx_source_compiles_with_flags()` function handles this case and accepts
`CXXFLAGS` as a list, simplifying the code at the caller site.
#]=]
function(check_cxx_source_compiles_with_flags source result_var)
cmake_parse_arguments(PARSE_ARGV 2 _ "" "" "CXXFLAGS;LDFLAGS;LINK_LIBRARIES")
list(JOIN __CXXFLAGS " " CMAKE_REQUIRED_FLAGS)
set(CMAKE_REQUIRED_LINK_OPTIONS ${__LDFLAGS})
set(CMAKE_REQUIRED_LIBRARIES ${__LINK_LIBRARIES})
include(CheckCXXSourceCompiles)
check_cxx_source_compiles("${source}" ${result_var})
set(${result_var} ${${result_var}} PARENT_SCOPE)
endfunction()

View File

@@ -25,10 +25,13 @@ function(test_append_socket_library target)
}
")
include(CheckSourceCompilesAndLinks)
check_cxx_source_links("${check_socket_source}" IFADDR_LINKS_WITHOUT_LIBSOCKET)
include(CheckCXXSourceCompiles)
check_cxx_source_compiles("${check_socket_source}" IFADDR_LINKS_WITHOUT_LIBSOCKET)
if(NOT IFADDR_LINKS_WITHOUT_LIBSOCKET)
check_cxx_source_links_with_libs(socket "${check_socket_source}" IFADDR_NEEDS_LINK_TO_LIBSOCKET)
include(CheckSourceCompilesWithFlags)
check_cxx_source_compiles_with_flags("${check_socket_source}" IFADDR_NEEDS_LINK_TO_LIBSOCKET
LINK_LIBRARIES socket
)
if(IFADDR_NEEDS_LINK_TO_LIBSOCKET)
target_link_libraries(${target} INTERFACE socket)
else()
@@ -77,16 +80,17 @@ function(test_append_atomic_library target)
}
")
include(CheckSourceCompilesAndLinks)
check_cxx_source_links("${check_atomic_source}" STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC)
if(STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC)
return()
endif()
check_cxx_source_links_with_libs(atomic "${check_atomic_source}" STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC)
if(STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC)
target_link_libraries(${target} INTERFACE atomic)
else()
message(FATAL_ERROR "Cannot figure out how to use std::atomic.")
include(CheckCXXSourceCompiles)
check_cxx_source_compiles("${check_atomic_source}" STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC)
if(NOT STD_ATOMIC_LINKS_WITHOUT_LIBATOMIC)
include(CheckSourceCompilesWithFlags)
check_cxx_source_compiles_with_flags("${check_atomic_source}" STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC
LINK_LIBRARIES atomic
)
if(STD_ATOMIC_NEEDS_LINK_TO_LIBATOMIC)
target_link_libraries(${target} INTERFACE atomic)
else()
message(FATAL_ERROR "Cannot figure out how to use std::atomic.")
endif()
endif()
endfunction()