Skip to content

Commit

Permalink
Merge #192: cmake: Switch to libsecp256k1 upstream build system
Browse files Browse the repository at this point in the history
458d54c cmake: Run secp256k1 tests (Hennadii Stepanov)
bfe7c4d [FIXUP] cmake: Enable tests before adding subdirectories (Hennadii Stepanov)
70ec2d9 cmake: Reorder subdirectories (Hennadii Stepanov)
cdd843a cmake: Override build configurations for libsecp256k1 (Hennadii Stepanov)
65f15d1 cmake: Pass to secp256k1 build system only sanitizer flags (Hennadii Stepanov)
469e1ad cmake: Remove C stuff from summary (Hennadii Stepanov)
c3134b0 cmake: Let libsecp256k1 manage config-specific C flags (Hennadii Stepanov)
f0aec78 cmake: Switch to libsecp256k1 upstream build system (Hennadii Stepanov)

Pull request description:

  To test this PR, one can compare the ouputs of the following commands:
  - Autotools:
  ```
  $ make -C src/secp256k1 libsecp256k1.la V=1
  ```
  - CMake:
  ```
  $ cmake --build build -t secp256k1 --verbose
  ```

  Also sets of the libsecp256k1-specific tests can be compared between `make check` and `ctest`.

ACKs for top commit:
  real-or-random:
    utACK 458d54c as far as the secp256k1 config is concerned

Tree-SHA512: 212830a31832fb4af609eb7799ad140a22a2bc13c0685286dc58a25d9e5767394e94cbc969268a4e6adcf3c6a0b059f0f74dd7220e30dbbcbfebd1647279e5bd
  • Loading branch information
hebasto committed Jun 30, 2024
2 parents 8948ba8 + 458d54c commit a7cc7d8
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 132 deletions.
38 changes: 11 additions & 27 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,13 @@ target_link_libraries(core_base_interface INTERFACE
Threads::Threads
)

add_library(sanitize_interface INTERFACE)
target_link_libraries(core_base_interface INTERFACE sanitize_interface)
if(SANITIZERS)
# First check if the compiler accepts flags. If an incompatible pair like
# -fsanitize=address,thread is used here, this check will fail. This will also
# fail if a bad argument is passed, e.g. -fsanitize=undfeined
try_append_cxx_flags("-fsanitize=${SANITIZERS}" TARGET core_base_interface
try_append_cxx_flags("-fsanitize=${SANITIZERS}" TARGET sanitize_interface
RESULT_VAR cxx_supports_sanitizers
SKIP_LINK
)
Expand All @@ -353,7 +355,7 @@ if(SANITIZERS)
message(FATAL_ERROR "Linker did not accept requested flags, you are missing required libraries.")
endif()
endif()
target_link_options(core_base_interface INTERFACE ${SANITIZER_LDFLAGS})
target_link_options(sanitize_interface INTERFACE ${SANITIZER_LDFLAGS})

include(AddBoostIfNeeded)
add_boost_if_needed()
Expand All @@ -369,7 +371,6 @@ include(cmake/ccache.cmake)
include(cmake/crc32c.cmake)
include(cmake/leveldb.cmake)
include(cmake/minisketch.cmake)
include(cmake/secp256k1.cmake)

add_library(warn_interface INTERFACE)
target_link_libraries(core_interface INTERFACE warn_interface)
Expand Down Expand Up @@ -429,15 +430,12 @@ target_compile_definitions(core_interface_debug INTERFACE
)
# We leave assertions on.
if(MSVC)
remove_c_flag_from_all_configs(/DNDEBUG)
remove_cxx_flag_from_all_configs(/DNDEBUG)
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.
# Adjust flags used by the 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)

are_flags_overridden(CMAKE_CXX_FLAGS_DEBUG cxx_flags_debug_overridden)
Expand All @@ -447,6 +445,7 @@ else()
if(compiler_supports_g3)
replace_cxx_flag_in_config(Debug -g -g3)
endif()
unset(compiler_supports_g3)

try_append_cxx_flags("-ftrapv" RESULT_VAR compiler_supports_ftrapv)
if(compiler_supports_ftrapv)
Expand All @@ -463,24 +462,6 @@ else()
)
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()
unset(compiler_supports_g3)
unset(c_flags_debug_overridden)
endif()

