Skip to content

Conversation

@hebasto
Copy link
Owner

@hebasto hebasto commented Dec 8, 2023

The only goal of this PR is to backport changes from bitcoin#28349.

As they include changes to the depends build system, I decided to combine C++20 changes in CMake stuff with the rebasing of the cmake-staging branch on top of the master @ 3e691258d8789a4a89cce42e7e71b130491594d7 (the fixup/autosquash approach doesn't work well as it introduces a number of conflicts).

For me, the git diff cmake-staging..231208-linear -- CMakeLists.txt gives:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -31,11 +31,14 @@ set(COPYRIGHT_HOLDERS "The %s developers")
 set(COPYRIGHT_HOLDERS_FINAL "The ${PROJECT_NAME} developers")
 set(PACKAGE_BUGREPORT "https://github.com/bitcoin/bitcoin/issues")
 
+set(CMAKE_CXX_STANDARD 20)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+set(CMAKE_CXX_EXTENSIONS OFF)
+
 list(APPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/module)
 
 # Configurable options.
 # When adding a new option, end the <help_text> with a full stop for consistency.
-include(CMakeDependentOption)
 option(BUILD_DAEMON "Build bitcoind executable." ON)
 option(BUILD_CLI "Build bitcoin-cli executable." ON)
 option(BUILD_TX "Build bitcoin-tx executable." ON)
@@ -49,9 +52,9 @@ include(TristateOption)
 tristate_option(WITH_SQLITE "Enable SQLite wallet support." "if libsqlite3 is found." AUTO)
 tristate_option(WITH_BDB "Enable Berkeley DB (BDB) wallet support." "if libdb_cxx is found." AUTO)
 option(WARN_INCOMPATIBLE_BDB "Warn when using a Berkeley DB (BDB) version other than 4.8." ON)
+include(CMakeDependentOption)
 cmake_dependent_option(BUILD_WALLET_TOOL "Build bitcoin-wallet tool." ON "ENABLE_WALLET" OFF)
 
-cmake_dependent_option(CXX20 "Enable compilation in C++20 mode." OFF "NOT MSVC" ON)
 option(THREADLOCAL "Enable features that depend on the C++ thread_local keyword (currently just thread names in debug logs)." ON)
 option(HARDENING "Attempt to harden the resulting executables." ON)
 option(REDUCE_EXPORTS "Attempt to reduce exported symbols in the resulting executables." OFF)
@@ -71,14 +74,6 @@ option(BUILD_TESTS "Build test_bitcoin executable." ON)
 option(BUILD_BENCH "Build bench_bitcoin executable." ON)
 option(INSTALL_MAN "Install man pages." ON)
 
-if(CXX20)
-  set(CMAKE_CXX_STANDARD 20)
-else()
-  set(CMAKE_CXX_STANDARD 17)
-endif()
-set(CMAKE_CXX_STANDARD_REQUIRED ON)
-set(CMAKE_CXX_EXTENSIONS OFF)
-
 set(configure_warnings)
 
 include(CheckPIESupported)

Changes from bitcoin#28894 have been backported as well.


What to test?

  1. The minimum supported compilers.
  2. Any fancy build setup mentioned in build: Require C++20 compiler bitcoin/bitcoin#28349.

hebasto and others added 30 commits December 8, 2023 19:50
Co-authored-by: Cory Fields <[email protected]>
Co-authored-by: Vasil Dimov <[email protected]>
To configure CMake for cross-compiling, use
`--toolchain depends/${HOST}/share/toolchain.cmake` command-line option.
@hebasto
Copy link
Owner Author

hebasto commented Dec 10, 2023

Copy link

@TheCharlatan TheCharlatan left a comment

Choose a reason for hiding this comment

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

LGTM ACK

@hebasto
Copy link
Owner Author

hebasto commented Dec 11, 2023

@TheCharlatan

Thank you for the reviewing!


This branch has been force pushed into the https://github.com/hebasto/bitcoin/tree/cmake-staging.

Closing.

@hebasto hebasto closed this Dec 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants