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

2c4b229 cmake: Introduce `FUZZ_LIBS` (Hennadii Stepanov)
ea929c0 scripted-diff: Rename CMake helper module (Hennadii Stepanov)
8d238c1 cmake: Delete `check_cxx_source_links*` macros (Hennadii Stepanov)
71bf829 cmake: Convert `check_cxx_source_compiles_with_flags` to a function (Hennadii Stepanov)
88ee680 cmake: Delete `check_cxx_source_links_with_flags` macro (Hennadii Stepanov)
09e8fd2 build: Don't override CMake's default try_compile target (Hennadii Stepanov)

Pull request description:

  This was requested in bitcoin/bitcoin#31359 (comment).

  From bitcoin/bitcoin#31359 (comment):
  > (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](https://github.com/bitcoin/bitcoin/blob/d3f42fa08fc385752881afa5740f40287cfb4d8b/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 bitcoin/bitcoin#31359 (comment):
  > > 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 2c4b229
  theuni:
    utACK 2c4b229

Tree-SHA512: f72ffa8f50f216fc1a2f8027ba8ddfd4acd42b94ff6c1cb2138f2da51eb8f945660e97d3c247d7f3f7ec8dfebbccec3ab84347d6ae2e3f8a40f3d7aa8b14cde9

Author: fanquake

CMakeLists.txt

@@ -378,13 +378,16 @@ endif()
 target_link_options(sanitize_interface INTERFACE ${SANITIZER_LDFLAGS})
 
 if(BUILD_FUZZ_BINARY)
-  include(CheckSourceCompilesAndLinks)
-  check_cxx_source_links_with_flags("${SANITIZER_LDFLAGS}" "
+  target_link_libraries(core_interface INTERFACE ${FUZZ_LIBS})
+  include(CheckSourceCompilesWithFlags)
+  check_cxx_source_compiles_with_flags("
       #include <cstdint>
       #include <cstddef>
       extern \"C\" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { return 0; }
       // No main() function.
     " FUZZ_BINARY_LINKS_WITHOUT_MAIN_FUNCTION
+    LDFLAGS ${SANITIZER_LDFLAGS}
+    LINK_LIBRARIES ${FUZZ_LIBS}
   )
 endif()
 

cmake/crc32c.cmake

@@ -7,6 +7,7 @@
 # buildsystem.
 
 include(CheckCXXSourceCompiles)
+include(CheckSourceCompilesWithFlags)
 
 # Check for __builtin_prefetch support in the compiler.
 check_cxx_source_compiles("
@@ -42,7 +43,7 @@ if(MSVC)
 else()
   set(SSE42_CXXFLAGS -msse4.2)
 endif()
-check_cxx_source_compiles_with_flags("${SSE42_CXXFLAGS}" "
+check_cxx_source_compiles_with_flags("
   #include <cstdint>
   #if defined(_MSC_VER)
   #include <intrin.h>
@@ -58,11 +59,12 @@ check_cxx_source_compiles_with_flags("${SSE42_CXXFLAGS}" "
     return l;
   }
   " HAVE_SSE42
+  CXXFLAGS ${SSE42_CXXFLAGS}
 )
 
 # Check for ARMv8 w/ CRC and CRYPTO extensions support in the compiler.
 set(ARM64_CRC_CXXFLAGS -march=armv8-a+crc+crypto)
-check_cxx_source_compiles_with_flags("${ARM64_CRC_CXXFLAGS}" "
+check_cxx_source_compiles_with_flags("
   #include <arm_acle.h>
   #include <arm_neon.h>
 
@@ -76,6 +78,7 @@ check_cxx_source_compiles_with_flags("${ARM64_CRC_CXXFLAGS}" "
     return 0;
   }
   " HAVE_ARM64_CRC32C
+  CXXFLAGS ${ARM64_CRC_CXXFLAGS}
 )
 
 add_library(crc32c STATIC EXCLUDE_FROM_ALL

cmake/introspection.cmake

@@ -160,11 +160,11 @@ check_cxx_source_compiles("
 )
 
 if(NOT MSVC)
-  include(CheckSourceCompilesAndLinks)
+  include(CheckSourceCompilesWithFlags)
 
   # Check for SSE4.1 intrinsics.
   set(SSE41_CXXFLAGS -msse4.1)
-  check_cxx_source_compiles_with_flags("${SSE41_CXXFLAGS}" "
+  check_cxx_source_compiles_with_flags("
     #include <immintrin.h>
 
     int main()
@@ -175,12 +175,13 @@ if(NOT MSVC)
       return _mm_extract_epi32(r, 3);
     }
     " HAVE_SSE41
+    CXXFLAGS ${SSE41_CXXFLAGS}
   )
   set(ENABLE_SSE41 ${HAVE_SSE41})
 
   # Check for AVX2 intrinsics.
   set(AVX2_CXXFLAGS -mavx -mavx2)
-  check_cxx_source_compiles_with_flags("${AVX2_CXXFLAGS}" "
+  check_cxx_source_compiles_with_flags("
     #include <immintrin.h>
 
     int main()
@@ -189,12 +190,13 @@ if(NOT MSVC)
       return _mm256_extract_epi32(l, 7);
     }
     " HAVE_AVX2
+    CXXFLAGS ${AVX2_CXXFLAGS}
   )
   set(ENABLE_AVX2 ${HAVE_AVX2})
 
   # Check for x86 SHA-NI intrinsics.
   set(X86_SHANI_CXXFLAGS -msse4 -msha)
-  check_cxx_source_compiles_with_flags("${X86_SHANI_CXXFLAGS}" "
+  check_cxx_source_compiles_with_flags("
     #include <immintrin.h>
 
     int main()
@@ -205,12 +207,13 @@ if(NOT MSVC)
       return _mm_extract_epi32(_mm_sha256rnds2_epu32(i, j, k), 0);
     }
     " HAVE_X86_SHANI
+    CXXFLAGS ${X86_SHANI_CXXFLAGS}
   )
   set(ENABLE_X86_SHANI ${HAVE_X86_SHANI})
 
   # Check for ARMv8 SHA-NI intrinsics.
   set(ARM_SHANI_CXXFLAGS -march=armv8-a+crypto)
-  check_cxx_source_compiles_with_flags("${ARM_SHANI_CXXFLAGS}" "
+  check_cxx_source_compiles_with_flags("
     #include <arm_neon.h>
 
     int main()
@@ -222,6 +225,7 @@ if(NOT MSVC)
       vsha256su1q_u32(a, b, c);
     }
     " HAVE_ARM_SHANI
+    CXXFLAGS ${ARM_SHANI_CXXFLAGS}
   )
   set(ENABLE_ARM_SHANI ${HAVE_ARM_SHANI})
 endif()

cmake/minisketch.cmake

@@ -2,13 +2,15 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or https://opensource.org/license/mit/.
 
+include(CheckSourceCompilesWithFlags)
+
 # Check for clmul instructions support.
 if(MSVC)
-  set(CLMUL_CXXFLAGS)
+  set(CLMUL_CXXFLAGS "")
 else()
   set(CLMUL_CXXFLAGS -mpclmul)
 endif()
-check_cxx_source_compiles_with_flags("${CLMUL_CXXFLAGS}" "
+check_cxx_source_compiles_with_flags("
   #include <immintrin.h>
   #include <cstdint>
 
@@ -22,6 +24,7 @@ check_cxx_source_compiles_with_flags("${CLMUL_CXXFLAGS}" "
     return e == 0;
   }
   " HAVE_CLMUL
+  CXXFLAGS ${CLMUL_CXXFLAGS}
 )
 
 add_library(minisketch_common INTERFACE)

cmake/module/CheckSourceCompilesAndLinks.cmake

@@ -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()

cmake/module/CheckSourceCompilesWithFlags.cmake

@@ -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()