-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: Add CMake-based build system (3 of N) #7
Changes from all commits
6317164
a06290e
0e2287c
eb3546f
e990724
318dd82
d028a49
ffd10d1
a80fb45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,71 @@ | ||||
# Copyright (c) 2023 The Bitcoin Core developers | ||||
# Distributed under the MIT software license, see the accompanying | ||||
# file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||||
|
||||
# Check for clmul instructions support. | ||||
if(MSVC) | ||||
set(CLMUL_CXXFLAGS) | ||||
hebasto marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
else() | ||||
set(CLMUL_CXXFLAGS -mpclmul) | ||||
endif() | ||||
check_cxx_source_compiles_with_flags("${CLMUL_CXXFLAGS}" " | ||||
#include <immintrin.h> | ||||
#include <cstdint> | ||||
|
||||
int main() | ||||
{ | ||||
__m128i a = _mm_cvtsi64_si128((uint64_t)7); | ||||
__m128i b = _mm_clmulepi64_si128(a, a, 37); | ||||
__m128i c = _mm_srli_epi64(b, 41); | ||||
__m128i d = _mm_xor_si128(b, c); | ||||
uint64_t e = _mm_cvtsi128_si64(d); | ||||
return e == 0; | ||||
} | ||||
" HAVE_CLMUL | ||||
) | ||||
|
||||
add_library(minisketch_defs INTERFACE) | ||||
target_compile_definitions(minisketch_defs INTERFACE | ||||
DISABLE_DEFAULT_FIELDS | ||||
ENABLE_FIELD_32 | ||||
$<$<AND:$<BOOL:${HAVE_BUILTIN_CLZL}>,$<BOOL:${HAVE_BUILTIN_CLZLL}>>:HAVE_CLZ> | ||||
) | ||||
|
||||
if(HAVE_CLMUL) | ||||
add_library(minisketch_clmul OBJECT EXCLUDE_FROM_ALL | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why all the trouble of another library? For others so far, we've been changing source properties to append necessary compiler flags. I think that'd be easier here too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both approaches are essentially the same. This one seems more concise and maintainable because source files are mentioned once only. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The point is that an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still not convinced that this is cleaner, but I'm also not opposed at all. Point taken that I need to get my head out of autotools mode, heh. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm used to seeing these object libraries in cases where the compiled objects are used more than once by some project internal libraries. If these object libraries are to be used instead of defining properties on a GLOB'ed collection of source files I'd like to know when to use either, since this is setting a precedent. Can you comment on this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still considering it only as a style difference; therefore, more DRYer and readable option should be chosen. |
||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_1byte.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_2bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_3bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_4bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_5bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_6bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_7bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/clmul_8bytes.cpp | ||||
) | ||||
target_compile_definitions(minisketch_clmul PUBLIC HAVE_CLMUL) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To propagate this usage requirement to the Comes from bitcoin/src/Makefile.minisketch.include Line 27 in 74981aa
|
||||
target_compile_options(minisketch_clmul PRIVATE ${CLMUL_CXXFLAGS}) | ||||
target_link_libraries(minisketch_clmul PRIVATE minisketch_defs) | ||||
endif() | ||||
|
||||
add_library(minisketch STATIC EXCLUDE_FROM_ALL | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/minisketch.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_1byte.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_2bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_3bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_4bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_5bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_6bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_7bytes.cpp | ||||
${PROJECT_SOURCE_DIR}/src/minisketch/src/fields/generic_8bytes.cpp | ||||
) | ||||
|
||||
target_include_directories(minisketch | ||||
PUBLIC | ||||
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src/minisketch/include> | ||||
) | ||||
|
||||
target_link_libraries(minisketch | ||||
PRIVATE | ||||
minisketch_defs | ||||
$<TARGET_NAME_IF_EXISTS:minisketch_clmul> | ||||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
# Copyright (c) 2023 The Bitcoin Core developers | ||
# Distributed under the MIT software license, see the accompanying | ||
# file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
||
function(add_boost_if_needed) | ||
#[=[ | ||
TODO: Not all targets, which will be added in the future, require | ||
Boost. Therefore, a proper check will be appropriate here. | ||
|
||
Implementation notes: | ||
Although only Boost headers are used to build Bitcoin Core, | ||
we still leverage a standard CMake's approach to handle | ||
dependencies, i.e., the Boost::headers "library". | ||
A command target_link_libraries(target PRIVATE Boost::headers) | ||
will propagate Boost::headers usage requirements to the target. | ||
For Boost::headers such usage requirements is an include | ||
directory and other added INTERFACE properties. | ||
]=] | ||
|
||
if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin" AND BREW_COMMAND) | ||
execute_process( | ||
COMMAND ${BREW_COMMAND} --prefix boost | ||
OUTPUT_VARIABLE BOOST_ROOT | ||
ERROR_QUIET | ||
OUTPUT_STRIP_TRAILING_WHITESPACE | ||
) | ||
endif() | ||
|
||
set(Boost_NO_BOOST_CMAKE ON) | ||
find_package(Boost 1.64.0 REQUIRED) | ||
set_target_properties(Boost::boost PROPERTIES IMPORTED_GLOBAL TRUE) | ||
target_compile_definitions(Boost::boost INTERFACE | ||
$<$<CONFIG:Debug>:BOOST_MULTI_INDEX_ENABLE_SAFE_MODE> | ||
) | ||
if(CMAKE_VERSION VERSION_LESS 3.15) | ||
add_library(Boost::headers ALIAS Boost::boost) | ||
endif() | ||
|
||
mark_as_advanced(Boost_INCLUDE_DIR) | ||
endfunction() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
# Copyright (c) 2023 The Bitcoin Core developers | ||
# Distributed under the MIT software license, see the accompanying | ||
# file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
||
macro(check_evhttp_connection_get_peer target) | ||
# Check whether evhttp_connection_get_peer expects const char**. | ||
# Fail if neither are available. | ||
check_cxx_source_compiles(" | ||
#include <cstdint> | ||
#include <event2/http.h> | ||
|
||
int main() | ||
{ | ||
evhttp_connection* conn = (evhttp_connection*)1; | ||
const char* host; | ||
uint16_t port; | ||
evhttp_connection_get_peer(conn, &host, &port); | ||
} | ||
" HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR | ||
) | ||
target_compile_definitions(${target} INTERFACE | ||
$<$<BOOL:${HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR}>:HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR=1> | ||
) | ||
endmacro() | ||
|
||
function(add_libevent_if_needed) | ||
# TODO: Not all targets, which will be added in the future, | ||
# require libevent. Therefore, a proper check will be | ||
# appropriate here. | ||
|
||
set(libevent_minimum_version 2.1.8) | ||
|
||
if(MSVC) | ||
find_package(Libevent ${libevent_minimum_version} REQUIRED COMPONENTS extra CONFIG) | ||
check_evhttp_connection_get_peer(libevent::extra) | ||
add_library(libevent::libevent ALIAS libevent::extra) | ||
return() | ||
endif() | ||
|
||
find_package(PkgConfig) | ||
pkg_check_modules(libevent REQUIRED libevent>=${libevent_minimum_version} IMPORTED_TARGET GLOBAL) | ||
check_evhttp_connection_get_peer(PkgConfig::libevent) | ||
hebasto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
target_link_libraries(PkgConfig::libevent INTERFACE | ||
$<$<BOOL:${MINGW}>:iphlpapi;ws2_32> | ||
) | ||
add_library(libevent::libevent ALIAS PkgConfig::libevent) | ||
|
||
if(NOT WIN32) | ||
pkg_check_modules(libevent_pthreads REQUIRED libevent_pthreads>=${libevent_minimum_version} IMPORTED_TARGET) | ||
endif() | ||
endfunction() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
# Copyright (c) 2023 The Bitcoin Core developers | ||
# Distributed under the MIT software license, see the accompanying | ||
# file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
||
function(add_threads_if_needed) | ||
# TODO: Not all targets, which will be added in the future, | ||
# require Threads. Therefore, a proper check will be | ||
# appropriate here. | ||
|
||
set(THREADS_PREFER_PTHREAD_FLAG ON) | ||
find_package(Threads REQUIRED) | ||
|
||
set(thread_local) | ||
if(MINGW) | ||
#[=[ | ||
mingw32's implementation of thread_local has been shown to behave | ||
erroneously under concurrent usage. | ||
See: | ||
- https://github.com/bitcoin/bitcoin/pull/15849 | ||
- https://gist.github.com/jamesob/fe9a872051a88b2025b1aa37bfa98605 | ||
]=] | ||
elseif(CMAKE_SYSTEM_NAME STREQUAL "FreeBSD") | ||
#[=[ | ||
FreeBSD's implementation of thread_local is buggy. | ||
See: | ||
- https://github.com/bitcoin/bitcoin/pull/16059 | ||
- https://groups.google.com/d/msg/bsdmailinglist/22ncTZAbDp4/Dii_pII5AwAJ | ||
]=] | ||
elseif(THREADLOCAL) | ||
set(thread_local "$<$<COMPILE_FEATURES:cxx_thread_local>:HAVE_THREAD_LOCAL>") | ||
endif() | ||
set(THREAD_LOCAL_IF_AVAILABLE "${thread_local}" PARENT_SCOPE) | ||
endfunction() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
# Copyright (c) 2023 The Bitcoin Core developers | ||
# Distributed under the MIT software license, see the accompanying | ||
# file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
||
# GCC 8.x (libstdc++) requires -lstdc++fs | ||
# Clang 8.x (libc++) requires -lc++fs | ||
|
||
function(check_std_filesystem) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is very nice 👍 |
||
set(source " | ||
#include <filesystem> | ||
|
||
int main() | ||
{ | ||
(void)std::filesystem::current_path().root_name(); | ||
} | ||
") | ||
|
||
include(CheckCXXSourceCompiles) | ||
check_cxx_source_links("${source}" STD_FILESYSTEM_NO_EXTRA_LIBS_NEEDED) | ||
if(STD_FILESYSTEM_NO_EXTRA_LIBS_NEEDED) | ||
return() | ||
endif() | ||
|
||
add_library(std_filesystem INTERFACE) | ||
check_cxx_source_links_with_libs(stdc++fs "${source}" STD_FILESYSTEM_NEEDS_LINK_TO_LIBSTDCXXFS) | ||
if(STD_FILESYSTEM_NEEDS_LINK_TO_LIBSTDCXXFS) | ||
target_link_libraries(std_filesystem INTERFACE stdc++fs) | ||
return() | ||
endif() | ||
check_cxx_source_links_with_libs(c++fs "${source}" STD_FILESYSTEM_NEEDS_LINK_TO_LIBCXXFS) | ||
if(STD_FILESYSTEM_NEEDS_LINK_TO_LIBCXXFS) | ||
target_link_libraries(std_filesystem INTERFACE c++fs) | ||
return() | ||
endif() | ||
message(FATAL_ERROR "Cannot figure out how to use std::filesystem.") | ||
endfunction() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
# Copyright (c) 2023 The Bitcoin Core developers | ||
# Distributed under the MIT software license, see the accompanying | ||
# file COPYING or http://www.opensource.org/licenses/mit-license.php. | ||
|
||
# This file is part of the transition from Autotools to CMake. Once CMake | ||
# support has been merged we should switch to using the upstream CMake | ||
# buildsystem. | ||
|
||
set(CMAKE_C_STANDARD 90) | ||
set(CMAKE_C_EXTENSIONS OFF) | ||
|
||
include(CheckCSourceCompiles) | ||
check_c_source_compiles(" | ||
#include <stdint.h> | ||
|
||
int main() | ||
{ | ||
uint64_t a = 11, tmp; | ||
__asm__ __volatile__(\"movq $0x100000000,%1; mulq %%rsi\" : \"+a\"(a) : \"S\"(tmp) : \"cc\", \"%rdx\"); | ||
} | ||
" HAVE_64BIT_ASM | ||
) | ||
|
||
add_library(secp256k1 STATIC EXCLUDE_FROM_ALL | ||
hebasto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
${PROJECT_SOURCE_DIR}/src/secp256k1/src/secp256k1.c | ||
${PROJECT_SOURCE_DIR}/src/secp256k1/src/precomputed_ecmult.c | ||
${PROJECT_SOURCE_DIR}/src/secp256k1/src/precomputed_ecmult_gen.c | ||
) | ||
|
||
target_compile_definitions(secp256k1 | ||
PRIVATE | ||
ECMULT_GEN_PREC_BITS=4 | ||
ECMULT_WINDOW_SIZE=15 | ||
ENABLE_MODULE_RECOVERY | ||
ENABLE_MODULE_SCHNORRSIG | ||
ENABLE_MODULE_EXTRAKEYS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logic for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks! Updated. |
||
$<$<BOOL:${HAVE_64BIT_ASM}>:USE_ASM_X86_64=1> | ||
) | ||
|
||
target_include_directories(secp256k1 | ||
PUBLIC | ||
$<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src/secp256k1/include> | ||
) | ||
|
||
target_compile_options(secp256k1 | ||
PRIVATE | ||
$<$<CXX_COMPILER_ID:MSVC>:/wd4146 /wd4334> | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to split this part out as a dependency of the secp256k1 asm detection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm reading this correctly this ASM option has no influence on secp256k1 asm usage and neither does its analog in the current autotools build system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this has always been handled awkwardly, and agreed that behavior doesn't seem to be changing here.