set(CMAKE_CXX_FLAGS_COVERAGE "-Og --coverage")
Expand Down Expand Up @@ -599,6 +580,9 @@ if(DEFINED ENV{LDFLAGS})
deduplicate_flags(CMAKE_EXE_LINKER_FLAGS)
endif()

if(BUILD_TESTS)
enable_testing()
endif()
# TODO: The `CMAKE_SKIP_BUILD_RPATH` variable setting can be deleted
# in the future after reordering Guix script commands to
# perform binary checks after the installation step.
Expand All @@ -607,10 +591,11 @@ endif()
# - https://github.com/bitcoin/bitcoin/pull/30312#issuecomment-2191235833
set(CMAKE_SKIP_BUILD_RPATH TRUE)
set(CMAKE_SKIP_INSTALL_RPATH TRUE)
add_subdirectory(src)
add_subdirectory(test)
add_subdirectory(doc)

add_subdirectory(src)

include(cmake/tests.cmake)

include(Maintenance)
Expand Down Expand Up @@ -664,7 +649,6 @@ else()
set(cross_status "FALSE")
endif()
message("Cross compiling ....................... ${cross_status}")
message("C compiler ............................ ${CMAKE_C_COMPILER_ID} ${CMAKE_C_COMPILER_VERSION}, ${CMAKE_C_COMPILER}")
message("C++ compiler .......................... ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}, ${CMAKE_CXX_COMPILER}")
include(FlagsSummary)
flags_summary()
Expand Down
11 changes: 0 additions & 11 deletions cmake/module/FlagsSummary.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,6 @@ function(print_flags_per_config config indent_num)
get_target_interface(definitions ${config} core_interface COMPILE_DEFINITIONS)
indent_message("Preprocessor defined macros ..........." "${definitions}" ${indent_num})

string(STRIP "${CMAKE_C_FLAGS} ${CMAKE_C_FLAGS_${config_uppercase}}" combined_c_flags)
string(STRIP "${combined_c_flags} ${CMAKE_C${CMAKE_C_STANDARD}_STANDARD_COMPILE_OPTION}" combined_c_flags)
if(CMAKE_POSITION_INDEPENDENT_CODE)
string(JOIN " " combined_c_flags ${combined_c_flags} ${CMAKE_C_COMPILE_OPTIONS_PIC})
endif()
get_target_interface(core_c_flags ${config} core_base_interface COMPILE_OPTIONS)
string(STRIP "${combined_c_flags} ${core_c_flags}" combined_c_flags)
string(STRIP "${combined_c_flags} ${APPEND_CPPFLAGS}" combined_c_flags)
string(STRIP "${combined_c_flags} ${APPEND_CFLAGS}" combined_c_flags)
indent_message("C compiler flags ......................" "${combined_c_flags}" ${indent_num})

string(STRIP "${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${config_uppercase}}" combined_cxx_flags)
string(STRIP "${combined_cxx_flags} ${CMAKE_CXX${CMAKE_CXX_STANDARD}_STANDARD_COMPILE_OPTION}" combined_cxx_flags)
if(CMAKE_POSITION_INDEPENDENT_CODE)
Expand Down
28 changes: 0 additions & 28 deletions cmake/module/ProcessConfigurations.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -84,23 +84,6 @@ function(set_default_config config)
endif()
endfunction()

function(remove_c_flag_from_all_configs flag)
get_all_configs(all_configs)
foreach(config IN LISTS all_configs)
string(TOUPPER "${config}" config_uppercase)
set(flags "${CMAKE_C_FLAGS_${config_uppercase}}")
separate_arguments(flags)
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()

