Skip to content
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 (4 of N) #10

Merged
merged 6 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 24 additions & 11 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ project("Bitcoin Core"
VERSION 24.99.0
DESCRIPTION "Bitcoin client software"
HOMEPAGE_URL "https://bitcoincore.org/"
LANGUAGES CXX C ASM
LANGUAGES CXX ASM
)

set(CLIENT_VERSION_IS_RELEASE "false")
Expand Down Expand Up @@ -52,16 +52,6 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS OFF)

set(configure_warnings)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.14)
include(CheckPIESupported)
check_pie_supported(OUTPUT_VARIABLE check_pie_output LANGUAGES CXX)
if(NOT CMAKE_CXX_LINK_PIE_SUPPORTED)
list(APPEND configure_warnings "PIE link options are not supported for executable targets: ${check_pie_output}.")
endif()
else()
list(APPEND configure_warnings "No PIE options will be passed to a linker for executable targets.")
endif()
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

if(WIN32)
#[=[
Expand Down Expand Up @@ -98,6 +88,10 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
add_compile_definitions(MAC_OSX)
endif()

if(CMAKE_CROSSCOMPILING AND DEPENDS_ALLOW_HOST_PACKAGES)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this be done in the toolchain file by conditionally setting the CMAKE_FIND_ROOT_PATH_* vars?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my initial idea as well :)

It turns out that a toolchain file is being processed more than once (the actual number of runs depends on CMake version). Therefore, its processing must be idempotent. list(APPEND ...) command is not idempotent. That is why it has been moved out of the toolchain file.

list(APPEND CMAKE_FIND_ROOT_PATH "${CMAKE_SYSTEM_PREFIX_PATH}")
endif()

include(AddThreadsIfNeeded)
add_threads_if_needed()

Expand All @@ -119,6 +113,19 @@ include(cmake/secp256k1.cmake)
include(CheckStdFilesystem)
check_std_filesystem()

if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.14)
include(CheckPIESupported)
check_pie_supported(OUTPUT_VARIABLE check_pie_output LANGUAGES CXX)
if(CMAKE_CXX_LINK_PIE_SUPPORTED)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
endif()
else()
check_cxx_source_links_with_flags(-fPIE "int main(){}" COMPILER_SUPPORTS_PIE)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing a few things compared to the autotools check:
AX_CHECK_LINK_FLAG([-fPIE -pie], [PIE_FLAGS="-fPIE"; HARDENED_LDFLAGS="$HARDENED_LDFLAGS -pie"], [], [$CXXFLAG_WERROR])

  • This needs -pie as well to check the link option.

  • need to make sure we're checking for warnings, as several platforms spew annoying warnings rather than errors.

if(COMPILER_SUPPORTS_PIE)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
endif()
endif()

add_subdirectory(src)

message("\n")
Expand All @@ -127,6 +134,12 @@ message("=================")
message("Executables:")
message(" bitcoind ............................ ${BUILD_DAEMON}")
message("")
if(CMAKE_CROSSCOMPILING)
set(cross_status "TRUE, for ${CMAKE_SYSTEM_NAME}, ${CMAKE_SYSTEM_PROCESSOR}")
else()
set(cross_status "FALSE")
endif()
message("Cross compiling ....................... ${cross_status}")
get_directory_property(definitions COMPILE_DEFINITIONS)
string(REPLACE ";" " " definitions "${definitions}")
message("Preprocessor defined macros ........... ${definitions}")
Expand Down
6 changes: 3 additions & 3 deletions cmake/module/AddLibeventIfNeeded.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ function(add_libevent_if_needed)
return()
endif()

find_package(PkgConfig)
pkg_check_modules(libevent REQUIRED libevent>=${libevent_minimum_version} IMPORTED_TARGET GLOBAL)
include(CrossPkgConfig)
cross_pkg_check_modules(libevent REQUIRED libevent>=${libevent_minimum_version} IMPORTED_TARGET GLOBAL)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to the changes here, but I found this little diamond in the CMake docs:

The following variables may be set upon return. Two sets of values exist: One for the common case ( = ) and another for the information pkg-config provides when called with the --static option ( = _STATIC).

I mentioned that we should do this here: #7 (comment)

Maybe slot in a switch to _STATIC as a fixup commit?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, ignore this for now as it's more complicated than just switching to _STATIC. That should only be done if we're building from depends.

check_evhttp_connection_get_peer(PkgConfig::libevent)
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)
cross_pkg_check_modules(libevent_pthreads REQUIRED libevent_pthreads>=${libevent_minimum_version} IMPORTED_TARGET)
endif()
endfunction()
9 changes: 9 additions & 0 deletions cmake/module/AddThreadsIfNeeded.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,17 @@ function(add_threads_if_needed)
# require Threads. Therefore, a proper check will be
# appropriate here.

if(CMAKE_C_COMPILER_LOADED)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with the explanation, I still don't understand what's going on here.

What's the overlap between c/c++/mingw/threads?

Is this an assertion to make sure we're never testing with a C compiler, or do we actually hit this?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an assertion to make sure we're never testing with a C compiler..?

Yes, it is. We are testing C++ compiler only.

What's the overlap between c/c++/mingw/threads?

Otherwise errors mentioned by @TheCharlatan happen.

message(FATAL_ERROR [=[
To make FindThreads check C++ language features, C language must be
disabled. This is essential, at least, when cross-compiling for MinGW-w64
because two different threading models are available.
]=] )
endif()

set(THREADS_PREFER_PTHREAD_FLAG ON)
find_package(Threads REQUIRED)
set_target_properties(Threads::Threads PROPERTIES IMPORTED_GLOBAL TRUE)

set(thread_local)
if(MINGW)
Expand Down
19 changes: 19 additions & 0 deletions cmake/module/CrossPkgConfig.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# 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.

find_package(PkgConfig REQUIRED)

macro(cross_pkg_check_modules)
if(CMAKE_CROSSCOMPILING)
set(pkg_config_path_saved "$ENV{PKG_CONFIG_PATH}")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to imply that these hacks shouldn't be needed if CMAKE_PREFIX_PATH is set by the toolchain. Have you experimented with that?

Copy link
Owner Author

@hebasto hebasto Mar 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, this part was a bit problematic when I worked on it back in the day. I remember I did try the approach you mentioned. But cannot point the exact issue I faced.

One of the point of the current implementation is it supports the ALLOW_HOST_PACKAGES variable for depends.

set(pkg_config_libdir_saved "$ENV{PKG_CONFIG_LIBDIR}")
set(ENV{PKG_CONFIG_PATH} ${PKG_CONFIG_PATH})
set(ENV{PKG_CONFIG_LIBDIR} ${PKG_CONFIG_LIBDIR})
pkg_check_modules(${ARGV})
set(ENV{PKG_CONFIG_PATH} ${pkg_config_path_saved})
set(ENV{PKG_CONFIG_LIBDIR} ${pkg_config_libdir_saved})
else()
pkg_check_modules(${ARGV})
endif()
endmacro()
1 change: 1 addition & 0 deletions cmake/secp256k1.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
# support has been merged we should switch to using the upstream CMake
# buildsystem.

enable_language(C)
set(CMAKE_C_STANDARD 90)
set(CMAKE_C_EXTENSIONS OFF)

Expand Down
32 changes: 31 additions & 1 deletion depends/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ $(host_arch)_$(host_os)_native_toolchain?=$($(host_os)_native_toolchain)
include funcs.mk