function(remove_cxx_flag_from_all_configs flag)
get_all_configs(all_configs)
foreach(config IN LISTS all_configs)
Expand All @@ -118,17 +101,6 @@ function(remove_cxx_flag_from_all_configs flag)
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}}")
Expand Down
62 changes: 0 additions & 62 deletions cmake/secp256k1.cmake

This file was deleted.

2 changes: 0 additions & 2 deletions cmake/tests.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
# Distributed under the MIT software license, see the accompanying
# file COPYING or https://opensource.org/license/mit/.

include(CTest)

if(TARGET bitcoin-util AND TARGET bitcoin-tx AND PYTHON_COMMAND)
add_test(NAME util_test_runner
COMMAND ${CMAKE_COMMAND} -E env BITCOINUTIL=$<TARGET_FILE:bitcoin-util> BITCOINTX=$<TARGET_FILE:bitcoin-tx> ${PYTHON_COMMAND} ${CMAKE_BINARY_DIR}/test/util/test_runner.py
Expand Down
36 changes: 34 additions & 2 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Copyright (c) 2023 The Bitcoin Core developers
# Copyright (c) 2023-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
# file COPYING or https://opensource.org/license/mit/.

include(GNUInstallDirs)
include(AddWindowsResources)
Expand Down Expand Up @@ -38,6 +38,38 @@ if(WITH_MULTIPROCESS)
add_subdirectory(ipc)
endif()

#=============================
# secp256k1 subtree
#=============================
message("")
message("Configuring secp256k1 subtree...")
set(SECP256K1_DISABLE_SHARED ON CACHE BOOL "" FORCE)
set(SECP256K1_ENABLE_MODULE_ECDH OFF CACHE BOOL "" FORCE)
set(SECP256K1_ENABLE_MODULE_RECOVERY ON CACHE BOOL "" FORCE)
set(SECP256K1_ECMULT_GEN_KB 86 CACHE STRING "" FORCE)
set(SECP256K1_BUILD_BENCHMARK OFF CACHE BOOL "" FORCE)
set(SECP256K1_BUILD_TESTS ${BUILD_TESTS} CACHE BOOL "" FORCE)
set(SECP256K1_BUILD_EXHAUSTIVE_TESTS ${BUILD_TESTS} CACHE BOOL "" FORCE)
set(SECP256K1_BUILD_CTIME_TESTS OFF CACHE BOOL "" FORCE)
set(SECP256K1_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE)
include(GetTargetInterface)
# -fsanitize and related flags apply to both C++ and C,
# so we can pass them down to libsecp256k1 as CFLAGS.
get_target_interface(core_sanitizer_cxx_flags "" sanitize_interface COMPILE_OPTIONS)
set(SECP256K1_LATE_CFLAGS ${core_sanitizer_cxx_flags} CACHE STRING "" FORCE)
unset(core_sanitizer_cxx_flags)
# We want to build libsecp256k1 with the most tested RelWithDebInfo configuration.
enable_language(C)
foreach(config IN LISTS CMAKE_BUILD_TYPE CMAKE_CONFIGURATION_TYPES)
if(config STREQUAL "")
continue()
endif()
string(TOUPPER "${config}" config)
set(CMAKE_C_FLAGS_${config} "${CMAKE_C_FLAGS_RELWITHDEBINFO}")
endforeach()
add_subdirectory(secp256k1)
set_target_properties(secp256k1 PROPERTIES EXPORT_COMPILE_COMMANDS OFF)
string(APPEND CMAKE_C_COMPILE_OBJECT " ${APPEND_CPPFLAGS} ${APPEND_CFLAGS}")

# Stable, backwards-compatible consensus functionality.
add_library(bitcoin_consensus STATIC EXCLUDE_FROM_ALL
Expand Down

0 comments on commit a7cc7d8

Please sign in to comment.