final_build_id_long+=$(shell $(build_SHA256SUM) config.site.in)
final_build_id_long+=$(shell $(build_SHA256SUM) toolchain.cmake.in)
final_build_id+=$(shell echo -n "$(final_build_id_long)" | $(build_SHA256SUM) | cut -c-$(HASH_LENGTH))
$(host_prefix)/.stamp_$(final_build_id): $(native_packages) $(packages)
rm -rf $(@D)
Expand Down Expand Up @@ -257,6 +258,34 @@ $(host_prefix)/share/config.site : config.site.in $(host_prefix)/.stamp_$(final_
$< > $@
touch $@

$(host_prefix)/share/toolchain.cmake : toolchain.cmake.in $(host_prefix)/.stamp_$(final_build_id)
@mkdir -p $(@D)
sed -e 's|@host_system@|$($(host_os)_cmake_system)|' \
-e 's|@host_arch@|$(host_arch)|' \
-e 's|@CC@|$(host_CC)|' \
-e 's|@CXX@|$(host_CXX)|' \
-e 's|@AR@|$(host_AR)|' \
-e 's|@RANLIB@|$(host_RANLIB)|' \
-e 's|@STRIP@|$(host_STRIP)|' \
-e 's|@OBJCOPY@|$(host_OBJCOPY)|' \
-e 's|@INSTALL_NAME_TOOL@|$(host_INSTALL_NAME_TOOL)|' \
-e 's|@OTOOL@|$(host_OTOOL)|' \
-e 's|@depends_prefix@|$(host_prefix)|' \
-e 's|@CFLAGS@|$(strip $(host_CFLAGS) $(host_$(release_type)_CFLAGS))|' \
-e 's|@CXXFLAGS@|$(strip $(host_CXXFLAGS) $(host_$(release_type)_CXXFLAGS))|' \
-e 's|@CPPFLAGS@|$(strip $(host_CPPFLAGS) $(host_$(release_type)_CPPFLAGS))|' \
-e 's|@allow_host_packages@|$(ALLOW_HOST_PACKAGES)|' \
-e 's|@no_qt@|$(NO_QT)|' \
-e 's|@no_qr@|$(NO_QR)|' \
-e 's|@no_zmq@|$(NO_ZMQ)|' \
-e 's|@no_wallet@|$(NO_WALLET)|' \
-e 's|@no_bdb@|$(NO_BDB)|' \
-e 's|@no_sqlite@|$(NO_SQLITE)|' \
-e 's|@no_upnp@|$(NO_UPNP)|' \
-e 's|@no_natpmp@|$(NO_NATPMP)|' \
-e 's|@no_usdt@|$(NO_USDT)|' \
$< > $@
touch $@

define check_or_remove_cached
mkdir -p $(BASE_CACHE)/$(host)/$(package) && cd $(BASE_CACHE)/$(host)/$(package); \
Expand All @@ -278,6 +307,7 @@ check-sources:
@$(foreach package,$(all_packages),$(call check_or_remove_sources,$(package));)

$(host_prefix)/share/config.site: check-packages
$(host_prefix)/share/toolchain.cmake: check-packages

check-packages: check-sources

Expand All @@ -287,7 +317,7 @@ clean-all: clean
clean:
@rm -rf $(WORK_PATH) $(BASE_CACHE) $(BUILD) *.log

install: check-packages $(host_prefix)/share/config.site
install: check-packages $(host_prefix)/share/config.site $(host_prefix)/share/toolchain.cmake


download-one: check-sources $(all_sources)
Expand Down
137 changes: 137 additions & 0 deletions depends/toolchain.cmake.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# This file is expected to be highly volatile and may still change substantially.

set(CMAKE_SYSTEM_NAME @host_system@)
set(CMAKE_SYSTEM_PROCESSOR @host_arch@)

function(split_compiler_launcher env_compiler launcher compiler)
set(${launcher})
list(GET ${env_compiler} 0 start_token)
if(start_token STREQUAL "env")
set(${compiler})
set(env_arg_parsing TRUE)
foreach(token IN LISTS ${env_compiler})
if(env_arg_parsing)
list(APPEND ${launcher} ${token})
set(env_arg_parsing FALSE)
continue()
elseif(token STREQUAL "-u")
list(APPEND ${launcher} ${token})
set(env_arg_parsing TRUE)
continue()
endif()
list(APPEND ${compiler} ${token})
endforeach()
else()
set(${compiler} ${${env_compiler}})
endif()
set(${launcher} ${${launcher}} PARENT_SCOPE)
set(${compiler} ${${compiler}} PARENT_SCOPE)
endfunction()

if(NOT CMAKE_C_COMPILER)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you give a hint as to what's going on here? I'm a little lost :)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal is to handle every "compiler invocation command", which we have in our build and test environment, properly. Including such cases as follows:

The main part is done in the split_compiler_launcher() function, which splits env -u LIBRARY_PATH clang in env -u LIBRARY_PATH and clang.

Rest of code makes it compatible with the supported range of CMake versions.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this prohibit the use of CMAKE_CXX_COMPILER_LAUNCHER for ccache?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this prohibit the use of CMAKE_CXX_COMPILER_LAUNCHER for ccache?

The future use of CMAKE_{C,CXX}_COMPILER_LAUNCHER with ccache can be peeked in bitcoin@6f48c34.

set(DEPENDS_C_COMPILER_WITH_LAUNCHER @CC@)
split_compiler_launcher(DEPENDS_C_COMPILER_WITH_LAUNCHER CMAKE_C_COMPILER_LAUNCHER CMAKE_C_COMPILER)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21)
set(CMAKE_C_LINKER_LAUNCHER ${CMAKE_C_COMPILER_LAUNCHER})
endif()
if(CMAKE_VERSION VERSION_LESS 3.19)
set(DEPENDS_C_COMPILER_FLAGS ${CMAKE_C_COMPILER})
list(REMOVE_AT DEPENDS_C_COMPILER_FLAGS 0)
string(REPLACE ";" " " DEPENDS_C_COMPILER_FLAGS "${DEPENDS_C_COMPILER_FLAGS}")
list(GET CMAKE_C_COMPILER 0 CMAKE_C_COMPILER)
endif()
endif()
set(CMAKE_C_FLAGS_INIT "${DEPENDS_C_COMPILER_FLAGS} @CPPFLAGS@ @CFLAGS@")

if(NOT CMAKE_CXX_COMPILER)
set(DEPENDS_CXX_COMPILER_WITH_LAUNCHER @CXX@)
split_compiler_launcher(DEPENDS_CXX_COMPILER_WITH_LAUNCHER CMAKE_CXX_COMPILER_LAUNCHER CMAKE_CXX_COMPILER)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21)
set(CMAKE_CXX_LINKER_LAUNCHER ${CMAKE_CXX_COMPILER_LAUNCHER})
endif()
if(CMAKE_VERSION VERSION_LESS 3.19)
set(DEPENDS_CXX_COMPILER_FLAGS ${CMAKE_CXX_COMPILER})
list(REMOVE_AT DEPENDS_CXX_COMPILER_FLAGS 0)
string(REPLACE ";" " " DEPENDS_CXX_COMPILER_FLAGS "${DEPENDS_CXX_COMPILER_FLAGS}")
list(GET CMAKE_CXX_COMPILER 0 CMAKE_CXX_COMPILER)
endif()
endif()
set(CMAKE_CXX_FLAGS_INIT "${DEPENDS_CXX_COMPILER_FLAGS} @CPPFLAGS@ @CXXFLAGS@")

if(NOT CMAKE_OBJCXX_COMPILER)
set(DEPENDS_OBJCXX_COMPILER_WITH_LAUNCHER @CXX@)
split_compiler_launcher(DEPENDS_OBJCXX_COMPILER_WITH_LAUNCHER CMAKE_OBJCXX_COMPILER_LAUNCHER CMAKE_OBJCXX_COMPILER)
if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.21)
set(CMAKE_OBJCXX_LINKER_LAUNCHER ${CMAKE_OBJCXX_COMPILER_LAUNCHER})
endif()
if(CMAKE_VERSION VERSION_LESS 3.19)
set(DEPENDS_OBJCXX_COMPILER_FLAGS ${CMAKE_OBJCXX_COMPILER})
list(REMOVE_AT DEPENDS_OBJCXX_COMPILER_FLAGS 0)
string(REPLACE ";" " " DEPENDS_OBJCXX_COMPILER_FLAGS "${DEPENDS_OBJCXX_COMPILER_FLAGS}")
list(GET CMAKE_OBJCXX_COMPILER 0 CMAKE_OBJCXX_COMPILER)
endif()
endif()
set(CMAKE_OBJCXX_FLAGS_INIT "${DEPENDS_OBJCXX_COMPILER_FLAGS} @CPPFLAGS@ @CXXFLAGS@")

set(CMAKE_AR "@AR@")
set(CMAKE_RANLIB "@RANLIB@")
set(CMAKE_STRIP "@STRIP@")
set(CMAKE_OBJCOPY "@OBJCOPY@")
set(CMAKE_INSTALL_NAME_TOOL "@INSTALL_NAME_TOOL@")
set(OTOOL "@OTOOL@")

set(CMAKE_FIND_ROOT_PATH "@depends_prefix@")
set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY)
set(PKG_CONFIG_PATH "@depends_prefix@/lib/pkgconfig")
if("@allow_host_packages@" STREQUAL "1")
set(DEPENDS_ALLOW_HOST_PACKAGES TRUE)
else()
set(DEPENDS_ALLOW_HOST_PACKAGES FALSE)
set(PKG_CONFIG_LIBDIR "${PKG_CONFIG_PATH}")
endif()
set(QT_TRANSLATIONS_DIR "@depends_prefix@/translations")

if(NOT WITH_GUI AND "@no_qt@" STREQUAL "1")
set(WITH_GUI "no" CACHE STRING "")
endif()

if(NOT WITH_QRENCODE AND "@no_qr@" STREQUAL "1")
set(WITH_QRENCODE OFF CACHE STRING "")
endif()

if(NOT WITH_ZMQ AND "@no_zmq@" STREQUAL "1")
set(WITH_ZMQ OFF CACHE STRING "")
endif()

if(NOT ENABLE_WALLET AND "@no_wallet@" STREQUAL "1")
set(ENABLE_WALLET OFF CACHE BOOL "")
endif()

if(NOT WITH_BDB AND "@no_bdb@" STREQUAL "1")
set(WITH_BDB OFF CACHE STRING "")
endif()

if(NOT WITH_SQLITE AND "@no_sqlite@" STREQUAL "1")
set(WITH_SQLITE OFF CACHE STRING "")
endif()

if(NOT WITH_MINIUPNPC AND "@no_upnp@" STREQUAL "1")
set(WITH_MINIUPNPC OFF CACHE STRING "")
endif()

if(NOT WITH_NATPMP AND "@no_natpmp@" STREQUAL "1")
set(WITH_NATPMP OFF CACHE STRING "")
endif()

if(NOT WITH_USDT AND "@no_usdt@" STREQUAL "1")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the line currently drawn between configuration already supported and configuration to be added later? We support usdt here, but don't support multiprocess, LTO, or some of the qt features.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the line currently drawn between configuration already supported and configuration to be added later?

Well, it is vague, indeed.

We support usdt here, but don't support multiprocess...

See: bitcoin@1238ed5

... LTO...

Not implemented in the parent bitcoin#25797 yet. Sorry about that.

... or some of the qt features.

It is more complicated. For example, see bitcoin@50ac8b0.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the references, I'll be sure to come back to them later.

set(WITH_USDT OFF CACHE STRING "")
endif()

if(DEFINED ENV{PYTHONPATH})
set(PYTHONPATH "@depends_prefix@/native/lib/python3/dist-packages:$ENV{PYTHONPATH}")
else()
set(PYTHONPATH "@depends_prefix@/native/lib/python3/dist-packages")
endif()