build: Introduce CMake-based build system #30454

pull hebasto wants to merge 68 commits into bitcoin:master from hebasto:240716-cmake changing 113 files +5872 −518
  1. hebasto commented at 2:55 pm on July 16, 2024: member

    This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.

    ML announcement: https://groups.google.com/g/bitcoindev/c/hgKkfQWzrTo

    As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.

    This PR branch is essentially the staging branch, with every change reviewed and tested by a group of contributors, including (in alphabetical order):

    Reviewing in a separate staging repo was suggested in #27060 (comment).

    The accompanying changes to the OSS-Fuzz project are available in https://github.com/hebasto/oss-fuzz/pull/8.

    Please refer to the build options parity table. The “auto” value is no longer available; non-default values must be specified explicitly. Additionally, the new default values have been chosen to suit the everyday build experience for the majority of developers.

    System requirements for using the CMake-based build system:

    • CMake >= 3.22 (if not available in your system’s repository, it can be downloaded from https://cmake.org/download/)
    • a build tool of your choice:
      • any Make (GNU Make is no longer a requirement); GNU Make is still required to build depends
      • Ninja (https://ninja-build.org/)
      • MSBuild
      • Xcode

    A note for Windows users: The default installation of the latest version of MSVC 17.10.4 includes both CMake 3.28.3 and the vcpkg package manager).


    We, the build system developers, kindly ask reviewers to refrain from making suggestions that are not directly related to the migration process or can be implemented separately. Bugs in the scripts and errors in the updated documentation should be the focus of this PR. Please be advised that comments not aligned with this PR’s goal may be ignored.

    Thank you all for your understanding.

  2. DrahtBot commented at 2:55 pm on July 16, 2024: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK vasild, TheCharlatan, maflcko, sipsorcery, pablomartin4btc, i-am-yuvi, theuni, fanquake
    Concept ACK Sjors
    Stale ACK whitslack

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #30717 (rpc: Add test-only RPCs under -test=<option> flag by Prabhat1308)
    • #30685 (build: Mark x86_64-linux-gnu release binaries as CET-enabled by hebasto)
    • #30634 (ci: Use clang-19 from apt.llvm.org by maflcko)
    • #30619 (doc: Change nproc in docs to getconf _NPROCESSORS_ONLN by l0rinc)
    • #30527 (Bump python minimum supported version to 3.10 by maflcko)
    • #30465 (depends: Set CMAKE_SYSTEM_VERSION for CMake builds by hebasto)
    • #30043 (net: Replace libnatpmp with built-in PCP+NATPMP implementation by laanwj)
    • #29881 (guix: use GCC 13 to builds releases by fanquake)
    • #29868 (Reintroduce external signer support for Windows by hebasto)
    • #25573 ([POC] guix: produce a fully -static-pie bitcoind by fanquake)
    • #19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
    • #19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
    • #10102 (Multiprocess bitcoin by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. DrahtBot added the label Build system on Jul 16, 2024
  4. maflcko added the label Needs CMake port on Jul 16, 2024
  5. maflcko removed the label Needs CMake port on Jul 16, 2024
  6. maflcko commented at 3:08 pm on July 16, 2024: member

    The only differences from the staging branch are:

    I’d say the section can be removed (or moved to a separate comment), because when this will be merged, I presume many more differences will accumulate. Even looking at the outstanding ports (https://github.com/bitcoin/bitcoin/issues?q=label%3A%22Needs+CMake+port%22+is%3Aclosed) right now, there are some.

  7. maflcko commented at 3:18 pm on July 16, 2024: member

    Please refer to the build options parity table.

    Missing link/reference?

  8. hebasto commented at 3:23 pm on July 16, 2024: member

    Please refer to the build options parity table.

    Missing link/reference?

    Thanks! Added.

  9. in src/crypto/sha256.cpp:5 in ff368465a6 outdated
    1@@ -2,8 +2,6 @@
    2 // Distributed under the MIT software license, see the accompanying
    3 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
    4 
    5-#include <config/bitcoin-config.h> // IWYU pragma: keep
    


    theuni commented at 3:52 pm on July 16, 2024:

    In commit ff368465a66298487a88d8da575f5251d094d687

    Could you remind me what’s going on here, please? I assume these aren’t needed because we’re meant to be adding compile definitions to specific libs. But from what I can tell, we’re not actually adding them to sha256.cpp? So… is this currently using an un-optimized sha2?

    Generally I’d feel more comfortable turning these into opt-outs, I think, to avoid accidentally building without optims.

    For the sake of review/sanity, I think for this commit it’s worth documenting (in the commit message) exactly which defines are affected and where in CMake they’ve been added.


    hebasto commented at 4:14 pm on July 16, 2024:

    But from what I can tell, we’re not actually adding them to sha256.cpp? So… is this currently using an un-optimized sha2?

    Please note the PUBLIC keyword, which propagates the definition as a usage requirement: https://github.com/bitcoin/bitcoin/blob/2b7c0f4fdb81ec1cf0579fa71b37f56672934ab5/src/crypto/CMakeLists.txt#L125

    and

    https://github.com/bitcoin/bitcoin/blob/2b7c0f4fdb81ec1cf0579fa71b37f56672934ab5/src/crypto/CMakeLists.txt#L128

    where that usage requirement is actually applied.

    The same approach works for other definitions used in src/crypto/sha256.cpp.

    For the sake of review/sanity, I think for this commit it’s worth documenting (in the commit message) exactly which defines are affected and where in CMake they’ve been added.

    Sure thing! I’ll update the commit message during the next branch update.


    maflcko commented at 6:10 pm on July 16, 2024:

    One can see that they are indeed disabled now in https://corecheck.dev/bitcoin/bitcoin/pulls/30454 (coverage report + benchmarks).

    Shouldn’t autotools be nuked in this pull request, assuming that cmake and autotools are apparently incompatible, as designed now?


    hebasto commented at 6:06 pm on July 18, 2024:

    hebasto commented at 2:58 pm on July 24, 2024:

    @theuni @maflcko

    Can you confirm please that the recent push resolved this issue?


    maflcko commented at 7:17 am on July 25, 2024:

    hebasto commented at 4:14 pm on July 25, 2024:

    Hmm https://corecheck.dev/bitcoin/bitcoin/pulls/30454 is down again

    It is up now :)


    maflcko commented at 3:02 pm on July 31, 2024:

    Hmm https://corecheck.dev/bitcoin/bitcoin/pulls/30454 is down again

    It is up now :)

    Nice. The output there looks better now. But I haven’t yet tried locally.

  10. in ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh:11 in 2b7c0f4fdb outdated
     6@@ -7,9 +7,9 @@
     7 export LC_ALL=C.UTF-8
     8 
     9 export CONTAINER_NAME=ci_native_nowallet_libbitcoinkernel
    10-export CI_IMAGE_NAME_TAG="docker.io/debian:bullseye"
    11-# Use minimum supported python3.9 and clang-16, see doc/dependencies.md
    12+export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04"
    13+# Use minimum supported python3.9 (or best-effort 3.12) and clang-16, see doc/dependencies.md
    


    maflcko commented at 4:21 pm on July 16, 2024:

    hebasto commented at 7:59 pm on July 18, 2024:
  11. in src/crypto/CMakeLists.txt:50 in 2b7c0f4fdb outdated
    45+    }
    46+    " HAVE_X86_SHANI
    47+  )
    48+
    49+  # Check for ARMv8 SHA-NI intrinsics.
    50+  set(ARM_SHANI_CXXFLAGS -march=armv8-a+crypto)
    


    fanquake commented at 4:22 pm on July 16, 2024:
    Looks like these don’t allow overriding/take into account user flags? i.e If I configure with -DCMAKE_CXX_FLAGS_RELEASE="-march=armv8-a+nocrypto", I’d expect the ARM SHA-NI check to fail, but it doesn’t. Also doesn’t seem to work with APPEND_CXXFLAGS, which should always be (globally) taken into account?

    hebasto commented at 9:39 pm on July 23, 2024:
    Cross-posted in https://github.com/hebasto/bitcoin/issues/279 to make tracking easier.

    hebasto commented at 1:22 pm on July 31, 2024:
    With the recent push, one can configure for such a use case with -DAPPEND_CXXFLAGS="-march=armv8-a+nocrypto".
  12. hebasto commented at 8:16 pm on July 16, 2024: member
    A couple of functional tests-related commits have been split to #30463, as suggested by @fanquake offline.
  13. maflcko commented at 8:32 am on July 17, 2024: member

    which means that 29.0 will be built using CMake.

    Could add the 29.0 milestone?

  14. in ci/test/00_setup_env_native_fuzz_with_msan.sh:22 in 2b7c0f4fdb outdated
    17@@ -18,7 +18,8 @@ export PACKAGES="ninja-build"
    18 export DEP_OPTS="DEBUG=1 NO_BDB=1 NO_QT=1 CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
    19 export GOAL="install"
    20 # _FORTIFY_SOURCE is not compatible with MSAN.
    21-export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE -U_FORTIFY_SOURCE'"
    22+export BITCOIN_CONFIG="-DENABLE_FUZZ=ON -DSANITIZERS=fuzzer,memory -DCMAKE_CXX_FLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE' \
    23+-DAPPEND_CPPFLAGS='-U_FORTIFY_SOURCE'"
    


    fanquake commented at 8:39 am on July 17, 2024:
    CPP flag handling seems inconsistent throughout the CI changes, which is confusing. It’d be good to consistently put all CPPFLAGS into CPPFLAGS, rather than having them sometimes in CXXFLAGS, sometimes in (append) CPPFLAGS, and sometimes spread between the two, i.e here.

    hebasto commented at 7:05 am on July 20, 2024:
  15. hebasto added this to the milestone 29.0 on Jul 17, 2024
  16. hebasto commented at 8:48 am on July 17, 2024: member

    which means that 29.0 will be built using CMake.

    Could add the 29.0 milestone?

    Done.

  17. fanquake referenced this in commit 9c8b36eba6 on Jul 18, 2024
  18. DrahtBot added the label Needs rebase on Jul 18, 2024
  19. theuni commented at 3:38 pm on July 18, 2024: member

    Our plan for the next few weeks:

    This PR has already pointed out some very helpful high-level issues.

    During today’s working-group call, we agreed on the following plan:

    Because we don’t want to create too much noise (yet) in this PR, we’re going to maintain the staging workflow for another ~two weeks. This means discussing and merging into @hebasto’s repo, then occasionally syncing with the upstream PR. More specifically:

    Goals for week 1:

    • Upstream (and hopefully get merged) all outstanding issues/prerequisites pointed out by review so far (#30464, #30463, #30477, others?)
    • Ensure that the staging branch can be built successfully with autotools and cmake by reverting some CMake-specific changes that aren’t strictly necessary for the port
    • Try to make the staging branch contain no actual code/test changes (qt plugins as the exception), only build-system changes.
    • Sync the upstream PR with staging when the above is complete
    • Announce at next week’s IRC meeting that this PR is ready for review/scrutiny

    Goals for week 2:

    At this point, reviewers may still point out more chunks that can be merged ahead-of-time, low-level complaints that require build-specific discussion, differences with autotools, and other boring details that aren’t worth polluting the main PR. During week two we’ll address this feedback on hebasto’s staging repo to keep the discussion/PR noise out of the mainline PR. Hopefully this will be minor stuff. At the end of week two, we’ll aim to sync the staging repo with this PR for the last time, EOL the staging repo, and have all remaining discussion/arguing here from that point on.

    Plan for removing autotools:

    Because it’s useful to test Autotools/CMake on the same commit, we’ve decided not to remove Autotools as part of the CMake PR. Additionally, it will be useful for posterity to have a point in time (even if only a single commit) where both were present, so that it’s obvious what before/after behavior was intended.

    However, as maintaining both systems is not a goal, we intend to remove Autotools directly after CMake’s merge. There should be no expectation of it hanging around for more than a day or two.

  20. fanquake marked this as a draft on Jul 18, 2024
  21. hebasto referenced this in commit 945d5c6f24 on Jul 18, 2024
  22. in depends/packages/native_libmultiprocess.mk:2 in 4883197abc outdated
    0@@ -1,8 +1,8 @@
    1 package=native_libmultiprocess
    2-$(package)_version=8da797c5f1644df1bffd84d10c1ae9836dc70d60
    3+$(package)_version=003eb04d6d0029fd24a330ab63d5a9ba08cf240f
    


    theuni commented at 6:26 pm on July 19, 2024:
    Looks like this can be bumped further now that https://github.com/chaincodelabs/libmultiprocess/pull/98 has been merged?

    hebasto commented at 9:31 pm on July 23, 2024:
    Bumped even further in #30513 :)
  23. in src/test/compilerbug_tests.cpp:15 in 2b7c0f4fdb outdated
    10+// cases matching filter or all test cases were disabled" errror.
    11+BOOST_AUTO_TEST_CASE(dummy)
    12+{
    13+    BOOST_CHECK(true);
    14+}
    15+
    


    hebasto commented at 0:02 am on July 20, 2024:
    This diff will gone after https://github.com/hebasto/bitcoin/pull/272.
  24. hebasto renamed this:
    build: Introduce CMake-based buid system
    build: Introduce CMake-based build system
    on Jul 20, 2024
  25. fanquake referenced this in commit efeb39785a on Jul 20, 2024
  26. fanquake referenced this in commit 8d57361157 on Jul 20, 2024
  27. glozow referenced this in commit 3a29ff5dea on Jul 22, 2024
  28. hebasto referenced this in commit ad996126d3 on Jul 22, 2024
  29. fanquake referenced this in commit b927a39c63 on Jul 22, 2024
  30. hebasto referenced this in commit 77da79f7d2 on Jul 23, 2024
  31. hebasto referenced this in commit d3a10e6aed on Jul 23, 2024
  32. hebasto referenced this in commit 7a59ed6910 on Jul 24, 2024
  33. hebasto referenced this in commit 6d23915521 on Jul 24, 2024
  34. hebasto force-pushed on Jul 24, 2024
  35. DrahtBot removed the label Needs rebase on Jul 24, 2024
  36. hebasto commented at 2:56 pm on July 24, 2024: member

    Rebased. This PR branch is the current cmake-staging branch with 2 top commits dropped.

    Also some comments have been addressed.

    Tasks from #30454 (comment):

    • Ensure that the staging branch can be built successfully with autotools and cmake by reverting some CMake-specific changes that aren’t strictly necessary for the port
    • Try to make the staging branch contain no actual code/test changes (qt plugins as the exception), only build-system changes
    • Sync the upstream PR with staging when the above is complete
  37. in ci/test/00_setup_env_native_asan.sh:22 in dcd411eba7 outdated
    18@@ -19,11 +19,16 @@ else
    19 fi
    20 
    21 export CONTAINER_NAME=ci_native_asan
    22-export PACKAGES="systemtap-sdt-dev clang-18 llvm-18 libclang-rt-18-dev python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libqrencode-dev libsqlite3-dev ${BPFCC_PACKAGE}"
    23+export PACKAGES="systemtap-sdt-dev clang-18 llvm-18 libclang-rt-18-dev python3-zmq qtbase5-dev qttools5-dev qttools5-dev-tools libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libqrencode-dev libsqlite3-dev ${BPFCC_PACKAGE}"
    


    maflcko commented at 7:34 am on July 25, 2024:
    note to myself: Could split this up

    maflcko commented at 9:32 am on July 25, 2024:
  38. in depends/README.md:57 in dcd411eba7 outdated
    53@@ -58,7 +54,7 @@ For more information, see [SDK Extraction](../contrib/macdeploy/README.md#sdk-ex
    54 
    55 #### For Win64 cross compilation
    56 
    57-- see [build-windows.md](../doc/build-windows.md#cross-compilation-for-ubuntu-and-windows-subsystem-for-linux)
    58+- see [build-windows-mingw.md](../doc/build-windows-mingw.md#cross-compilation-for-ubuntu-and-windows-subsystem-for-linux)
    


    maflcko commented at 7:42 am on July 25, 2024:
    The rename could be split out and merged ahead?

    hebasto commented at 9:58 am on July 25, 2024:

    hebasto commented at 1:25 pm on July 31, 2024:
    It has not been an issue since the recent push.
  39. DrahtBot added the label Needs rebase on Jul 25, 2024
  40. rot13maxi commented at 3:55 pm on July 25, 2024: none
    Loaded this branch into CLion Nova (the latest C/C++ focused jetbrains IDE) and it was able to correctly load the project, and set up a Debug profile for 63 build targets with no additional configuration. Build and debugging seems to work out of the box (tested for bitcoin-cli and bitcoind). This is awesome!
  41. sipa commented at 5:18 pm on July 25, 2024: member

    Trying to build the benchmarks:

    • Is it expected that the default for BUILD_BENCH is OFF? (the default with autoconf is on)
    • If a specify something weird like cmake -B build -DBUILD_BENCH=ab123, I get as configure output bench_bitcoin ....................... ab123 (and it builds the bench binary). I would expect an error when specifying anything but OFF or ON or equivalent.
  42. maflcko commented at 5:28 pm on July 25, 2024: member

    Is it expected that the default for BUILD_BENCH is OFF? (the default with autoconf is on)

    Yes, according to https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e (the parity table, linked in OP). I presume developer (not users) are encouraged to set -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_GUI=ON on all of their builds, if they want to catch compile errors before opening a pull request.

  43. hebasto commented at 6:47 pm on July 25, 2024: member

    Trying to build the benchmarks:

    • Is it expected that the default for BUILD_BENCH is OFF? (the default with autoconf is on)

    Regarding the part about “the default with autoconf is on” part, in the CMake working group, we decided to enable by default only the “essential” build targets that most developers build most often in their everyday work. This means, for instance, that -DBUILD_BENCH and -DBUILD_GUI are OFF by default.

    • If a specify something weird like cmake -B build -DBUILD_BENCH=ab123, I get as configure output bench_bitcoin ....................... ab123 (and it builds the bench binary). I would expect an error when specifying anything but OFF or ON or equivalent.

    The current implementation uses the standard CMake’s boolean options without any extra checks. Provided constants are evaluated according to the CMake rules. Usually, boolean values are represented as ON/OFF, TRUE/FALSE, YES/NO, 1/0.

    I presume developer (not users) are encouraged to set -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_UTIL_CHAINSTATE=ON -DBUILD_GUI=ON on all of their builds, if they want to catch compile errors before opening a pull request.

    User presets are most suitable and convenient for that.

  44. sipa commented at 7:19 pm on July 25, 2024: member

    Yes, according to https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e (the parity table, linked in OP).

    Right, I saw that. I should have phrased my question as “is it intentional that the default changed”.

    Regarding the part about “the default with autoconf is on” part, in the CMake working group, we decided to enable by default only the “essential” build targets that most developers build most often in their everyday work.

    Thanks, that answers my question.

  45. furszy commented at 2:30 pm on July 26, 2024: member
    It seems that the bitcoin-qt qrencode dependency default value deviates from the one in autotools. I don’t have the qrencode library available in my system and I’m able to compile the gui out of the box through autotools but not through cmake due to the missing dependency. It would be nice to use the same default value for now. And discuss any possible default value change (if needed) in a follow-up.
  46. hebasto force-pushed on Jul 31, 2024
  47. hebasto commented at 1:14 pm on July 31, 2024: member

    Rebased due to the conflicts. This PR branch is the current staging branch with the top 3 commits dropped.

    Also some feedback has been addressed.

  48. in depends/README.md:95 in 80073c75d1 outdated
    91+    pkgin install bash gmake
    92+
    93 ### Install the required dependencies: OpenBSD
    94 
    95-    pkg_add bash gtar
    96+    pkg_add bash gmake gtar
    


    maflcko commented at 1:34 pm on July 31, 2024:
    nit: Not sure about listing the required deps from the native guide again. Maybe native depends install instructions can be moved to native guide (./doc/build-openbsd.md)?

    hebasto commented at 2:16 pm on July 31, 2024:
    None of these package are required for the native build with CMake.

    maflcko commented at 2:30 pm on July 31, 2024:

    None of these package are required for the native build with CMake.

    Correct. I was mostly trying to say that packages required for a native build are not listed here again as a dependency for a native (non-cross) depends build. Otherwise, autoconf automake should be listed here as well, because they are still required for some depends packages? In any case you are removing autoconf automake and gmake from the instructions in doc/build-openbsd.md, so overall it seems a bit inconsistent?


    hebasto commented at 2:36 pm on July 31, 2024:

    Otherwise, autoconf automake should be listed here as well, because they are still required for some depends packages? In any case you are removing autoconf automake and gmake from the instructions in doc/build-openbsd.md, so overall it seems a bit inconsistent?

    I agree that this issue has to be resolved in depends/README.md.


    hebasto commented at 0:13 am on August 1, 2024:

    Otherwise, autoconf automake should be listed here as well, because they are still required for some depends packages?

    Apparently, they are not. Thanks to the recent migration of dependency packages to CMake.

    cc @theStack


    fanquake commented at 7:00 am on August 1, 2024:

    Apparently, they are not.

    SQLite still uses Autotools?


    maflcko commented at 7:15 am on August 1, 2024:

    Ok, I see. I just completed on current master a depends on Linux build without any of autoconf automake autotools-dev libtool. Possibly they could be removed from the Common section above, but that seems unrelated to this pull request.

    Just to clarify, removing gmake from the instructions in doc/build-openbsd.md is also intentional in this pull request?


    hebasto commented at 9:52 am on August 1, 2024:

    Just to clarify, removing gmake from the instructions in doc/build-openbsd.md is also intentional in this pull request?

    Yes, it is. CMake can work with any flavour of Make, so GNU Make is not longer a requirement.


    hebasto commented at 10:08 am on August 1, 2024:

    Apparently, they are not.

    SQLite still uses Autotools?

    The sqlite-autoconf-3380500.tar.gz archive contains pre-built configure script and Makefile.in file. Therefore, Autotools is not required on our side.


    fanquake commented at 10:19 am on August 1, 2024:

    Autotools might not be used currently for sqlite (libtool still is), but we can’t rule out entirely that it wont be used going forward. For example, if we need to regenerate the configure script for some reason. This has happened in the past, for example, something like 6897c4bdf51a4aa74320ebfffa9467db14670109. This might be very unlikely here, but not impossible.

    Note that this would also breakdown, if we were to shift to building from source (requiring us to generate the build scripts oursevles), rather than using the tarballs with everything pre-generated (and unverifiable/opauge). (In this case it’d also be all of the Linux GUI packages that’d require it).


    hebasto commented at 10:26 am on August 1, 2024:
    Right. Such a possibility does exist. However, instructing the user to install packages that are not currently required does not seem compelling to me.

    hebasto commented at 10:48 am on August 1, 2024:

    Autotools might not be used currently for sqlite (libtool still is)…

    The tarball also contains a pre-built ltmain.sh script, so libtool package installation is not needed.


    maflcko commented at 10:53 am on August 1, 2024:

    Just to clarify, removing gmake from the instructions in doc/build-openbsd.md is also intentional in this pull request?

    Yes, it is. CMake can work with any flavour of Make, so GNU Make is not longer a requirement.

    But the document still refers to gmake, no? For example line 47 of doc/build-openbsd.md says:

    0gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_NATPMP=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1
    

    Maybe gmake is installed as part of cmake, but I haven’t checked that.

    Just a nit in any case.


    hebasto commented at 11:10 am on August 1, 2024:
    This PR does not change how packages are built in depends. Therefore, GNU Make is still a requirement to build depends. But it is not for the main build system.

    hebasto commented at 11:19 am on August 1, 2024:

    But the document still refers to gmake, no? For example line 47 of doc/build-openbsd.md says:

    0gmake -C depends NO_BOOST=1 NO_LIBEVENT=1 NO_QT=1 NO_SQLITE=1 NO_NATPMP=1 NO_UPNP=1 NO_ZMQ=1 NO_USDT=1
    

    The line above refers to depends/README.md, where the required build dependencies, including gmake, are suggested to be installed.


    hebasto commented at 2:13 pm on August 1, 2024:
    Resolving this conversation based “thumbs up” emoji.
  49. in doc/README.md:44 in 80073c75d1 outdated
    40@@ -41,7 +41,7 @@ The following are developer notes on how to build Bitcoin Core on your native pl
    41 - [Dependencies](dependencies.md)
    42 - [macOS Build Notes](build-osx.md)
    43 - [Unix Build Notes](build-unix.md)
    44-- [Windows Build Notes](build-windows.md)
    


    maflcko commented at 1:34 pm on July 31, 2024:
    why remove the link?

    hebasto commented at 2:00 pm on July 31, 2024:
    This list description mentions “on your native platform”. Does it mean “to build natively”? If so, then build-windows.md doesn’t belong to this list. If I misunderstood the description, let me know.
  50. maflcko commented at 1:37 pm on July 31, 2024: member
    left two nits. Feel free to ignore
  51. in doc/build-windows.md:63 in 80073c75d1 outdated
    59@@ -60,7 +60,7 @@ example /usr/src/bitcoin, AND not under /mnt/d/. If this is not the case the dep
    60 This means you cannot use a directory that is located directly on the host Windows file system to perform the build.
    61 
    62 Additional WSL Note: WSL support for [launching Win32 applications](https://learn.microsoft.com/en-us/archive/blogs/wsl/windows-and-ubuntu-interoperability#launching-win32-applications-from-within-wsl)
    63-results in `Autoconf` configure scripts being able to execute Windows Portable Executable files. This can cause
    64+results in configure scripts being able to execute Windows Portable Executable files. This can cause
    


    fanquake commented at 2:15 pm on July 31, 2024:
    Was this removed because this is an issue for CMake as well?

    hebasto commented at 9:56 pm on August 2, 2024:
    The documentation has been amended in https://github.com/hebasto/bitcoin/pull/299.

    hebasto commented at 7:22 pm on August 6, 2024:
    Addressed in #30454 (comment).
  52. DrahtBot removed the label Needs rebase on Jul 31, 2024
  53. fanquake referenced this in commit 357f195391 on Aug 2, 2024
  54. DrahtBot added the label Needs rebase on Aug 2, 2024
  55. maflcko added the label DrahtBot Guix build requested on Aug 2, 2024
  56. hebasto referenced this in commit a68701662e on Aug 4, 2024
  57. l0rinc commented at 10:35 am on August 4, 2024: contributor

    I can barely contain my excitement about this change, finally the whole project came alive for me in CLion - a few of the features that are working with cmake that didn’t work before:

    • right click to run boost test + debug:

    • built-in code coverage (and partially the profiler) is working:

    • extract method and field rename refactoring (still extremely slow, but at least it’s working - will see if it’s faster after it finishes indexing):


    Edit: I checked the speed of compilation before & after with:

     0build_and_run() {
     1  echo "Building commit: $COMMIT with parallelism: $PARALLEL"
     2  if [ -f CMakeLists.txt ]; then
     3    cmake -B build -DWITH_CCACHE=OFF && cmake --build build -j$PARALLEL
     4    BITCOIND="./build/src/bitcoind"
     5  else
     6    ./autogen.sh && ./configure --disable-ccache && make -j$PARALLEL
     7    BITCOIND="./src/bitcoind"
     8  fi
     9  $BITCOIND --version | head -n 1
    10}
    11export -f build_and_run
    12
    13hyperfine \
    14  --warmup 1 --runs 5 \
    15  --shell=bash \
    16  --parameter-list COMMIT 2ed820,80073c \
    17  --parameter-list PARALLEL $(nproc),1 \
    18  --prepare 'git checkout {COMMIT} && git clean -fxd && git reset --hard' \
    19  --export-json ~/final_results.json \
    20  --style full \
    21  'COMMIT={COMMIT} PARALLEL={PARALLEL} build_and_run'
    

    which resulted in

     0Benchmark 1: COMMIT=2ed820 PARALLEL=8 build_and_run
     1  Time (mean ± σ):     678.382 s ±  1.707 s    [User: 4860.119 s, System: 286.087 s]
     2  Range (min  max):   675.542 s  680.083 s    5 runs
     3
     4Benchmark 2: COMMIT=80073c PARALLEL=8 build_and_run
     5  Time (mean ± σ):     458.423 s ±  0.498 s    [User: 3043.589 s, System: 172.502 s]
     6  Range (min  max):   457.940 s  458.967 s    5 runs
     7
     8Benchmark 3: COMMIT=2ed820 PARALLEL=1 build_and_run
     9  Time (mean ± σ):     2815.758 s ±  1.076 s    [User: 2627.491 s, System: 187.484 s]
    10  Range (min  max):   2814.468 s  2816.904 s    5 runs
    11
    12Benchmark 4: COMMIT=80073c PARALLEL=1 build_and_run
    13  Time (mean ± σ):     1791.808 s ±  1.458 s    [User: 1675.999 s, System: 115.596 s]
    14  Range (min  max):   1790.071 s  1793.572 s    5 runs
    

    Speedup with 8x parallelism: 1.48 times faster Speedup with 1x parallelism: 1.57 times faster

    While this doesn’t fully reflect the cached states (which I’m not sure how to test), I think this is really impressive change.


    Edit2: CLion generated cmake-build-debug and cmake-build-debug-coverage to separate the builds corresponding to different run configs, could we add /cmake-*/ to .gitignore?

  58. in depends/toolchain.cmake.in:82 in 80073c75d1 outdated
    109+set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)
    110+set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)
    111+set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY)
    112+set(QT_TRANSLATIONS_DIR "@depends_prefix@/translations")
    113+
    114+if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND NOT CMAKE_HOST_APPLE)
    


    l0rinc commented at 8:07 am on August 5, 2024:

    Is the intent to handle any cross-compilation to Apple platforms from non-Apple hosts, or specifically cross-compilation to macOS? Is this the same as

    0CMAKE_CROSSCOMPILING AND NOT CMAKE_HOST_APPLE
    

    ?


    hebasto commented at 9:51 am on August 5, 2024:

    Is the intent to handle any cross-compilation to Apple platforms from non-Apple hosts, or specifically cross-compilation to macOS?

    The former.

    Is this the same as

    0CMAKE_CROSSCOMPILING AND NOT CMAKE_HOST_APPLE
    

    ?

    In the context of a toolchain file, it is not. The CMAKE_CROSSCOMPILING variable is well-defined after a project() call, but not within a toolchain file.


    l0rinc commented at 10:54 am on August 5, 2024:
    Thanks
  59. in cmake/module/GenerateHeaders.cmake:9 in 80073c75d1 outdated
    0@@ -0,0 +1,21 @@
    1+# Copyright (c) 2023-present The Bitcoin Core developers
    2+# Distributed under the MIT software license, see the accompanying
    3+# file COPYING or https://opensource.org/license/mit/.
    4+
    5+function(generate_header_from_json json_source_relpath)
    6+  add_custom_command(
    7+    OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${json_source_relpath}.h
    8+    COMMAND ${CMAKE_COMMAND} -DJSON_SOURCE_PATH=${CMAKE_CURRENT_SOURCE_DIR}/${json_source_relpath} -DHEADER_PATH=${CMAKE_CURRENT_BINARY_DIR}/${json_source_relpath}.h -P ${CMAKE_SOURCE_DIR}/cmake/script/GenerateHeaderFromJson.cmake
    9+    DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${json_source_relpath} ${CMAKE_SOURCE_DIR}/cmake/script/GenerateHeaderFromJson.cmake
    


    l0rinc commented at 8:17 am on August 5, 2024:
    When is CMAKE_SOURCE_DIR, PROJECT_SOURCE_DIR and CMAKE_CURRENT_SOURCE_DIR used here? I’m not an expert in this area, but browsing a bit reveals that others were often bitten by the differences among these. Ignore, if the question doesn’t make sense in this case…

    hebasto commented at 10:12 am on August 5, 2024:
    Addressed in https://github.com/hebasto/bitcoin/pull/301. Your review is welcome :)

    l0rinc commented at 10:57 am on August 5, 2024:
    Thanks, ACK f9b48505d6beb2c4e388e275d89bad77a14de2bb

    hebasto commented at 7:21 pm on August 6, 2024:
    Addressed in #30454 (comment).
  60. in cmake/module/FlagsSummary.cmake:13 in 80073c75d1 outdated
     8+  if(indent_num GREATER 0)
     9+    string(REPEAT " " ${indent_num} indentation)
    10+    string(REPEAT "." ${indent_num} tail)
    11+    string(REGEX REPLACE "${tail}$" "" header "${header}")
    12+  endif()
    13+  message("${indentation}${header} ${content}")
    


    l0rinc commented at 8:24 am on August 5, 2024:
    I noticed that we’re not using the message(STATUS construct in this file, was that deliberate?

    hebasto commented at 9:44 am on August 5, 2024:

    From the message command docs:

    The CMake command-line tool displays STATUS … messages on stdout with the message preceded by two hyphens and a space.

    I’m afraid that two hyphens in every line could clutter the summary.

  61. in cmake/module/FlagsSummary.cmake:1 in 80073c75d1 outdated
    0@@ -0,0 +1,73 @@
    1+# Copyright (c) 2024-present The Bitcoin Core developers
    


    l0rinc commented at 8:25 am on August 5, 2024:
    nit: here we have the current year, would it make sense to change all of them to 2024 as a slightly better documentation of historical changes?

    hebasto commented at 9:37 am on August 5, 2024:
    If you mean the “2023” instances in the copyright headers, they reflect the actual time when the code was written.
  62. l0rinc commented at 8:34 am on August 5, 2024: contributor
    Left a few question - ignore if they don’t make sense in this context, it’s not my area of expertise
  63. in CMakeLists.txt:587 in 80073c75d1 outdated
    582+  endif()
    583+endif()
    584+
    585+if(REDUCE_EXPORTS)
    586+  set(CMAKE_CXX_VISIBILITY_PRESET hidden)
    587+  set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)
    


    fanquake commented at 10:25 am on August 5, 2024:

    set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)

    Why is this (-fvisibility-inlines-hidden) being added, if we don’t use it in master?


    hebasto commented at 10:45 am on August 5, 2024:

    theuni commented at 4:02 pm on August 5, 2024:
    Ah, I should’ve caught this in review. I wonder why we don’t use this in master? IIRC there was some issue with boost visibility years ago that wouldn’t be relevant anymore. Wonder if that was it?

    hebasto commented at 4:04 pm on August 5, 2024:

    Wonder if that was it?

    I don’t remember.

  64. hebasto commented at 10:37 am on August 5, 2024: member

    @paplorinc

    Edit2: CLion generated cmake-build-debug and cmake-build-debug-coverage to separate the builds corresponding to different run configs, could we add /cmake-*/ to .gitignore?

    Addressed in https://github.com/hebasto/bitcoin/pull/302.

  65. hebasto referenced this in commit 874e21ea1a on Aug 5, 2024
  66. hebasto referenced this in commit 2feccd4d96 on Aug 5, 2024
  67. in cmake/introspection.cmake:172 in 80073c75d1 outdated
    167+)
    168+
    169+check_cxx_source_compiles("
    170+  int foo(void) __attribute__((visibility(\"default\")));
    171+  int main(){}
    172+  " HAVE_DEFAULT_VISIBILITY_ATTRIBUTE
    


    fanquake commented at 2:09 pm on August 5, 2024:
    Shouldn’t this be stopping the build if this check fails and REDUCE_EXPORTS is used (same as master)? Although, given they are unused, could be better to just remove entirely, rather than porting and not matching the behaviour. I’ll open a PR.

    fanquake commented at 2:11 pm on August 5, 2024:
    See #30590.

    hebasto commented at 7:20 pm on August 6, 2024:
    Addressed in #30454 (comment).
  68. in CMakeLists.txt:338 in 80073c75d1 outdated
    327+
    328+# Use 64-bit off_t on 32-bit Linux.
    329+if (CMAKE_SYSTEM_NAME STREQUAL "Linux" AND CMAKE_SIZEOF_VOID_P EQUAL 4)
    330+  # Ensure 64-bit offsets are used for filesystem accesses for 32-bit compilation.
    331+  target_compile_definitions(core_interface INTERFACE
    332+    _FILE_OFFSET_BITS=64
    


    fanquake commented at 4:41 pm on August 5, 2024:
    Looks like AC_SYS_LARGEFILE / _LARGE_FILES was never ported?

    hebasto commented at 8:06 pm on August 7, 2024:

    Looks like AC_SYS_LARGEFILE / _LARGE_FILES was never ported?

    The comment in the implementation code mentions “32-bit AIX 4.2.1+, 32-bit z/OS”. Do we support those systems?

  69. hebasto force-pushed on Aug 5, 2024
  70. hebasto commented at 6:26 pm on August 5, 2024: member

    Rebased due to the conflicts. This PR branch is the current staging branch with the top 3 commits dropped.

    Some feedback has been addressed including @furszy’s #30454#pullrequestreview-2202007732.

    Also the staging branch contains no actual code/test changes (previously, there was an exception for qt plugins), only build-system changes.

  71. DrahtBot removed the label Needs rebase on Aug 5, 2024
  72. in CMakeLists.txt:202 in b440081908 outdated
    193+    COMPONENTS ${qt_components}
    194+  )
    195+  unset(qt_components)
    196+endif()
    197+
    198+option(BUILD_BENCH "Build bench_bitcoin executable." OFF)
    


    l0rinc commented at 8:29 pm on August 5, 2024:

    Previously benchmarks were enabled by default, right? (Edit, just noticed previous comments asking the same)

    Is this why the build is so much faster now?

    I can run the benchmarks from the command line, but the IDE is complaining they’re not part of the project. Is that expected?

    Same is true for the fuzz tests - was hoping I could run them from the IDE now.

    Edit: I see now that turning off bench was deliberate, my questions remain only about the IDE not detecting the benchmarks and fuzz even after I enable them, and running FUZZ tests from IDE, like we can run and debug other tests.


    hebasto commented at 8:36 am on August 6, 2024:

    Previously benchmarks were enabled by default, right?

    Right.

    Is this why the build is so much faster now?

    As mentioned earlier:

    Regarding the part about “the default with autoconf is on” part, in the CMake working group, we decided to enable by default only the “essential” build targets that most developers build most often in their everyday work. This means, for instance, that -DBUILD_BENCH and -DBUILD_GUI are OFF by default.

    I can run the benchmarks from the command line, but the IDE is complaining they’re not part of the project. Is that expected?

    Is the project configured with -DBUILD_BENCH=ON?


    l0rinc commented at 12:02 pm on August 6, 2024:
    yeah (even edited the scripts to have them ON by default), I can run a benchmark from the terminal, but the IDE doesn’t see them as part of the project. Maybe @rot13maxi can confirm or deny whether CLion sees the benchmarks.

    hebasto commented at 12:14 pm on August 6, 2024:

    @paplorinc

    Please open an issue in https://github.com/hebasto/bitcoin/issues with detailed steps to reproduce the problem from the scratch.


    l0rinc commented at 2:15 pm on August 6, 2024:

    I managed to make it work, I think the internal IDE setting were overwriting the ones I was setting from the command line.

    For the record, with these settings the bench and fuzz projects are enabled (though no IDE assistance - i.e. run from the IDE -, but that’s a different issue):

    I did open an unrelated suggestion for better error messages: https://github.com/hebasto/bitcoin/issues/306


    hebasto commented at 3:36 pm on August 6, 2024:

    I managed to make it work…

    I consider this conversation as resolved then.

  73. DrahtBot added the label Needs rebase on Aug 5, 2024
  74. in CMakeLists.txt:6 in b440081908 outdated
    0@@ -0,0 +1,658 @@
    1+# Copyright (c) 2023-present The Bitcoin Core developers
    2+# Distributed under the MIT software license, see the accompanying
    3+# file COPYING or https://opensource.org/license/mit/.
    4+
    5+# Ubuntu 22.04 LTS Jammy Jellyfish, https://wiki.ubuntu.com/Releases, EOSS in June 2027:
    6+#  - CMake 3.22.1, https://packages.ubuntu.com/jammy/cmake
    


    l0rinc commented at 6:58 am on August 6, 2024:

    if this refers to the docker base, we could update it to:

    0# Ubuntu 24.04 LTS Noble Numbat, https://wiki.ubuntu.com/Releases, EOSS in June 2029, EOL in April 2036:
    1#  - CMake 3.28.3, https://packages.ubuntu.com/noble/cmake
    

    hebasto commented at 8:05 am on August 6, 2024:

    if this refers to the docker base…

    It refers to a system that users are using to build Bitcoin Core. In most cases, it will be their regular machines. However, building Bitcoin Core within a Docker container is also supported.

    Ubuntu and CentOS are mentioned here as mainstream DEB and RPM-based distros. However, the new CMake-based build system is designed to work out-of-the-box on a wide range of other platforms.


    l0rinc commented at 11:55 am on August 6, 2024:
    Got it, so our tests are run with a different version than what we’re mentioning here, right? Not sure it makes sense, but could we have one of the docker images have this version, to expose ourselves to the documented os version as well (e.g. to catch if someone uses an old dependency, like a CMake 3.22.1 incompatibility)?

    hebasto commented at 12:09 pm on August 6, 2024:

    Got it, so our tests are run with a different version than what we’re mentioning here, right?

    CMake version can be any >= 3.22. However, CMake ensures backward compatibility using the policies. Only policies known to version 3.22 are enabled, regardless of the actual CMake version.

    Not sure it makes sense, but could we have one of the docker images have this version, to expose ourselves to the documented os version as well (e.g. to catch if someone uses an old dependency, like a CMake 3.22.1 incompatibility)?

    That was done for the staging branch (see .github/workflows/cmake.yml). For this PR, it is out of the scope as it is focused on the migration only. Anyway, this CI job uses Ubuntu 22.04 and CMake 3.22.1.


    l0rinc commented at 12:18 pm on August 6, 2024:
    Thanks
  75. in CMakeLists.txt:10 in b440081908 outdated
     5+# Ubuntu 22.04 LTS Jammy Jellyfish, https://wiki.ubuntu.com/Releases, EOSS in June 2027:
     6+#  - CMake 3.22.1, https://packages.ubuntu.com/jammy/cmake
     7+#
     8+# Centos Stream 9, EOL in May 2027:
     9+#  - CMake 3.26.5, https://mirror.stream.centos.org/9-stream/AppStream/x86_64/os/Packages/
    10+cmake_minimum_required(VERSION 3.22)
    


    l0rinc commented at 6:58 am on August 6, 2024:
    if we update the above, this can likely also be bumped, right?

    hebasto commented at 8:11 am on August 6, 2024:

    We still want to allow the building of Bitcoin Core on Ubuntu 22.04 using the cmake package available in the system repository.

    For reference, the last time the CMake minimum required version was updated was in https://github.com/hebasto/bitcoin/pull/123.

  76. hebasto referenced this in commit 755e34f580 on Aug 6, 2024
  77. hebasto referenced this in commit d8471379cd on Aug 6, 2024
  78. hebasto force-pushed on Aug 6, 2024
  79. hebasto commented at 7:19 pm on August 6, 2024: member
    Rebased due to the conflicts. More feedback has been addressed.
  80. DrahtBot removed the label Needs rebase on Aug 6, 2024
  81. in cmake/module/AddBoostIfNeeded.cmake:47 in ec5d1c372b outdated
    42+
    43+  # Prevent use of std::unary_function, which was removed in C++17,
    44+  # and will generate warnings with newer compilers for Boost
    45+  # older than 1.80.
    46+  # See: https://github.com/boostorg/config/pull/430.
    47+  set(CMAKE_REQUIRED_DEFINITIONS -DBOOST_NO_CXX98_FUNCTION_BASE)
    


    fanquake commented at 9:27 am on August 7, 2024:

    Pretty sure there are two separate bugs here:

    • The first is that BOOST_NO_CXX98_FUNCTION_BASE isn’t removed from CMAKE_REQUIRED_DEFINITIONS after this check, which means it’s incorrectly reused for the check below.

    • The second is that when -DWERROR=ON is used, -Werror isn’t being passed to the check for the Boost Test header (which also hides the first bug).


    hebasto commented at 3:55 pm on August 7, 2024:
    For the following discussion, please see https://github.com/hebasto/bitcoin/pull/312.

    hebasto commented at 8:11 am on August 17, 2024:
    Resolved in #30454 (comment).
  82. in cmake/introspection.cmake:41 in ec5d1c372b outdated
    36+check_cxx_symbol_exists(::_wsystem "stdlib.h" HAVE__WSYSTEM)
    37+if(HAVE_STD_SYSTEM OR HAVE__WSYSTEM)
    38+  set(HAVE_SYSTEM 1)
    39+endif()
    40+
    41+check_include_file_cxx(string.h HAVE_STRING_H)
    


    fanquake commented at 4:39 pm on August 7, 2024:
    I think we can just drop include checks like this (which don’t produce defines used anywhere else in the codebase). I realise this kind of check was ported from Autotools, but I no-longer think it’s necessary to do. Running a check to see if we can include string.h, just to then immediately compile something which includes string.h, seems like it should just be collapsed down to the compile. Looks like another one is check_include_file_cxx(unistd.h HAVE_UNISTD_H).

    hebasto commented at 7:11 pm on August 7, 2024:

    hebasto commented at 11:19 am on August 10, 2024:
    Resolved in #30454 (comment).
  83. hebasto referenced this in commit ad2140d4d8 on Aug 7, 2024
  84. in doc/fuzzing.md:152 in ec5d1c372b outdated
    149-./configure --enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=$(brew --prefix llvm)/bin/clang CXX=$(brew --prefix llvm)/bin/clang++
    150+$ cmake -B build_fuzz \
    151+   -DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
    152+   -DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
    153+   -DBUILD_FOR_FUZZING=ON \
    154+   -DSANITIZERS=undefined,address,fuzzer \
    


    l0rinc commented at 7:26 pm on August 7, 2024:
    do we need the escape at the end?

    hebasto commented at 8:08 pm on August 7, 2024:

    dergoegge commented at 9:01 am on August 8, 2024:
    We probably don’t.

    hebasto commented at 1:19 pm on August 9, 2024:

    hebasto commented at 11:19 am on August 10, 2024:
    Resolved in #30454 (comment).
  85. in doc/fuzzing.md:176 in ec5d1c372b outdated
    176+   -DCMAKE_C_COMPILER="$(pwd)/AFLplusplus/afl-clang-lto" \
    177+   -DCMAKE_CXX_COMPILER="$(pwd)/AFLplusplus/afl-clang-lto++" \
    178+   -DBUILD_FOR_FUZZING=ON
    179+$ cmake --build build_fuzz -j$(nproc)
    180+# For macOS you may need to ignore x86 compilation checks when running "cmake --build". If so,
    181+# try compiling using: `AFL_NO_X86=1 cmake --build build_fuzz -j$(nproc)`
    


    l0rinc commented at 7:29 pm on August 7, 2024:
    if this comment still refers to the mac build, note that mac doesn’t have nproc

    hebasto commented at 8:08 pm on August 7, 2024:

    dergoegge commented at 9:02 am on August 8, 2024:
    Right, the “-j$(nproc)” bit should be removed. I don’t have a Mac so I don’t no if these instructions are still relevant.

    l0rinc commented at 9:38 am on August 8, 2024:
    The mac equivalent is make -j$(sysctl -n hw.ncpu)

    hebasto commented at 1:19 pm on August 9, 2024:

    hebasto commented at 11:20 am on August 10, 2024:
    Resolved in #30454 (comment).
  86. in doc/fuzzing.md:14 in ec5d1c372b outdated
    11-$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
    12+$ cmake -B build_fuzz \
    13+   -DCMAKE_C_COMPILER="clang" \
    14+   -DCMAKE_CXX_COMPILER="clang++" \
    15+   -DBUILD_FOR_FUZZING=ON \
    16+   -DSANITIZERS=undefined,address,fuzzer
    


    l0rinc commented at 8:13 pm on August 7, 2024:

    I wasn’t able to run the fuzz tests with cmake (tried all mac tricks I could find here), I’m getting:

    0-- Performing Test CXX_SUPPORTS__FSANITIZE_UNDEFINED_ADDRESS_FUZZER
    1-- Performing Test CXX_SUPPORTS__FSANITIZE_UNDEFINED_ADDRESS_FUZZER - Success
    2-- Performing Test LINKER_SUPPORTS__FSANITIZE_UNDEFINED_ADDRESS_FUZZER_a797
    3-- Performing Test LINKER_SUPPORTS__FSANITIZE_UNDEFINED_ADDRESS_FUZZER_a797 - Failed
    4CMake Error at CMakeLists.txt:389 (message):
    5  Linker did not accept requested flags, you are missing required libraries.
    6
    7
    8-- Configuring incomplete, errors occurred!
    

    given:

     0% brew --prefix llvm   
     1/opt/homebrew/opt/llvm
     2
     3% clang --version               
     4Homebrew clang version 18.1.8
     5Target: arm64-apple-darwin23.5.0
     6Thread model: posix
     7InstalledDir: /opt/homebrew/opt/llvm/bin
     8
     9% clang++ --version               
    10Homebrew clang version 18.1.8
    11Target: arm64-apple-darwin23.5.0
    12Thread model: posix
    13InstalledDir: /opt/homebrew/opt/llvm/bin
    

    Am I doing something wrong or should I open an issue for this as well?

    cc @dergoegge


    hebasto commented at 8:26 pm on August 7, 2024:

    I wasn’t able to run the fuzz tests with cmake

    Are you able to build them using Autotools?


    l0rinc commented at 8:55 pm on August 7, 2024:
    Kinda’, but I do get some errors at the beginning.

    fanquake commented at 9:00 am on August 8, 2024:

    Are you able to build them using Autotools?

    Yea, the steps in fuzzing.md currently work with master, but don’t work this branch (ad2140d4d8cccec82fdad07bdc7532e202282306).


    dergoegge commented at 9:14 am on August 8, 2024:
    I’ve only tried the instructions in this doc on linux when I wrote them a few weeks back. I’m guessing this is a Mac only issue but perhaps not.

    l0rinc commented at 11:29 am on August 8, 2024:
    Is there any way I could help here?

    hebasto commented at 1:56 pm on August 8, 2024:

    I’ve tested on macOS Monterey 12.7.6 (Intel) with Homebrew’s llvm@16. Everything works flawlessly.

    Is there any way I could help here?

    Mind providing full verbose build logs for both Autotools and CMake please?


    l0rinc commented at 8:49 pm on August 8, 2024:

    The problem was that brew gcc and Xcode 15 have an duplicate lib:

    ld: warning: ignoring duplicate libraries: ‘-lc++‘gs)

    But since we’ve enabled fatal warnings for Mac, we’re getting:

    ld: fatal warning(s) induced error (-fatal_warnings)

    Based on: https://stackoverflow.com/questions/77164140/ld-warning-ignoring-duplicate-libraries-lgcc-after-the-recent-update-of-xc we should be able to work around it:

     0diff --git a/cmake/module/TryAppendLinkerFlag.cmake b/cmake/module/TryAppendLinkerFlag.cmake
     1--- a/cmake/module/TryAppendLinkerFlag.cmake	(revision ec5d1c372b20d49147813aa0392195a3642b86a1)
     2+++ b/cmake/module/TryAppendLinkerFlag.cmake	(date 1723148834055)
     3@@ -79,7 +79,7 @@
     4 if(MSVC)
     5   try_append_linker_flag("/WX" VAR working_linker_werror_flag)
     6 elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
     7-  try_append_linker_flag("-Wl,-fatal_warnings" VAR working_linker_werror_flag)
     8+  try_append_linker_flag("-Wl,-no_warn_duplicate_libraries,-fatal_warnings" VAR working_linker_werror_flag) # https://github.com/orgs/Homebrew/discussions/4794#discussioncomment-7044468
     9 else()
    10   try_append_linker_flag("-Wl,--fatal-warnings" VAR working_linker_werror_flag)
    11 endif()
    

    which makes building with fuzz possible:

    ld: warning: ignoring duplicate libraries: ‘-lc++’, ‘../../../libleveldb.a’, ‘../../../libminisketch.a’, ‘../../libbitcoin_common.a’, ‘../../secp256k1/src/libsecp256k1.a’, ‘../../univalue/libunivalue.a’ [100%] Built target fuzz

    and fuzzing can start (with the same error as the Autotools version)

    % FUZZ=bech32 ./build_fuzz/src/test/fuzz/fuzz
    fuzz(91171,0x2060fcc00) malloc: nano zone abandoned due to inability to reserve vm space. /opt/homebrew/opt/llvm/bin/../include/c++/v1/variant:495:12: runtime error: call to function decltype(auto) std::__1::__variant_detail::__visitation::__base::__dispatcher<0ul, 0ul>::__dispatch[abi:ne180100]<void std::__1::__variant_detail::__ctor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue»::__generic_construct[abi:ne180100]<std::__1::__variant_detail::__move_constructor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>, (std::__1::__variant_detail::_Trait)1»(std::__1::__variant_detail::__ctor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue»&, std::__1::__variant_detail::__move_constructor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>, (std::__1::__variant_detail::_Trait)1>&&)::’lambda’(std::__1::__variant_detail::__move_constructor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>, (std::__1::__variant_detail::_Trait)1>&, auto&&)&&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>&&>(std::__1::__variant_detail::__move_constructor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>, (std::__1::__variant_detail::_Trait)1>, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>&&) through pointer to incorrect function type ‘void (*)((lambda at /opt/homebrew/opt/llvm/bin/../include/c++/v1/variant:814:11) &&, std::__variant_detail::__base<std::__variant_detail::_Trait::_Available, RPCArg::Optional, std::string, UniValue> &, std::__variant_detail::__base<std::__variant_detail::_Trait::_Available, RPCArg::Optional, std::string, UniValue> &&)’ variant:532: note: decltype(auto) std::__1::__variant_detail::__visitation::__base::__dispatcher<0ul, 0ul>::__dispatch[abi:ne180100]<void std::__1::__variant_detail::__ctor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue»::__generic_construct[abi:ne180100]<std::__1::__variant_detail::__move_constructor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>, (std::__1::__variant_detail::_Trait)1»(std::__1::__variant_detail::__ctor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue»&, std::__1::__variant_detail::__move_constructor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>, (std::__1::__variant_detail::_Trait)1>&&)::’lambda’(std::__1::__variant_detail::__move_constructor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>, (std::__1::__variant_detail::_Trait)1>&, auto&&)&&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>&&>(std::__1::__variant_detail::__move_constructor<std::__1::__variant_detail::__traits<RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>, (std::__1::__variant_detail::_Trait)1>, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>&, std::__1::__variant_detail::__base<(std::__1::__variant_detail::_Trait)1, RPCArg::Optional, std::__1::basic_string<char, std::__1::char_traits, std::__1::allocator>, UniValue>&&) defined here SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /opt/homebrew/opt/llvm/bin/../include/c++/v1/variant:495:12 INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 534925934 INFO: Loaded 1 modules (1210318 inline 8-bit counters): 1210318 [0x105d56048, 0x105e7d816), INFO: Loaded 1 PC tables (1210318 PCs): 1210318 [0x105e7d818,0x1070f54f8), INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes INFO: A corpus is not provided, starting from an empty corpus #2 INITED cov: 294 ft: 294 corp: 1/1b exec/s: 0 rss: 74Mb #3 NEW cov: 294 ft: 298 corp: 2/2b lim: 4 exec/s: 0 rss: 74Mb L: 1/1 MS: 1 ChangeBit- #5 NEW cov: 295 ft: 301 corp: 3/3b lim: 4 exec/s: 0 rss: 74Mb L: 1/1 MS: 2 ShuffleBytes-ChangeBit- #6 NEW cov: 295 ft: 303 corp: 4/4b lim: 4 exec/s: 0 rss: 74Mb L: 1/1 MS: 1 ChangeBit- #7 NEW cov: 295 ft: 305 corp: 5/5b lim: 4 exec/s: 0 rss: 74Mb L: 1/1 MS: 1 ChangeBit-


    fanquake commented at 8:41 am on August 9, 2024:

    The problem was that brew gcc and Xcode 15 have an duplicate lib:

    Not sure, given that we aren’t using Brew GCC, or Xcode, and the machine I tested on doesn’t have either of them installed. The warning output might match, but I don’t think this is the correct explanation (and it doesn’t explain why it doesn’t happen for Autotools build).


    l0rinc commented at 9:25 am on August 9, 2024:
    This definitely fixed it for me, how can I help with the other cases you’ve mentioned?

    hebasto commented at 9:39 am on August 9, 2024:

    and it doesn’t explain why it doesn’t happen for Autotools build

    Autotools lack -Wl,-fatal_warnings in the following check: https://github.com/bitcoin/bitcoin/blob/24ced5274438c0185963ae1296ec7dda97ea62f1/configure.ac#L365-L375


    fanquake commented at 9:42 am on August 9, 2024:
    Ok, but it’s still not clear why a compiler we don’t use / isn’t installed, would be adding libraries to the link line.

    l0rinc commented at 10:05 am on August 9, 2024:
    The proposed -no_warn_duplicate_libraries will only fix it for duplicate libs warnings (but any other warning would fail the build). Can we check if we’re getting any other warning - or whether the above change fixes it?

    m3dwards commented at 10:11 am on August 9, 2024:

    On Arm Mac I get the same configuration problem. In the logs I’m seeing:

    0ld: warning: ignoring duplicate libraries: '-lc++'
    1ld: fatal warning(s) induced error (-fatal_warnings)
    

    It works with autotools (perhaps warnings aren’t errors in the configure tests?). I’ve passed the full log to @hebasto.


    hebasto commented at 10:13 am on August 9, 2024:

    @m3dwards

    It works with autotools (perhaps warnings aren’t errors in the configure tests?).

    See: #30454 (review)


    m3dwards commented at 10:27 am on August 9, 2024:
    @hebasto I had a stale version of this page open and hadn’t seen the latest comments 🤦

    m3dwards commented at 10:49 am on August 9, 2024:

    hebasto commented at 11:01 am on August 9, 2024:

    On my macOS Sonoma 14.6.1:

    0% cat test.cpp 
    1int main() {}
    2% /opt/homebrew/opt/llvm/bin/clang++ --version
    3Homebrew clang version 18.1.8
    4Target: arm64-apple-darwin23.6.0
    5Thread model: posix
    6InstalledDir: /opt/homebrew/opt/llvm/bin
    7% /opt/homebrew/opt/llvm/bin/clang++ -fsanitize=fuzzer -o test test.cpp
    8ld: warning: ignoring duplicate libraries: '-lc++'
    

    UPD. Same for the llvm@17 package, which provides clang version 17.0.6.

    UPD 2. Same for the llvm@16 package, which provides clang version 16.0.6.


    l0rinc commented at 11:33 am on August 9, 2024:

    hebasto commented at 12:01 pm on August 9, 2024:
    As an alternative workaround, I’d suggest to use an extra configuration option -DAPPEND_LDFLAGS="-Wl,-no_warn_duplicate_libraries".

    l0rinc commented at 12:26 pm on August 9, 2024:
    btw, is it expected that modifying TryAppendLinkerFlag.cmake (e.g. the try_append_linker_flag("-Wl,-fatal_warnings" VAR working_linker_werror_flag) line) and rerunning cmake -B build_fuzz ignores the update? Shouldn’t it detect that this file was changed? I need to rm -rfd build_fuzz && cmake -B build_fuzz for it to be picked up.

    dergoegge commented at 12:34 pm on August 9, 2024:
    As a side note, the UBSan errors observed here are not caused by CMake. Other people have observed the same Mac specific behavior with autotools: #30413 (comment).

    l0rinc commented at 12:39 pm on August 9, 2024:

    errors observed #30454 (review) are not caused by CMake

    Yeah, my example in #30454 (review) was also in Autotools. The same happens with CMake in #30454 (review)


    hebasto commented at 12:47 pm on August 9, 2024:

    btw, is it expected that modifying TryAppendLinkerFlag.cmake (e.g. the try_append_linker_flag("-Wl,-fatal_warnings" VAR working_linker_werror_flag) line) and rerunning cmake -B build_fuzz ignores the update?

    Yes, that is because the result is cached.


    l0rinc commented at 12:48 pm on August 9, 2024:
    Of course, but shouldn’t this change invalidate the cache?

    hebasto commented at 12:52 pm on August 9, 2024:
    Apparently, not every change invalidates the cache. Mind reporting to https://gitlab.kitware.com/cmake/cmake/-/issues?

    hebasto commented at 1:20 pm on August 9, 2024:

    As an alternative workaround, I’d suggest to use an extra configuration option -DAPPEND_LDFLAGS="-Wl,-no_warn_duplicate_libraries".

    Addressed in https://github.com/hebasto/bitcoin/pull/316.


    l0rinc commented at 10:35 am on August 10, 2024:

    Apparently, not every change invalidates the cache

    My understanding is that we’re already avoiding the recompilation when the source didn’t change, but when working_linker_werror_flag changes, that’s ignored. I haven’t opened a bug report to CMake since it seems to me we can fix it ourselved, see: https://github.com/hebasto/bitcoin/pull/319


    hebasto commented at 11:23 am on August 10, 2024:

    I wasn’t able to run the fuzz tests with cmake

    Resolved in #30454 (comment).

  87. in src/CMakeLists.txt:13 in ec5d1c372b outdated
     8+configure_file(${PROJECT_SOURCE_DIR}/cmake/bitcoin-config.h.in config/bitcoin-config.h @ONLY)
     9+include_directories(${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_CURRENT_SOURCE_DIR})
    10+
    11+# After the transition from Autotools to CMake, the obj/ subdirectory
    12+# could be dropped as its only purpose was to separate a generated header
    13+# from source files.
    


    l0rinc commented at 9:53 am on August 8, 2024:
    nit, what’s the purpose of the comment? Is it to explain to the reviewers how to review it or will this make sense after we remove autotools? If the second, maybe we can explain in the commit message instead - or add a TODO in front of this to signal that it’s a temporary explanation (otherwise people might be afraid to remove it)

    hebasto commented at 6:24 pm on August 8, 2024:
    It is a TODO comment. I’ll update it if/when this file will be touched for other changes.

    hebasto commented at 1:20 pm on August 9, 2024:

    hebasto commented at 11:21 am on August 10, 2024:
    Resolved in #30454 (comment).
  88. in doc/build-osx.md:109 in ec5d1c372b outdated
    107-Skip if you don't intend to use the GUI.
    108+Bitcoin Core includes a GUI built with the cross-platform Qt Framework. To compile the GUI, we need to install
    109+the necessary parts of Qt, the libqrencode and pass `-DBUILD_GUI=ON`. Skip if you don't intend to use the GUI.
    110 
    111 ``` bash
    112 brew install qt@5
    


    l0rinc commented at 10:14 am on August 8, 2024:
    👍 GUI is working with cmake, but I had to execute a brew link qt5 --force as well after brew install qt@5 (applies to Autotools as well)
  89. in doc/build-netbsd.md:30 in ec5d1c372b outdated
    28+cmake -B build
    29     ...
    30-    CC="/usr/pkg/gcc12/bin/gcc" \
    31-    CXX="/usr/pkg/gcc12/bin/g++" \
    32+    -DCMAKE_C_COMPILER="/usr/pkg/gcc12/bin/gcc" \
    33+    -DCMAKE_CXX_COMPILER="/usr/pkg/gcc12/bin/g++" \
    


    l0rinc commented at 10:28 am on August 8, 2024:

    Nit:

    0    -DCMAKE_C_COMPILER=/usr/pkg/gcc12/bin/gcc \
    1    -DCMAKE_CXX_COMPILER=/usr/pkg/gcc12/bin/g++ \
    

    hebasto commented at 12:24 pm on August 8, 2024:
    The quoting style was inherited from the master branch. Such an approach makes the diff easier to review.
  90. in doc/build-osx.md:272 in ec5d1c372b outdated
    272@@ -276,8 +273,8 @@ tail -f $HOME/Library/Application\ Support/Bitcoin/debug.log
    273 ## Other commands:
    274 
    275 ```shell
    


    l0rinc commented at 10:37 am on August 8, 2024:

    nit: slightly unrelated, but in other cases similar code blocks are annotated as either “```bash” or “```sh” which are treated a bit different in e.g. CLion:

    Also note that “``` bash” is also a formatted differently than “```bash”.

    Should probably be done in a different PR, though…


    hebasto commented at 12:35 pm on August 8, 2024:

    This PR is intended to focus solely on migration. It helps to review it a lot.

    Should probably be done in a different PR, though…

    I agree.

  91. hebasto marked this as ready for review on Aug 8, 2024
  92. in src/wallet/test/CMakeLists.txt:10 in ec5d1c372b outdated
     5+# Do not use generator expressions in test sources because the
     6+# SOURCES property is processed to gather test suite macros.
     7+target_sources(test_bitcoin
     8+  PRIVATE
     9+    init_test_fixture.cpp
    10+    wallet_test_fixture.cpp
    


    l0rinc commented at 10:49 am on August 8, 2024:
    nit: not very important, but can help with avoiding merge conflicts if we always sort these alphabetically (would be nice to have an automatic check for these, of course). Unless this is deliberate, in which case a comment could help.

    hebasto commented at 6:29 pm on August 8, 2024:

    Source files were separated in two groups: fixtures and tests themselves. Both groups are sorted alphabetically.

    That seemed convenient to me while I was working on this script.

  93. in cmake/script/GenerateHeaderFromRaw.cmake:6 in ec5d1c372b outdated
    0@@ -0,0 +1,22 @@
    1+# Copyright (c) 2023-present The Bitcoin Core developers
    2+# Distributed under the MIT software license, see the accompanying
    3+# file COPYING or https://opensource.org/license/mit/.
    4+
    5+file(READ ${RAW_SOURCE_PATH} hex_content HEX)
    6+string(REGEX MATCHALL "([A-Za-z0-9][A-Za-z0-9])" bytes "${hex_content}")
    


    l0rinc commented at 11:24 am on August 8, 2024:
    Wanted to suggest"([A-Za-z0-9]{2})" (or \w{2}), but it seems the regex parser was written almost 40 years ago https://github.com/Kitware/CMake/blob/master/Source/kwsys/RegularExpression.cxx#L628

    hebasto commented at 12:31 pm on August 8, 2024:
    Yes. The CMake Regex Specification does not include \w support.
  94. l0rinc commented at 11:36 am on August 8, 2024: contributor
    Checked a few more scenarios on mac
  95. hebasto commented at 6:22 pm on August 8, 2024: member

    An announcement about the migration to the CMake build system was made on the mailing list. Therefore, more comments from fellow reviewers and testers are expected.

    To let us maintain focus on making progress, the following statement has been add to the PR description:

    We, the build system developers, kindly ask reviewers to refrain from making suggestions that are not directly related to the migration process or can be implemented separately. Bugs in the scripts and errors in the updated documentation should be the focus of this PR. Please be advised that comments not aligned with this PR’s goal may be ignored.

    Thank you all for your understanding and cooperation.

  96. DrahtBot commented at 4:37 am on August 9, 2024: contributor

    Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

    File commit 27a770b34b8f1dbb84760f442edb3e23a0c2420b(master) commit 2c5f22560b4b203f14bc64fd41bf42345cb0ba81(master and this pull)
    SHA256SUMS.part cc61b98f761ae0d2...
    *-aarch64-linux-gnu-debug.tar.gz 8520aad18b834d92...
    *-aarch64-linux-gnu.tar.gz b4981c155eff7bcb...
    *-arm-linux-gnueabihf-debug.tar.gz abc05bea90c911f2...
    *-arm-linux-gnueabihf.tar.gz 9afabfd5f1348c88...
    *-arm64-apple-darwin-unsigned.tar.gz a2f045d4d370e511...
    *-arm64-apple-darwin-unsigned.zip 570ec3c274a76cc4...
    *-arm64-apple-darwin.tar.gz 0289dbfde1855827...
    *-powerpc64-linux-gnu-debug.tar.gz 88bdbb314e5a7bf1...
    *-powerpc64-linux-gnu.tar.gz a22b2be3f32a37ee...
    *-riscv64-linux-gnu-debug.tar.gz e405c6dac4299122...
    *-riscv64-linux-gnu.tar.gz 89df8a8fb8316c45...
    *-x86_64-apple-darwin-unsigned.tar.gz f8e5a8ab26dc2c8c...
    *-x86_64-apple-darwin-unsigned.zip 7ee3a84b21371065...
    *-x86_64-apple-darwin.tar.gz d8991ce1e625ea73...
    *-x86_64-linux-gnu-debug.tar.gz 7e73078d515a706a...
    *-x86_64-linux-gnu.tar.gz dbd36be395769a40...
    *.tar.gz ce196c8f7dc80175... 87318cf6abea41dd...
    guix_build.log b1c81e8b50991225... f1d169a82935b789...
    guix_build.log.diff e986565f30dc297a...
  97. DrahtBot removed the label DrahtBot Guix build requested on Aug 9, 2024
  98. maflcko commented at 5:42 am on August 9, 2024: member
    (The drahtbot guix build failed due to a silent merge conflict, I presume)
  99. fanquake commented at 11:17 am on August 9, 2024: member

    Had a look at a Guix build. Stripping the macOS binaries is broken:

    0-- Installing: /distsrc-base/distsrc-ad2140d4d8cc-arm64-apple-darwin/installed/bitcoin-ad2140d4d8cc/bin/bitcoind
    1/root/.guix-profile/bin/llvm-strip: error: unknown argument '-u'
    
  100. fanquake commented at 12:57 pm on August 9, 2024: member

    I think we can improve the output when -DWITH_CCACHE=OFF is used. Depending on the system, that output might be:

    0cmake -B build -DWITH_CCACHE=OFF
    1< snip >
    2Use ccache for compiling .............. ccache masquerades as the compiler
    

    We should probably at least indicate that the option was respected by the build-system.

  101. hebasto commented at 1:52 pm on August 9, 2024: member

    (The drahtbot guix build failed due to a silent merge conflict, I presume)

    Should be fixed after porting #30051.

  102. in depends/README.md:21 in ec5d1c372b outdated
    25 
    26-    CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site ./configure
    27-
    28-The default install prefix when using `config.site` is `--prefix=depends/<host-platform-triplet>`,
    29-so depends build outputs will be installed in that location.
    30+    cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
    



    hebasto commented at 3:26 pm on August 9, 2024:

    are we planning on updating them here or in a separate pr, e.g. when removing Autotools?

    Here. Feel free to open a PR in https://github.com/hebasto/bitcoin/pulls against the cmake-staging branch. After merging, the changes will be pulled in here during the next update.

    UPD. Not all mentioned cases are related to CMake migration. For instance, depends still use the GNU Make.


    l0rinc commented at 12:18 pm on August 11, 2024:
    How much rewrite do we need to do here? I have pushed https://github.com/hebasto/bitcoin/pull/322 to synchronize on the style, let me know how you imagine it.


    hebasto commented at 1:29 pm on August 12, 2024:

    hebasto commented at 8:12 am on August 17, 2024:
    Resolved in #30454 (comment).
  103. DrahtBot commented at 8:49 pm on August 9, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/28425222462

    Make sure to run all tests locally, according to the documentation.

    The failure may happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

  104. DrahtBot added the label CI failed on Aug 9, 2024
  105. in CMakeLists.txt:49 in ec5d1c372b outdated
    40+  CMAKE_CXX_COMPILE_OBJECT
    41+  CMAKE_OBJCXX_COMPILE_OBJECT
    42+  CMAKE_CXX_LINK_EXECUTABLE
    43+)
    44+
    45+project(BitcoinCore
    


    l0rinc commented at 8:28 am on August 10, 2024:
    Q: given that FUZZ and BENCH are disabled by default, would it maybe make sense to make them separate projects? I’m not exactly familiar with what that would involve and don’t have strong arguments either way, was just wondering when investigating some other cmake related issue.

    hebasto commented at 11:11 am on August 10, 2024:
    Could you elaborate please? What are the benefits of maintaining the build system in separate projects? Is this practice used by other open-source projects?


    hebasto commented at 11:25 am on August 10, 2024:

    Still curios about:

    What are the benefits of maintaining the build system in separate projects?


    l0rinc commented at 11:32 am on August 10, 2024:

    I’m also curious, that’s what I’m asking basically, why did the other clones decide to do this differently.

    I could only find general Modularity and Reusability/Scalability/Improved Dependency Management/Isolation of Build Configurations arguments online, but not sure any applies here, hence my question.


    hebasto commented at 12:18 pm on August 10, 2024:

    Might be controversial, but other Bitcoin clones seem to split into multiple projects after their CMake migrations:

    It is not controversial, just broken :)

    The cmake -S src/bench -B build command fails.


    l0rinc commented at 10:20 am on August 11, 2024:

    The cmake -S src/bench -B build command fails.

    It seems to me they’re using ninja for these tasks, i.e. ninja bench-bitcoin instead of raw cmake.


    hebasto commented at 10:27 am on August 11, 2024:

    The cmake -S src/bench -B build command fails.

    It seems to me they’re using ninja for these tasks, i.e. ninja bench-bitcoin instead of raw cmake.

    It seems this discussion has gone off topic :)


    l0rinc commented at 10:29 am on August 11, 2024:
    Didn’t realize that, please resolve it in that case
  106. in src/crypto/CMakeLists.txt:13 in ec5d1c372b outdated
     8+  chacha20poly1305.cpp
     9+  hex_base.cpp
    10+  hkdf_sha256_32.cpp
    11+  hmac_sha256.cpp
    12+  hmac_sha512.cpp
    13+  poly1305.cpp
    


    l0rinc commented at 8:29 am on August 10, 2024:

    nit: alphabetic ordering?

     0diff --git a/src/crypto/CMakeLists.txt b/src/crypto/CMakeLists.txt
     1--- a/src/crypto/CMakeLists.txt	(revision 92f7fc21768b0d898655d68b5d76ff09ff71c682)
     2+++ b/src/crypto/CMakeLists.txt	(date 1723279760256)
     3@@ -4,19 +4,19 @@
     4 
     5 add_library(bitcoin_crypto STATIC EXCLUDE_FROM_ALL
     6   aes.cpp
     7-  chacha20.cpp
     8   chacha20poly1305.cpp
     9+  chacha20.cpp
    10   hex_base.cpp
    11   hkdf_sha256_32.cpp
    12   hmac_sha256.cpp
    13   hmac_sha512.cpp
    14-  poly1305.cpp
    15   muhash.cpp
    16+  poly1305.cpp
    17   ripemd160.cpp
    18   sha1.cpp
    19-  sha256.cpp
    20-  sha256_sse4.cpp
    21   sha3.cpp
    22+  sha256_sse4.cpp
    23+  sha256.cpp
    24   sha512.cpp
    25   siphash.cpp
    26   ../support/cleanse.cpp
    

    hebasto commented at 12:47 pm on August 10, 2024:

    hebasto commented at 8:12 am on August 17, 2024:
    Resolved in #30454 (comment).
  107. in src/wallet/CMakeLists.txt:24 in ec5d1c372b outdated
    20+  rpc/addresses.cpp
    21+  rpc/backup.cpp
    22+  rpc/coins.cpp
    23+  rpc/encrypt.cpp
    24+  rpc/spend.cpp
    25+  rpc/signmessage.cpp
    


    l0rinc commented at 10:37 am on August 10, 2024:

    Nit, feel free to ignore these if not important:

    0  rpc/spend.cpp
    1  rpc/signmessage.cpp
    

    hebasto commented at 12:47 pm on August 10, 2024:

    hebasto commented at 8:12 am on August 17, 2024:
    Resolved in #30454 (comment).
  108. in src/CMakeLists.txt:106 in ec5d1c372b outdated
    101+endif()
    102+
    103+# Home for common functionality shared by different executables and libraries.
    104+# Similar to `bitcoin_util` library, but higher-level.
    105+add_library(bitcoin_common STATIC EXCLUDE_FROM_ALL
    106+  addresstype.cpp
    


    l0rinc commented at 10:41 am on August 10, 2024:

    Nit:

     0diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
     1--- a/src/CMakeLists.txt	(revision 92f7fc21768b0d898655d68b5d76ff09ff71c682)
     2+++ b/src/CMakeLists.txt	(date 1723286445028)
     3@@ -106,8 +106,8 @@
     4   addresstype.cpp
     5   base58.cpp
     6   bech32.cpp
     7-  chainparamsbase.cpp
     8   chainparams.cpp
     9+  chainparamsbase.cpp
    10   coins.cpp
    11   common/args.cpp
    12   common/bloom.cpp
    13@@ -130,18 +130,18 @@
    14   key.cpp
    15   key_io.cpp
    16   merkleblock.cpp
    17+  net_permissions.cpp
    18   net_types.cpp
    19   netaddress.cpp
    20   netbase.cpp
    21-  net_permissions.cpp
    22   outputtype.cpp
    23   policy/feerate.cpp
    24   policy/policy.cpp
    25   protocol.cpp
    26   psbt.cpp
    27+  rpc/external_signer.cpp
    28   rpc/rawtransaction_util.cpp
    29   rpc/request.cpp
    30-  rpc/external_signer.cpp
    31   rpc/util.cpp
    32   scheduler.cpp
    33   script/descriptor.cpp
    

    hebasto commented at 12:47 pm on August 10, 2024:

    hebasto commented at 8:12 am on August 17, 2024:
    Resolved in #30454 (comment).
  109. l0rinc commented at 10:45 am on August 10, 2024: contributor
    👍 left a few things I’ve noticed, ignore the nits if out of scope
  110. hebasto force-pushed on Aug 10, 2024
  111. maflcko added the label DrahtBot Guix build requested on Aug 10, 2024
  112. hebasto commented at 11:17 am on August 10, 2024: member

    Rebased due to the conflicts. More feedback has been addressed.

    (The drahtbot guix build failed due to a silent merge conflict, I presume)

    Fixed.

  113. DrahtBot removed the label CI failed on Aug 10, 2024
  114. DrahtBot commented at 8:59 pm on August 10, 2024: contributor

    Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

    File commit c2d15d993ef06d97d4c117012bda6efa3dcbac45(master) commit 9e1d4f3ad4c5e2e6e13d613ce2f85c6fe4741fe9(master and this pull)
    SHA256SUMS.part 32718731b66fa579... 1fab9153bcd288be...
    *-aarch64-linux-gnu-debug.tar.gz 5f7e9224081d972f... 4f2ebfc23ddceda5...
    *-aarch64-linux-gnu.tar.gz a93bc3fa935dfadf... 3cc2ff8948fa899d...
    *-arm-linux-gnueabihf-debug.tar.gz 99730a0c004bc3db... a0d150805cd81152...
    *-arm-linux-gnueabihf.tar.gz 5a12c90fb5d29dba... 08e992a21f2bf799...
    *-arm64-apple-darwin-unsigned.tar.gz c8ea2e5320c64f1e... 61b2f96edc6a038d...
    *-arm64-apple-darwin-unsigned.zip d1e09bf51603721c... 17e1e1f520a91602...
    *-arm64-apple-darwin.tar.gz ece8050e1f74be5e... 9e3cf103f511cde7...
    *-powerpc64-linux-gnu-debug.tar.gz cf7ad7592957fefc... ea2c4b11cf2f88fd...
    *-powerpc64-linux-gnu.tar.gz f2b5b501f220b3ed... deb853798eb19d31...
    *-riscv64-linux-gnu-debug.tar.gz 1f77b7981aebf7e7... 3226082b3c39a8cf...
    *-riscv64-linux-gnu.tar.gz b6d59d74103eaaef... b1c42ee17321f910...
    *-x86_64-apple-darwin-unsigned.tar.gz c3035c4a841bbd6b... 84c25afd2415e35b...
    *-x86_64-apple-darwin-unsigned.zip 6b2cef38732b59bd... d080ade93e33fed0...
    *-x86_64-apple-darwin.tar.gz 610f7ff43cd32e87... 8f25211c18f59873...
    *-x86_64-linux-gnu-debug.tar.gz 460af85bfd9e7a29... b2679e869fa8187e...
    *-x86_64-linux-gnu.tar.gz 48d02aa0e3f4a6cd... bf1a6e0864f5cc0e...
    *.tar.gz d017f099a0a35f8e... e346a2a4572d2af3...
    guix_build.log 6643a4e45f91d3aa... 931faed9c884d257...
    guix_build.log.diff 7a8d0b3925122c0f...
  115. DrahtBot removed the label DrahtBot Guix build requested on Aug 10, 2024
  116. hebasto commented at 9:13 pm on August 10, 2024: member

    @fanquake in #30454 (comment):

    Had a look at a Guix build. Stripping the macOS binaries is broken:

    0-- Installing: /distsrc-base/distsrc-ad2140d4d8cc-arm64-apple-darwin/installed/bitcoin-ad2140d4d8cc/bin/bitcoind
    1/root/.guix-profile/bin/llvm-strip: error: unknown argument '-u'
    

    Fixed in https://github.com/hebasto/bitcoin/pull/321.

  117. hebasto referenced this in commit eff463b85e on Aug 11, 2024
  118. DrahtBot added the label Needs rebase on Aug 12, 2024
  119. hebasto commented at 2:46 pm on August 12, 2024: member

    @fanquake wrote:

    I think we can improve the output when -DWITH_CCACHE=OFF is used. Depending on the system, that output might be:

    0cmake -B build -DWITH_CCACHE=OFF
    1< snip >
    2Use ccache for compiling .............. ccache masquerades as the compiler
    

    We should probably at least indicate that the option was respected by the build-system.

    The issue, along with another bug I’ve noticed, is fixed in https://github.com/hebasto/bitcoin/pull/325.

  120. in cmake/ccache.cmake:28 in 67b1e23633 outdated
    16+    elseif(WITH_CCACHE)
    17+      list(APPEND CMAKE_C_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
    18+      list(APPEND CMAKE_CXX_COMPILER_LAUNCHER ${CCACHE_EXECUTABLE})
    19+    endif()
    20+    try_append_cxx_flags("-fdebug-prefix-map=A=B" TARGET core_interface SKIP_LINK
    21+      IF_CHECK_PASSED "-fdebug-prefix-map=${PROJECT_SOURCE_DIR}=."
    


    l0rinc commented at 7:41 pm on August 12, 2024:

    Note that with 80073c75d1218759f58ceefcb41b4fbc4a3d1ecd I could right click and debug and CLion would stop at a breakpoint, but with 67b1e236334f38ec4e4d2251dbdfb790f20ed88b (adding debug-prefix-map) it won’t stop at breakpoints anymore.

    But as stated in the docs, setting the target.source-map in ~/.lldbinit fixed debugging again. Or when that wasn’t working, setting “Use file name only” on the breakpoint, did.

  121. pablomartin4btc commented at 10:27 pm on August 12, 2024: member

    tACK 67b1e236334f38ec4e4d2251dbdfb790f20ed88b

    Built with GUI on macOS 14.4 following the instructions in /doc/build-osx.md (part of this PR) and run ./build/qt/bitcoin-qt successfully.

    A few observations:

    • Regarding the configuration: in the documentation says: “If berkeley-db@4 is installed, then legacy wallet support will be built.” But I have to use -DWITH_BDB=ON in order to get that functionality.
    • After I compile with cmake I get this warning (test_bitcoin): ld: warning: ignoring duplicate libraries: '../secp256k1/src/libsecp256k1.a'
  122. hebasto referenced this in commit 7c23041d05 on Aug 13, 2024
  123. fanquake commented at 10:52 am on August 14, 2024: member

    Is CMake meant to know about/be able to figure build-time dependencies?

    For example, on master, if I ./configure, and then call make check, make will compile and then run the tests; or, if I ./configure for macOS, and call make deploy, make will build bitcoin-qt, and then pacakge it.

    However, with CMake, it doesn’t seem to figure out what order to do things in. i.e it just tries to run the tests, without compiling them:

     0cmake -B build
     1cmake --build build --target test
     2Running tests...
     3Test project /bitcoin/build
     4Connected to MAKE jobserver
     5        Start   1: util_test_runner
     6  1/133 Test   [#1](/bitcoin-bitcoin/1/): util_test_runner .....................***Failed    0.13 sec
     7<snip>
     8	133 - walletload_tests (Not Run)
     9Errors while running CTest
    10Output from these tests are in: /bitcoin/build/Testing/Temporary/LastTest.log
    11Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
    

    and it tries to package bitcoin-qt, even though it hasn’t built it:

     0cmake -B build -DBUILD_GUI=ON
     1cmake --build build --target deploy     
     2[100%] Generating Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt
     3CMake Error at src/qt/cmake_install.cmake:41 (file):
     4  file INSTALL cannot find
     5  "/bitcoin/build/src/qt/bitcoin-qt": No such file or
     6  directory.
     7Call Stack (most recent call first):
     8  src/cmake_install.cmake:67 (include)
     9  cmake_install.cmake:52 (include)
    10
    11
    12gmake[3]: *** [CMakeFiles/deploy.dir/build.make:77: Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt] Error 1
    13gmake[2]: *** [CMakeFiles/Makefile2:606: CMakeFiles/deploy.dir/all] Error 2
    14gmake[1]: *** [CMakeFiles/Makefile2:613: CMakeFiles/deploy.dir/rule] Error 2
    15gmake: *** [Makefile:283: deploy] Error 2
    

    Are these just bugs with the implementation here, or an issue with CMake?

  124. hebasto commented at 11:31 am on August 14, 2024: member

    Are these just bugs with the implementation here, or an issue with CMake?

    That’s by CMake design.

    However, with CMake, it doesn’t seem to figure out what order to do things in. i.e it just tries to run the tests, without compiling them:

     0cmake -B build
     1cmake --build build --target test
     2Running tests...
     3Test project /bitcoin/build
     4Connected to MAKE jobserver
     5        Start   1: util_test_runner
     6  1/133 Test   [#1](/bitcoin-bitcoin/1/): util_test_runner .....................***Failed    0.13 sec
     7<snip>
     8	133 - walletload_tests (Not Run)
     9Errors while running CTest
    10Output from these tests are in: /bitcoin/build/Testing/Temporary/LastTest.log
    11Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
    

    The test target is generated by CMake. Such targets cannot be subject to top-level target dependencies.

    The CMake’s way to handle this is to use workflow presets.

  125. fanquake commented at 11:44 am on August 14, 2024: member

    The test target is generated by CMake. Such targets cannot be subject to top-level target dependencies.

    Ok. So what about deploy? If it’s a custom target of ours, it should know to build bitcoin-qt first?

  126. hebasto referenced this in commit a68171413e on Aug 15, 2024
  127. hebasto commented at 3:20 pm on August 15, 2024: member

    The test target is generated by CMake. Such targets cannot be subject to top-level target dependencies.

    Ok. So what about deploy? If it’s a custom target of ours, it should know to build bitcoin-qt first?

    Fixed in https://github.com/hebasto/bitcoin/pull/330.

  128. tdb3 commented at 1:33 am on August 16, 2024: contributor

    Built and ran tests successfully with 67b1e236334f38ec4e4d2251dbdfb790f20ed88b More details:

    Debian 12.6 cmake version 3.25.1 gcc/g++ (Debian 12.2.0-14) 12.2.0

    0cmake -B build
    1cmake --build build -j18
    2ctest --test-dir build
    

    Also sanity checked by running bitcoind, creating a wallet, and generating 101 blocks on regtest.

  129. maflcko added the label DrahtBot Guix build requested on Aug 16, 2024
  130. hebasto referenced this in commit 3c200d5d1a on Aug 16, 2024
  131. hebasto referenced this in commit 71384dae34 on Aug 16, 2024
  132. 0xB10C commented at 4:11 pm on August 16, 2024: contributor
    I tested with 67b1e236334f38ec4e4d2251dbdfb790f20ed88b that I’m able to do a nix build of the bitcoind (no GUI) and bitcoin (GUI) nix packages with the following modifications to the current nix package: https://github.com/0xB10C/nixpkgs/commit/fb77e0cf5194e48563e58eb418de8c2f9e0b48de. I also tested doing a development build with https://github.com/0xB10C/nix-bitcoin-core.
  133. cmake: Add root `CMakeLists.txt` file a2317e27b7
  134. cmake: Introduce interface libraries to encapsulate common flags
    Also add a sanity check for non-encapsulated (directory-wide) build
    properties.
    70683884c5
  135. cmake: Print compiler and linker flags in summary fe5cdace5f
  136. cmake: Add `config/bitcoin-config.h` support 27d687fc1f
  137. cmake: Add introspection module
    Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
    Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
    07069e2bb0
  138. cmake: Add position independent code support fd72d00ffe
  139. cmake: Add POSIX threads support 35cffc497d
  140. cmake: Add `TryAppendCXXFlags` module 4a0af29697
  141. cmake: Add `TryAppendLinkerFlag` module f98327931b
  142. cmake: Add global compiler and linker flags b6b5e732c8
  143. cmake: Redefine/adjust per-configuration flags cedfdf6c72
  144. cmake: Add `ccache` support dbb7ed14e8
  145. cmake: Build `secp256k1` subtree db7a198f29
  146. cmake: Build `crc32c` static library 51985c5304
  147. cmake: Build `leveldb` static library
    Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
    12bfbc8154
  148. cmake: Build `minisketch` static library 1f0a78edf3
  149. cmake: Generate `obj/build.h` header 752747fda8
  150. cmake: Build `univalue` static library 958971f476
  151. cmake: Build `bitcoin_crypto` library 0a9a521a70
  152. cmake: Build `bitcoin_util` static library 809a2f1929
  153. cmake: Build `bitcoin_consensus` library 3118e40c61
  154. cmake: Add `FindLibevent` module 97829ce2d5
  155. cmake: Build `bitcoind` executable a9813df826
  156. cmake: Build `bitcoin-cli` executable b27bf9700d
  157. cmake: Build `test_bitcoin` executable 959370bd76
  158. cmake: Create test suite for `ctest` ab2e99b0d9
  159. cmake: Add wallet functionality d10c5c34c3
  160. cmake: Build `bitcoin-tx` executable 027c6d7caa
  161. cmake: Build `bitcoin-util` executable e73e9304a1
  162. cmake: Add `libnatpmp` optional package support 6480e1dcdb
  163. cmake: Add `libminiupnpc` optional package support ae7b39a0e1
  164. cmake: Add `libzmq` optional package support d2fda82b49
  165. cmake: Add `systemtap-sdt` optional package support 353e0c9e96
  166. cmake: Add external signer support 801735163a
  167. cmake: Build `bench_bitcoin` executable 8bb0e85631
  168. cmake: Add `SANITIZERS` option 908530e312
  169. cmake: Add fuzzing options 3d85379570
  170. cmake: Add Python-based tests a8a2e364ac
  171. cmake: Add `HARDENING` option a01cb6e63f
  172. cmake: Add `REDUCE_EXPORTS` option c98d4a4c34
  173. cmake: Add `WERROR` option 30f642952c
  174. cmake: Build `bitcoin-qt` executable 57a6e2ef4a
  175. cmake: Add `libqrencode` optional package support 5bb5a4bc75
  176. cmake: Add `WITH_DBUS` option 10fcc668a3
  177. cmake: Build `test_bitcoin-qt` executable 975d67369b
  178. cmake: Build `bitcoinkernel` library
    Co-authored-by: TheCharlatan <seb.kung@gmail.com>
    aed38ea58c
  179. cmake: Build `bitcoin-chainstate` executable bb1a450dcb
  180. cmake: Add `MULTIPROCESS` option 90cec4d251
  181. depends: Amend handling flags environment variables
    If any of {C,CXX,CPP,LD}FLAGS is specified it should be assigned to
    a non-type-specific variable.
    6522af62af
  182. Revert "build, qt: Do not install *.prl files"
    This reverts commit 1155978d8f3fcc1cebf357302b933b834f9c9465.
    4a5208a81d
  183. depends: Rename `cmake_system` -> `cmake_system_name` 9b31209b4c
  184. depends: Add host-specific `cmake_system_version` variables 91a799247d
  185. build: Generate `toolchain.cmake` in depends 0d01c228a7
  186. cmake: Add cross-compiling support
    To configure CMake for cross-compiling, use
    `--toolchain depends/${HOST}/toolchain.cmake` command-line option.
    84ac35cfd4
  187. cmake: Implement `install` build target 973a3b0c5d
  188. cmake: Add `AddWindowsResources` module 2b43c45b13
  189. cmake: Add `APPEND_{CPP,C,CXX,LD}FLAGS` cache variables
    The content of those variables is appended to the each target after the
    flags added by the build system.
    1f60b30df0
  190. cmake: Add `Maintenance` module 747adb6ffe
  191. cmake: Migrate Guix build scripts to CMake e821f0a37a
  192. cmake: Add compiler diagnostic flags fb75ebbc33
  193. cmake: Add `docs` build target 65bdbc1ff2
  194. cmake: Add `Coverage` and `CoverageFuzz` scripts 8b6f1c4353
  195. cmake: Add vcpkg manifest file 7681746b20
  196. cmake: Add presets for native Windows builds 3885441ee0
  197. cmake, lint: Adjust `lint_includes_build_config` c360837ca5
  198. ci: Migrate CI scripts to CMake 9730288a0c
  199. doc: Update for CMake-based build system
    Co-authored-by: Lőrinc <pap.lorinc@gmail.com>
    Co-authored-by: pablomartin4btc <pablomartin4btc@gmail.com>
    6ce50fd9d0
  200. cmake: Ignore build subdirectories within source directory 41051290ab
  201. whitslack commented at 11:58 pm on August 16, 2024: contributor

    ACK 67b1e236334f38ec4e4d2251dbdfb790f20ed88b on Gentoo, modulo our usual hacks (now adapted to CMake) to allow linking against system-installed libsecp256k1 and LevelDB.

    One small gripe: When we enter the install phase, CMake always rebuilds src/clientversion.cpp, which forces a relink of all target executables. It would be better if ninja install would not rebuild anything.

  202. hebasto force-pushed on Aug 17, 2024
  203. hebasto commented at 8:10 am on August 17, 2024: member

    Rebased due to the conflicts. This PR branch is the current staging branch with the top 3 commits dropped.

    Some feedback has been addressed including:

  204. hebasto commented at 8:14 am on August 17, 2024: member

    @whitslack

    ACK 67b1e23 on Gentoo, modulo our usual hacks (now adapted to CMake) to allow linking against system-installed libsecp256k1 and LevelDB.

    Thanks for testing!

    One small gripe: When we enter the install phase, CMake always rebuilds src/clientversion.cpp, which forces a relink of all target executables. It would be better if ninja install would not rebuild anything.

    Is this still an issue with the recent update?

  205. hebasto commented at 8:29 am on August 17, 2024: member

    During the recent CMake Working Group call, it was agreed to freeze this branch until it is merged, unless a severe bug is reported or a merge conflict arises. All other reported issues will be addressed in follow-up PRs.

    Please continue testing this branch as thoroughly as possible.

  206. DrahtBot removed the label Needs rebase on Aug 17, 2024
  207. DrahtBot commented at 6:37 pm on August 17, 2024: contributor

    Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

    File commit ee367170cb2acf82b6ff8e0ccdbc1cce09730662(master) commit 3fbd65d8a8b0e781695628b08e166e73cc220956(master and this pull)
    SHA256SUMS.part 1c825f8e2152bd0e... 41737d558ce3e975...
    *-aarch64-linux-gnu-debug.tar.gz d9e2695d203aaf59... d96dda153880b989...
    *-aarch64-linux-gnu.tar.gz 08f591ff3379fe80... bcd4b94dcafb3722...
    *-arm-linux-gnueabihf-debug.tar.gz cb8171218c609a92... 52ba24dda03de328...
    *-arm-linux-gnueabihf.tar.gz 9905ff4c3e391749... 49d7901246e8e565...
    *-arm64-apple-darwin-unsigned.tar.gz 5914d325ce72cb8d... 002bb169db369d17...
    *-arm64-apple-darwin-unsigned.zip 8eeefe55c183da64... d7a57a8e5d709385...
    *-arm64-apple-darwin.tar.gz 217ccf7afa3c0eff... d413382a40108827...
    *-powerpc64-linux-gnu-debug.tar.gz 17a38cf70f9cc9cb... 6c32703a2bbf8581...
    *-powerpc64-linux-gnu.tar.gz 467c005ca4e985de... 916849c7d92bfb9c...
    *-riscv64-linux-gnu-debug.tar.gz 62d8366b2afc8bd4... 1b7a92467ac64d94...
    *-riscv64-linux-gnu.tar.gz c554640dea8aedee... c8e94f1d45145c26...
    *-x86_64-apple-darwin-unsigned.tar.gz 6ed41ffac060ec0a... 59d905fb430a38eb...
    *-x86_64-apple-darwin-unsigned.zip 92f80f305d144359... e3df80979cdb49e0...
    *-x86_64-apple-darwin.tar.gz 8db84bfbfbec5291... a2706374dc7c2fae...
    *-x86_64-linux-gnu-debug.tar.gz 40e0245ce7589be1... 30fce2e13664d578...
    *-x86_64-linux-gnu.tar.gz 1fac143a58c167a4... 7adef3f26bc16bae...
    *.tar.gz 9f3f71576dc99a8e... e7bc0f26ce908cbe...
    guix_build.log 1042c7ad0d54505a... 899ca7fcf714d8be...
    guix_build.log.diff 1347312935a4b9d2...
  208. DrahtBot removed the label DrahtBot Guix build requested on Aug 17, 2024
  209. sipsorcery commented at 9:03 pm on August 17, 2024: member

    I’m doing some test builds on Windows and it was a bit of a treasure hunt to find the binaries. The relevant doc states “CMake will put the resulting object files, libraries, and executables into a dedicated build directory.”. Would it be worth listing the directories?

    The Autotools Linux based builds put the binaries into a single obvious location. If that’s tricky with cmake then maybe the next best thing is to list the locations where the binaries can be found?

    Shouldn’t this PR also remove the build_msvc directory and related doc?

  210. hebasto commented at 8:50 am on August 18, 2024: member

    Shouldn’t this PR also remove the build_msvc directory and related doc?

    This is supposed to be done in a follow-up PR (see https://github.com/hebasto/bitcoin/pull/166 and #30664).

  211. hebasto commented at 8:56 am on August 18, 2024: member

    I’m doing some test builds on Windows and it was a bit of a treasure hunt to find the binaries. The relevant doc states “CMake will put the resulting object files, libraries, and executables into a dedicated build directory.”. Would it be worth listing the directories?

    The Autotools Linux based builds put the binaries into a single obvious location. If that’s tricky with cmake then maybe the next best thing is to list the locations where the binaries can be found?

    A solution similar to that in https://github.com/bitcoin-core/secp256k1/pull/1553 could be useful.

  212. in doc/build-windows-msvc.md:65 in 41051290ab
    60+
    61+```
    62+cmake -B build --preset vs2022-static          # It might take a while if the vcpkg binary cache is unpopulated or invalidated.
    63+cmake --build build --config Release           # Use "-j N" for N parallel jobs.
    64+ctest --test-dir build --build-config Release  # Use "-j N" for N parallel tests. Some tests are disabled if Python 3 is not available.
    65+cmake --install build --config Release         # Optional.
    


    sipsorcery commented at 10:26 am on August 18, 2024:
    It would be useful to know what the install command will do.

    hebasto commented at 1:37 pm on August 18, 2024:
    It installs the built project to the default install prefix, which is the value of the CMAKE_INSTALL_PREFIX variable.

    whitslack commented at 7:29 pm on August 18, 2024:

    I’m doing some test builds on Windows and it was a bit of a treasure hunt to find the binaries. @sipsorcery: Rather than going on a treasure hunt, wouldn’t you “install” the built product to a staging directory and then produce your distributable package from there? That way you’ll pick up all the bits that the build system wants you to have and none that it doesn’t.


    sipsorcery commented at 8:10 pm on August 18, 2024:

    @sipsorcery: Rather than going on a treasure hunt, wouldn’t you “install” the built product to a staging directory and then produce your distributable package from there? That way you’ll pick up all the bits that the build system wants you to have and none that it doesn’t.

    I think that would be a reasonable soln. I’m used to the way msvc places all the executables in a single directory. GIven the cmake --install command provides an option to achieve the same that would work.

    As mentioned in another comment I think the cmake --install explanation should be enhanced to assist those that use it infrequently, like me.

  213. bitcoin deleted a comment on Aug 18, 2024
  214. whitslack commented at 7:35 pm on August 18, 2024: contributor

    One small gripe: When we enter the install phase, CMake always rebuilds src/clientversion.cpp, which forces a relink of all target executables. It would be better if ninja install would not rebuild anything.

    Is this still an issue with the recent update? @hebasto: Nope! It works as expected at 41051290ab3b6c36312cec26a27f787cea9961b4. :grin: Thanks!

  215. bitcoin deleted a comment on Aug 18, 2024
  216. bitcoin deleted a comment on Aug 18, 2024
  217. vasild approved
  218. vasild commented at 8:52 am on August 19, 2024: contributor

    ACK 41051290ab3b6c36312cec26a27f787cea9961b4

    I did not review this entirely and thoroughly like I do for other pull requests. The development that lead to this happened in https://github.com/hebasto/bitcoin/pulls where every change that is in this PR was reviewed by me and/or other reviewers. In this respect, this is similar to master where I did not review all changes, but it is possible to check that all changes were properly reviewed (by me and/or other reviewers).

    This PR probably has some room for improvement and polishing, but I do not see any blockers to merging it. We have ~6 months for the polishing before 29.0 is released.

    compile:

     0--- compile_autotools	2024-08-19 10:34:32.383681000 +0200
     1+++ compile_cmake	2024-08-19 10:29:16.344723000 +0200
     2@@ -3,26 +3,17 @@
     3 -DDEBUG
     4 -DDEBUG_LOCKCONTENTION
     5 -DDEBUG_LOCKORDER
     6--DHAVE_BUILD_INFO
     7--DHAVE_CONFIG_H
     8--DPROVIDE_FUZZ_MAIN_FUNCTION
     9 -DRPC_DOC_CHECK
    10 -D_THREAD_SAFE
    11--I.
    12--I.
    13--I../src/config
    14--I./leveldb/include
    15--I./minisketch/include
    16--I./secp256k1/include
    17--I./univalue/include
    18--I/usr/local/include
    19--I/usr/local/include
    20+-I/SOURCE/bitcoin/bitcoin/src
    21+-I/SOURCE/bitcoin/bitcoin/src/leveldb/include
    22+-I/SOURCE/bitcoin/bitcoin/src/minisketch/include
    23+-I/SOURCE/bitcoin/bitcoin/src/univalue/include
    24+-I/BUILD/src
    25 -MD
    26--MF .deps/libbitcoin_node_a-net.Tpo
    27--MP
    28--MT libbitcoin_node_a-net.o
    29+-MF src/CMakeFiles/bitcoin_node.dir/net.cpp.o.d
    30+-MT src/CMakeFiles/bitcoin_node.dir/net.cpp.o
    31 -O0
    32--O2
    33 -Wall
    34 -Wconditional-uninitialized
    35 -Wdate-time
    36@@ -47,16 +38,15 @@
    37 -Wunused-member-function
    38 -Wvla
    39 -c
    40--fPIE
    41+-fPIC
    42 -fcf-protection=full
    43+-fcolor-diagnostics
    44 -fstack-clash-protection
    45 -fstack-protector-all
    46 -ftrapv
    47--g
    48 -g3
    49 -isystem /usr/local/include
    50--isystem /usr/local/include
    51--o libbitcoin_node_a-net.o
    52+-o src/CMakeFiles/bitcoin_node.dir/net.cpp.o
    53 -pthread
    54 -std=c++20
    55-`test -f 'net.cpp' || echo './'`net.cpp
    56+/SOURCE/bitcoin/bitcoin/src/net.cpp
    

    link:

     0--- link_autotools	2024-08-19 10:28:09.549987000 +0200
     1+++ link_cmake	2024-08-19 10:16:01.841351000 +0200
     2@@ -1,71 +1,33 @@
     3--L/usr/local/lib
     4--L/usr/local/lib
     5--L/usr/local/lib
     6--L/usr/local/lib
     7--L/usr/local/lib
     8 -O0
     9--O2
    10--Wall
    11--Wconditional-uninitialized
    12--Wdate-time
    13--Wdocumentation
    14--Werror
    15--Wextra
    16--Wformat
    17--Wformat-security
    18--Wgnu
    19--Wimplicit-fallthrough
    20 -Wl,-z,now
    21 -Wl,-z,relro
    22 -Wl,-z,separate-code
    23--Wloop-analysis
    24--Wmissing-field-initializers
    25--Wno-unused-parameter
    26--Woverloaded-virtual
    27--Wredundant-decls
    28--Wself-assign
    29--Wshadow-field
    30--Wstack-protector
    31--Wsuggest-override
    32--Wthread-safety
    33--Wundef
    34--Wunreachable-code
    35--Wunused-member-function
    36--Wvla
    37 -fPIE
    38 -fcf-protection=full
    39 -fstack-clash-protection
    40 -fstack-protector-all
    41 -ftrapv
    42--g
    43 -g3
    44--levent
    45--levent
    46--levent_pthreads
    47--lminiupnpc
    48--lnatpmp
    49--lpthread
    50--lsqlite3
    51--lzmq
    52--o bitcoind
    53+-o src/bitcoind
    54 -pie
    55 -pthread
    56--std=c++20
    57-bitcoind-bitcoind.o
    58-crc32c/libcrc32c.la
    59-crc32c/libcrc32c_sse42.la
    60-crypto/libbitcoin_crypto_avx2.la
    61-crypto/libbitcoin_crypto_base.la
    62-crypto/libbitcoin_crypto_sse41.la
    63-crypto/libbitcoin_crypto_x86_shani.la
    64-init/bitcoind-bitcoind.o
    65-leveldb/libleveldb.la
    66-leveldb/libmemenv.la
    67-libbitcoin_common.a
    68-libbitcoin_consensus.a
    69-libbitcoin_node.a
    70-libbitcoin_util.a
    71-libbitcoin_wallet.a
    72-libbitcoin_zmq.a
    73-libunivalue.la
    74-secp256k1/libsecp256k1.la
    75+/usr/local/lib/libevent.so
    76+/usr/local/lib/libevent_pthreads.so
    77+/usr/local/lib/libsqlite3.so
    78+libcrc32c.a
    79+libcrc32c_sse42.a
    80+libleveldb.a
    81+libminisketch.a
    82+src/CMakeFiles/bitcoind.dir/bitcoind.cpp.o
    83+src/CMakeFiles/bitcoind.dir/init/bitcoind.cpp.o
    84+src/crypto/libbitcoin_crypto.a
    85+src/crypto/libbitcoin_crypto_avx2.a
    86+src/crypto/libbitcoin_crypto_sse41.a
    87+src/crypto/libbitcoin_crypto_x86_shani.a
    88+src/libbitcoin_common.a
    89+src/libbitcoin_consensus.a
    90+src/libbitcoin_node.a
    91+src/secp256k1/src/libsecp256k1.a
    92+src/univalue/libunivalue.a
    93+src/util/libbitcoin_util.a
    94+src/wallet/libbitcoin_wallet.a
    

    Verdict: autotools is using -O2 -O0 and -g3 -g… lets get rid of it :pray:

    PS I do not know what is the -fPIE vs -fPIC difference.

  219. DrahtBot requested review from whitslack on Aug 19, 2024
  220. DrahtBot requested review from pablomartin4btc on Aug 19, 2024
  221. in CMakeLists.txt:16 in a2317e27b7 outdated
    11+if(POLICY CMP0141)
    12+  # MSVC debug information format flags are selected by an abstraction.
    13+  # We want to use the CMAKE_MSVC_DEBUG_INFORMATION_FORMAT variable
    14+  # to select the MSVC debug information format.
    15+  cmake_policy(SET CMP0141 NEW)
    16+endif()
    


    maflcko commented at 11:40 am on August 19, 2024:

    nit in https://github.com/bitcoin/bitcoin/commit/a2317e27b7fb86df4e32cd1674c06e09cb808248: Maybe add a comment that this can be removed after cmake_minimum_required(VERSION 3.25)?

    Alternatively, given that “The default installation of the latest version of MSVC 17.10.4 includes both CMake 3.28.3 and the vcpkg package manager”, it may be easier to just require cmake >= 3.25 on MSVC and enforce the policy this way?


    hebasto commented at 3:10 am on September 4, 2024:

    nit in a2317e2: Maybe add a comment that this can be removed after cmake_minimum_required(VERSION 3.25)?

    It seems redundant to me. Since policies are tied to specific versions, it’s generally understood that one should review every if(CMAKE_VERSION...) and if(POLICY...) statement in the code when updating the minimum required version.

    Alternatively, given that “The default installation of the latest version of MSVC 17.10.4 includes both CMake 3.28.3 and the vcpkg package manager”, it may be easier to just require cmake >= 3.25 on MSVC and enforce the policy this way?

    The build system becomes aware that it is building on MSVC only after the project() call, which makes such an implementation unfeasible given modern CMake requirements (here and here).

    Besides, the current implementation is idiomatic for enforcing individual policies.

    The other alternative is to specify cmake_minimum_required(VERSION 3.22...3.25), but this has a drawback, as the build system’s behavior depends on the actual CMake version.


    hebasto commented at 9:12 pm on September 6, 2024:

    Apparently, there is no need to enable the CMP0141 policy, as we have not used the CMAKE_MSVC_DEBUG_INFORMATION_FORMAT variable since https://github.com/hebasto/bitcoin/pull/215.

    Fixed in #30803.

  222. Sjors commented at 12:14 pm on August 19, 2024: member

    Concept ACK.

    In my testing of #30664 I keep seeing share/rpcauth/__pycache__/ in git status, so that might need to be added to .gitignore. Update: I can’t reproduce this anymore, so maybe is was a rebase issue.

  223. in cmake/module/TryAppendCXXFlags.cmake:137 in 4a0af29697 outdated
    132+endfunction()
    133+
    134+if(MSVC)
    135+  try_append_cxx_flags("/WX /options:strict" VAR working_compiler_werror_flag SKIP_LINK)
    136+else()
    137+  try_append_cxx_flags("-Werror" VAR working_compiler_werror_flag SKIP_LINK)
    


    maflcko commented at 10:47 am on August 20, 2024:

    nit in commit https://github.com/bitcoin/bitcoin/commit/4a0af29697b62d32af6f60d3ec70cd2ed4d7243c (“cmake: Add TryAppendCXXFlags module”):

    could it make sense to mention that IF_CHECK_FAILED is never applied to the linker flags, whereas IF_CHECK_PASSED seems to be? (IF_CHECK_FAILED is unused dead code, so nothing blocking)


    hebasto commented at 3:37 pm on September 8, 2024:
    The unused dead code has been deleted in #30803.
  224. in src/CMakeLists.txt:60 in 3118e40c61 outdated
    55@@ -56,3 +56,24 @@ endforeach()
    56 set(CMAKE_EXPORT_COMPILE_COMMANDS OFF)
    57 add_subdirectory(secp256k1)
    58 set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
    59+
    60+# Stable, backwards-compatible consensus functionality.
    


    maflcko commented at 9:55 am on August 21, 2024:
    nit in 3118e40c6157c8ab9e264518d1065d2b0fc07795: The comment seems wrong? This is an internal lib, neither stable nor “compatible”


    maflcko commented at 4:17 pm on September 3, 2024:
    Thanks for the reference. Looks like it was added in commit dc1e7ad7a5713d885f70ccc6c93e7a4c07e76559 by @ryanofsky. My preference would be to just drop the prefix “Stable, backwards-compatible”, because it is unclear whether it refers to the build system artefacts, or the consensus rules. In either case, the prefix seems either wrong or an approximation that lacks nuance.

    maflcko commented at 5:08 pm on September 3, 2024:
  225. in CMakeLists.txt:125 in d2fda82b49 outdated
    120+if(WITH_ZMQ)
    121+  if(VCPKG_TARGET_TRIPLET)
    122+    find_package(ZeroMQ CONFIG REQUIRED)
    123+  else()
    124+    # The ZeroMQ project has provided config files since v4.2.2.
    125+    # TODO: Switch to find_package(ZeroMQ) at some point in the future.
    


    maflcko commented at 2:42 pm on August 21, 2024:
    d2fda82b4954f4af7e7d340cf42b9cb34d96cde1: 4.2.2 Looks ancient, so maybe it can be dropped already?

    hebasto commented at 3:55 pm on September 3, 2024:
    Unfortunately, packages in the main distros still lack CMake configuration files. For example, see https://packages.debian.org/sid/amd64/libzmq3-dev/filelist.

    maflcko commented at 4:19 pm on September 3, 2024:
    Thanks. Makes sense. I guess the comment should be adjusted to say that this won’t be possible to do any time soon, or at all, for the reason given by you?

    hebasto commented at 5:23 pm on September 3, 2024:
    Fixed in #30803.
  226. in CMakeLists.txt:145 in 3d85379570 outdated
    140@@ -141,6 +141,8 @@ endif()
    141 cmake_dependent_option(ENABLE_EXTERNAL_SIGNER "Enable external signer support." ON "NOT WIN32" OFF)
    142 
    143 option(BUILD_BENCH "Build bench_bitcoin executable." OFF)
    144+option(BUILD_FUZZ_BINARY "Build fuzz binary." OFF)
    145+cmake_dependent_option(BUILD_FOR_FUZZING "Build for fuzzing. Enabling this will disable all other targets and override BUILD_FUZZ_BINARY." OFF "NOT MSVC" OFF)
    


    maflcko commented at 9:21 am on August 22, 2024:
    3d853795707c5a1828dcd09c1f68bb07dee472cd: Why does this need special handling for msvc? What problem is this trying to solve?

    hebasto commented at 3:45 pm on September 3, 2024:
    Lack of fuzzing engines for MSVC?

    maflcko commented at 4:08 pm on September 3, 2024:

    BUILD_FOR_FUZZING has nothing to do with fuzz engines. According to the documentation it is an option to “Build for fuzzing. Enabling this will disable all other targets and override BUILD_FUZZ_BINARY.”

    Can you explain this a bit better? Maybe with an example output of a msvc compile error or otherwise steps to reproduce?


    hebasto commented at 10:01 am on September 8, 2024:

    BUILD_FOR_FUZZING has nothing to do with fuzz engines. According to the documentation it is an option to “Build for fuzzing. Enabling this will disable all other targets and override BUILD_FUZZ_BINARY.”

    You’re right, there’s no need for a special case for MSVC.


    hebasto commented at 10:04 am on September 8, 2024:
    Fixed in #30803.
  227. TheCharlatan approved
  228. TheCharlatan commented at 9:41 am on August 22, 2024: contributor

    ACK 41051290ab3b6c36312cec26a27f787cea9961b4

    I reviewed most of this and at least glanced over every pull request in hebasto’s repository since the project started review in his repository early last year. The change does contain some tradeoffs compared to the current build system, most of which are inherent to cmake and challenges that are also faced by other projects. However none of these are serious compared to the benefits that this migration will bring. I hope the better IDE integration cmake brings compared to autotools will boost productivity and make it easier for developers to onboard in the future.

    Guix builds (aarch64):

     0c4e0333106b259d1ab11066677fc32fba8dd380afc2f9c334d9eff2dccccba39  guix-build-41051290ab3b/output/aarch64-linux-gnu/SHA256SUMS.part
     1bfce0ebd120bd45576586691c46b47e66bb0031f7540772a924d4cb1c2602576  guix-build-41051290ab3b/output/aarch64-linux-gnu/bitcoin-41051290ab3b-aarch64-linux-gnu-debug.tar.gz
     2615aae82e503fc07ee1053649b3a45ad2f0dd9c02d5a85394f8ba33b3bd5be88  guix-build-41051290ab3b/output/aarch64-linux-gnu/bitcoin-41051290ab3b-aarch64-linux-gnu.tar.gz
     32a8ef06d7795513a3d53c43560c3810c94cbe1072a71780ab32ffbda53f7be70  guix-build-41051290ab3b/output/arm-linux-gnueabihf/SHA256SUMS.part
     4fe9b3740ddcf221ee6b2a16c1fcc51f0f1276226a6f0d5ede4ec9a5ec8630473  guix-build-41051290ab3b/output/arm-linux-gnueabihf/bitcoin-41051290ab3b-arm-linux-gnueabihf-debug.tar.gz
     5459f02e850925be6c7c7e04f2c5ce851838505c1fa3fb86ad796e3892b62b561  guix-build-41051290ab3b/output/arm-linux-gnueabihf/bitcoin-41051290ab3b-arm-linux-gnueabihf.tar.gz
     63c8fc1d8f4bbf87e7106dfcd6fb9ac89fdc98c47ccfd27f791e85396458acf6d  guix-build-41051290ab3b/output/arm64-apple-darwin/SHA256SUMS.part
     7db755f2f4321ed1ba930ffd73aaa6d3f30ce028c1cbcbc120e3a4f9729d44aeb  guix-build-41051290ab3b/output/arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin-unsigned.tar.gz
     87375d138f0351c9f41b25c02b3628894b2fdb8add8783952468d71afbf3fbb47  guix-build-41051290ab3b/output/arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin-unsigned.zip
     9152a84bd68011d9af6e30830f4da1a72ebee401160c1fefd46e9de0cae77c94a  guix-build-41051290ab3b/output/arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin.tar.gz
    1097c437130ae60c72400ba9738fe2ef03dbd7bc25e303185061b426ac8f0244ea  guix-build-41051290ab3b/output/dist-archive/bitcoin-41051290ab3b.tar.gz
    113f0961cf0912d9469c279c76987d81c217e755b7b2593fcf04171663f453ef1d  guix-build-41051290ab3b/output/powerpc64-linux-gnu/SHA256SUMS.part
    12c02e3d07355b36481a9c442085a2d3370d19031defa0c1831b39f443abc650d1  guix-build-41051290ab3b/output/powerpc64-linux-gnu/bitcoin-41051290ab3b-powerpc64-linux-gnu-debug.tar.gz
    136a1e7e6eedc15ed13be15c526c4c9145241e012cc7b23ecf23bbde455eb0e74e  guix-build-41051290ab3b/output/powerpc64-linux-gnu/bitcoin-41051290ab3b-powerpc64-linux-gnu.tar.gz
    14e99090bd1371547fae416e70fe0977ab8679e4d710446ee7b1df980618a728b6  guix-build-41051290ab3b/output/riscv64-linux-gnu/SHA256SUMS.part
    15d5bd95a0f718e72b01732e37de33a9e63ae26182e67d4458b47ca6051fc2ce9d  guix-build-41051290ab3b/output/riscv64-linux-gnu/bitcoin-41051290ab3b-riscv64-linux-gnu-debug.tar.gz
    16fbdd2ca094b9f69c17fdebedc9d790eab0cd58a2ce12833efec602d210a39a42  guix-build-41051290ab3b/output/riscv64-linux-gnu/bitcoin-41051290ab3b-riscv64-linux-gnu.tar.gz
    17aeeb2f5ab3a3197481f0c6c2aea1837026dc322b0066c229b365ba8747d829f2  guix-build-41051290ab3b/output/x86_64-apple-darwin/SHA256SUMS.part
    180a7214b6617da6f36cd632a9dc1edbdeda25a5d3049cd2b597634164d906abe8  guix-build-41051290ab3b/output/x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin-unsigned.tar.gz
    19c266669f9e663c7f35060adeba9148e8b5cda45376ae90ee917eb411d6890e46  guix-build-41051290ab3b/output/x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin-unsigned.zip
    20a52f9e74d790f71f8ec89a23338df518053dce6ad5c37338ba13ba4438a367cd  guix-build-41051290ab3b/output/x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin.tar.gz
    211c1dcc180e415c893a253315786397b6dbbed7a10b40fa9fc4b85f3ae102d370  guix-build-41051290ab3b/output/x86_64-linux-gnu/SHA256SUMS.part
    2291da98db1e4eea4738b839ed3b425dcf6ffdccd2177debe46314cceb785f0728  guix-build-41051290ab3b/output/x86_64-linux-gnu/bitcoin-41051290ab3b-x86_64-linux-gnu-debug.tar.gz
    23f8bc57976b26c4a378dc7a3be5e6f60de9e7b1d73fae6a2a96505801194f1420  guix-build-41051290ab3b/output/x86_64-linux-gnu/bitcoin-41051290ab3b-x86_64-linux-gnu.tar.gz
    24c7642ef6d7529c6c3b3d929de6172ab594ace611c3ab04b11a6c953bcd48823c  guix-build-41051290ab3b/output/x86_64-w64-mingw32/SHA256SUMS.part
    25572691a757abc27a32a37b0f10c84a361809f4de76fb4a39e39815ded836a26a  guix-build-41051290ab3b/output/x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-debug.zip
    26d02a1bc08fb97a2c4e726fabb3166293061834f0f32dd488a4238304cfaefe28  guix-build-41051290ab3b/output/x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-setup-unsigned.exe
    27963836e69f2382ccd48fbbf68146cd29a78e79b6784297c76b415824ed3e4175  guix-build-41051290ab3b/output/x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-unsigned.tar.gz
    28f42b645f14fd2d9fe88ce44e03f0b175fce4a703961e9ef5dd1f6cf4ad986de1  guix-build-41051290ab3b/output/x86_64-w64-mingw32/bitcoin-41051290ab3b-win64.zip
    
  229. DrahtBot requested review from Sjors on Aug 22, 2024
  230. maflcko requested review from maflcko on Aug 22, 2024
  231. in src/test/fuzz/CMakeLists.txt:54 in 41051290ab
    49+  flatfile.cpp
    50+  float.cpp
    51+  golomb_rice.cpp
    52+  headerssync.cpp
    53+  hex.cpp
    54+  http_request.cpp
    


    marcofleon commented at 11:31 am on August 22, 2024:
    i2p.cpp is missing here

    hebasto commented at 11:37 am on August 22, 2024:

    Thanks!

    I don’t consider this a blocker, so it will be fixed in a follow-up.


    vasild commented at 11:44 am on August 22, 2024:
    Is there a list of known things to be addressed in a followup? It would help us not to forget things and also should avoid people reporting duplicates.

    l0rinc commented at 11:49 am on August 22, 2024:
    we’re adding some follow-ups as PRs to https://github.com/hebasto/bitcoin/pulls

    hebasto commented at 12:00 pm on August 22, 2024:

    dergoegge commented at 1:02 pm on August 22, 2024:
    Comparing the output of PRINT_ALL_FUZZ_TARGETS_AND_ABORT=1 src/test/fuzz/fuzz for both autotools and cmake can be used to verify that all harnesses are ported.
  232. hebasto commented at 11:38 am on August 22, 2024: member

    My Guix build:

     0x86_64
     1c4e0333106b259d1ab11066677fc32fba8dd380afc2f9c334d9eff2dccccba39  guix-build-41051290ab3b/output/aarch64-linux-gnu/SHA256SUMS.part
     2bfce0ebd120bd45576586691c46b47e66bb0031f7540772a924d4cb1c2602576  guix-build-41051290ab3b/output/aarch64-linux-gnu/bitcoin-41051290ab3b-aarch64-linux-gnu-debug.tar.gz
     3615aae82e503fc07ee1053649b3a45ad2f0dd9c02d5a85394f8ba33b3bd5be88  guix-build-41051290ab3b/output/aarch64-linux-gnu/bitcoin-41051290ab3b-aarch64-linux-gnu.tar.gz
     42a8ef06d7795513a3d53c43560c3810c94cbe1072a71780ab32ffbda53f7be70  guix-build-41051290ab3b/output/arm-linux-gnueabihf/SHA256SUMS.part
     5fe9b3740ddcf221ee6b2a16c1fcc51f0f1276226a6f0d5ede4ec9a5ec8630473  guix-build-41051290ab3b/output/arm-linux-gnueabihf/bitcoin-41051290ab3b-arm-linux-gnueabihf-debug.tar.gz
     6459f02e850925be6c7c7e04f2c5ce851838505c1fa3fb86ad796e3892b62b561  guix-build-41051290ab3b/output/arm-linux-gnueabihf/bitcoin-41051290ab3b-arm-linux-gnueabihf.tar.gz
     73c8fc1d8f4bbf87e7106dfcd6fb9ac89fdc98c47ccfd27f791e85396458acf6d  guix-build-41051290ab3b/output/arm64-apple-darwin/SHA256SUMS.part
     8db755f2f4321ed1ba930ffd73aaa6d3f30ce028c1cbcbc120e3a4f9729d44aeb  guix-build-41051290ab3b/output/arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin-unsigned.tar.gz
     97375d138f0351c9f41b25c02b3628894b2fdb8add8783952468d71afbf3fbb47  guix-build-41051290ab3b/output/arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin-unsigned.zip
    10152a84bd68011d9af6e30830f4da1a72ebee401160c1fefd46e9de0cae77c94a  guix-build-41051290ab3b/output/arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin.tar.gz
    1197c437130ae60c72400ba9738fe2ef03dbd7bc25e303185061b426ac8f0244ea  guix-build-41051290ab3b/output/dist-archive/bitcoin-41051290ab3b.tar.gz
    123f0961cf0912d9469c279c76987d81c217e755b7b2593fcf04171663f453ef1d  guix-build-41051290ab3b/output/powerpc64-linux-gnu/SHA256SUMS.part
    13c02e3d07355b36481a9c442085a2d3370d19031defa0c1831b39f443abc650d1  guix-build-41051290ab3b/output/powerpc64-linux-gnu/bitcoin-41051290ab3b-powerpc64-linux-gnu-debug.tar.gz
    146a1e7e6eedc15ed13be15c526c4c9145241e012cc7b23ecf23bbde455eb0e74e  guix-build-41051290ab3b/output/powerpc64-linux-gnu/bitcoin-41051290ab3b-powerpc64-linux-gnu.tar.gz
    15e99090bd1371547fae416e70fe0977ab8679e4d710446ee7b1df980618a728b6  guix-build-41051290ab3b/output/riscv64-linux-gnu/SHA256SUMS.part
    16d5bd95a0f718e72b01732e37de33a9e63ae26182e67d4458b47ca6051fc2ce9d  guix-build-41051290ab3b/output/riscv64-linux-gnu/bitcoin-41051290ab3b-riscv64-linux-gnu-debug.tar.gz
    17fbdd2ca094b9f69c17fdebedc9d790eab0cd58a2ce12833efec602d210a39a42  guix-build-41051290ab3b/output/riscv64-linux-gnu/bitcoin-41051290ab3b-riscv64-linux-gnu.tar.gz
    18aeeb2f5ab3a3197481f0c6c2aea1837026dc322b0066c229b365ba8747d829f2  guix-build-41051290ab3b/output/x86_64-apple-darwin/SHA256SUMS.part
    190a7214b6617da6f36cd632a9dc1edbdeda25a5d3049cd2b597634164d906abe8  guix-build-41051290ab3b/output/x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin-unsigned.tar.gz
    20c266669f9e663c7f35060adeba9148e8b5cda45376ae90ee917eb411d6890e46  guix-build-41051290ab3b/output/x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin-unsigned.zip
    21a52f9e74d790f71f8ec89a23338df518053dce6ad5c37338ba13ba4438a367cd  guix-build-41051290ab3b/output/x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin.tar.gz
    221c1dcc180e415c893a253315786397b6dbbed7a10b40fa9fc4b85f3ae102d370  guix-build-41051290ab3b/output/x86_64-linux-gnu/SHA256SUMS.part
    2391da98db1e4eea4738b839ed3b425dcf6ffdccd2177debe46314cceb785f0728  guix-build-41051290ab3b/output/x86_64-linux-gnu/bitcoin-41051290ab3b-x86_64-linux-gnu-debug.tar.gz
    24f8bc57976b26c4a378dc7a3be5e6f60de9e7b1d73fae6a2a96505801194f1420  guix-build-41051290ab3b/output/x86_64-linux-gnu/bitcoin-41051290ab3b-x86_64-linux-gnu.tar.gz
    25c7642ef6d7529c6c3b3d929de6172ab594ace611c3ab04b11a6c953bcd48823c  guix-build-41051290ab3b/output/x86_64-w64-mingw32/SHA256SUMS.part
    26572691a757abc27a32a37b0f10c84a361809f4de76fb4a39e39815ded836a26a  guix-build-41051290ab3b/output/x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-debug.zip
    27d02a1bc08fb97a2c4e726fabb3166293061834f0f32dd488a4238304cfaefe28  guix-build-41051290ab3b/output/x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-setup-unsigned.exe
    28963836e69f2382ccd48fbbf68146cd29a78e79b6784297c76b415824ed3e4175  guix-build-41051290ab3b/output/x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-unsigned.tar.gz
    29f42b645f14fd2d9fe88ce44e03f0b175fce4a703961e9ef5dd1f6cf4ad986de1  guix-build-41051290ab3b/output/x86_64-w64-mingw32/bitcoin-41051290ab3b-win64.zip
    
  233. in cmake/module/Maintenance.cmake:45 in 747adb6ffe outdated
    40+    COMMAND ${CMAKE_COMMAND} -E env CXX=${cxx_compiler_command} CXXFLAGS=${CMAKE_CXX_FLAGS} LDFLAGS=${CMAKE_EXE_LINKER_FLAGS} ${PYTHON_COMMAND} ${PROJECT_SOURCE_DIR}/contrib/devtools/test-security-check.py TestSecurityChecks.test_${exe_format}
    41+    COMMAND ${CMAKE_COMMAND} -E env CXX=${cxx_compiler_command} CXXFLAGS=${CMAKE_CXX_FLAGS} LDFLAGS=${CMAKE_EXE_LINKER_FLAGS} ${PYTHON_COMMAND} ${PROJECT_SOURCE_DIR}/contrib/devtools/test-symbol-check.py TestSymbolChecks.test_${exe_format}
    42+    VERBATIM
    43+  )
    44+
    45+  foreach(target IN ITEMS bitcoind bitcoin-qt bitcoin-cli bitcoin-tx bitcoin-util bitcoin-wallet test_bitcoin bench_bitcoin)
    


    maflcko commented at 11:54 am on August 22, 2024:
    747adb6ffe9b06d476fc5eaebbaf9a62b91a78c5: missing bitcoin-node

    hebasto commented at 4:55 pm on September 3, 2024:
    This is used for the check-symbols and check-security targets, which are intended to be run in a Guix environment during the release process. Currently, we do not support the multiprocess feature in this workflow, right?
  234. Sjors commented at 11:59 am on August 22, 2024: member
    Getting the same guix hashes as @hebasto and x86_64. Briefly tested that the macOS zip still contains a working (x86_64) binary.
  235. in doc/build-windows.md:67 in 6ce50fd9d0 outdated
    75-    cd ..
    76-    ./autogen.sh
    77-    CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site ./configure --prefix=/
    78-    make # use "-j N" for N parallel jobs
    79-    sudo bash -c "echo 1 > /proc/sys/fs/binfmt_misc/status" # Enable WSL support for Win32 applications.
    80+    gmake -C depends HOST=x86_64-w64-mingw32  # Use "-j N" for N parallel jobs.
    


    maflcko commented at 6:31 pm on August 22, 2024:
    make, not gmake

    hebasto commented at 3:40 pm on September 3, 2024:

    gmake is an accepted alias for make when it refers to GNU Make, which is required to build depends.

    Also please refer to https://packages.ubuntu.com/noble/amd64/make/filelist.

  236. in doc/build-windows.md:70 in 6ce50fd9d0 outdated
    78-    make # use "-j N" for N parallel jobs
    79-    sudo bash -c "echo 1 > /proc/sys/fs/binfmt_misc/status" # Enable WSL support for Win32 applications.
    80+    gmake -C depends HOST=x86_64-w64-mingw32  # Use "-j N" for N parallel jobs.
    81+    cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
    82+    cmake --build build     # Use "-j N" for N parallel jobs.
    83+    ctest --test-dir build  # Use "-j N" for N parallel tests. Some tests are disabled if Python 3 is not available.
    


    maflcko commented at 6:32 pm on August 22, 2024:
    You can’t run exe on WSL without wine, can you?

    hebasto commented at 3:42 pm on September 3, 2024:
    I don’t recall installing Wine.

    fanquake commented at 3:56 pm on September 3, 2024:

    Regardless of WSL, given these are the general instructions for any Linux, wine should be added as a requirement, otherwise the current instructions will fail:

    0gmake -C depends HOST=x86_64-w64-mingw32 -j12 NO_QT=1 NO_WALLET=1 NO_ZMQ=1 NO_NATPMP=1 NO_UPNP=1
    1cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
    2cmake --build build
    3bitcoin# wine
    4bash: wine: command not found
    5bitcoin# wine64
    6bash: wine64: command not found
    7<snip>
    8ctest --test-dir build
    90% tests passed, 117 tests failed out of 117
    

    maflcko commented at 4:24 pm on September 3, 2024:
    Thanks for testing @fanquake. Just to clarify: I left the review comment, because running the tests was added as part of this pull request, even though it wasn’t present previously with autotools.

    hebasto commented at 3:16 am on September 4, 2024:

    I’d rather not include Wine installation instructions in this build doc. It is not a trivial process and could create an additional maintenance burden in the future.

    it wasn’t present previously with autotools.

    Let’s just remove it.


    hebasto commented at 7:09 am on September 4, 2024:
    Fixed in #30803.

    fanquake commented at 7:14 am on September 4, 2024:

    I’d rather not include Wine installation instructions in this build doc. It is not a trivial process and could create an additional maintenance burden in the future.

    Not sure I agree. Can you elaborate on this “burden”? Why is that better to avoid than running the tests?


    maflcko commented at 7:17 am on September 4, 2024:
    @fanquake I am not sure how useful it is to run the tests in the build environment on Linux on Wine, when the binaries itself are meant to be run on a completely different platform (Windows). It would be better to run the tests on the actual platform that will later run the binaries instead. If the docs don’t mention it, they can be adjusted to say so.

    hebasto commented at 7:26 am on September 4, 2024:

    Can you elaborate on this “burden”?

    Wine config creation? Need of the wine-binfmt package? People’s requests to fix their Wine installations?


    fanquake commented at 8:47 am on September 4, 2024:

    Seems like a better reason to not do this is that it doesn’t actually work properly with CTest? Did this work when you tested it previously? i.e:

     0ctest --test-dir build -R mempool_tests --rerun-failed --output-on-failure
     1Internal ctest changing into directory: /bitcoin/build
     2Test project /bitcoin/build
     3    Start 53: mempool_tests
     41/1 Test [#53](/bitcoin-bitcoin/53/): mempool_tests ....................***Failed    0.01 sec
     5/bitcoin/build/src/test/test_bitcoin.exe: 1: MZ????@???: not found
     6/bitcoin/build/src/test/test_bitcoin.exe: 2: Syntax error: ")" unexpected
     7
     8
     90% tests passed, 1 tests failed out of 1
    10
    11Total Test time (real) =   0.02 sec
    12
    13The following tests FAILED:
    14	 53 - mempool_tests (Failed)
    15Errors while running CTest
    

    If it’s expected that this is broken, then we should probably document that, otherwise we might end up with issues opened about non-functioning unit tests.

    When run directly under wine, the unit tests run, but also have issues:

     0bitcoin# wine /bitcoin/build/src/test/test_bitcoin.exe       
     10044:err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded.
     20044:err:winediag:nodrv_CreateWindow L"The explorer process failed to start."
     30044:err:systray:initialize_systray Could not create tray window
     4Running 563 test cases...
     5/bitcoin/src/test/fs_tests.cpp(61): error: in "fs_tests/fsbridge_fstream": check input_buffer == "bitcoin" has failed [ != bitcoin]
     6/bitcoin/src/test/fs_tests.cpp(77): error: in "fs_tests/fsbridge_fstream": check input_buffer == "bitcointests" has failed [ != bitcointests]
     7/bitcoin/src/test/fs_tests.cpp(87): error: in "fs_tests/fsbridge_fstream": check input_buffer == "bitcoin" has failed [ != bitcoin]
     8unknown location(0): fatal error: in "dbwrapper_tests/unicodepath": std::filesystem::__cxx11::filesystem_error: filesystem error: cannot create directories: No such file or directory [C:\users\root\Temp
     9\test_common_Bitcoin Core\110aa87c039e797f10589751c6d5c303ef8cbe847c202a40f7b7188f48aedf19\test_runner_a??_????_20191128_104644]
    10/bitcoin/src/test/dbwrapper_tests.cpp(389): last checkpoint: "unicodepath" test entry
    11
    12*** 4 failures are detected in the test module "Bitcoin Core Test Suite"
    

    maflcko commented at 9:05 am on September 4, 2024:

    I don’t recall when wine was ever working. There was always a a number of intermittent or consistent errors, regardless of cmake, see #29881 (review)

    Fixing that seems unrelated to the cmake transition.

    I think simply removing the accidentally added line is fine and other follow-ups can be done in a follow-up, if they are needed.


    maflcko commented at 7:25 am on September 5, 2024:
    0*** 4 failures are detected in the test module "Bitcoin Core Test Suite"
    

    Does export LC_ALL=C.UTF-8 make any difference? This is set in the CI, where it “works” before and after the cmake migration. Maybe a separate issue can be created, as it seems unrelated to cmake?


    fanquake commented at 9:28 am on September 5, 2024:

    Does export LC_ALL=C.UTF-8 make any difference?

    Yes, this seems to fix the failures when running directly under wine. Can followup with an issues/docs.


    maflcko commented at 9:34 am on September 5, 2024:
    Longer term, I wonder if it makes sense for the build system (and guix) to produce a “test kit” that contains the test config ini file required to run the tests (unit and functional ones) as well as other required stuff, so that on native Windows, one can simply move the exe files to a folder inside the “test kit” and then run the kit to execute all tests.

    hebasto commented at 11:05 am on September 8, 2024:

    Seems like a better reason to not do this is that it doesn’t actually work properly with CTest? Did this work when you tested it previously? i.e:

     0ctest --test-dir build -R mempool_tests --rerun-failed --output-on-failure
     1Internal ctest changing into directory: /bitcoin/build
     2Test project /bitcoin/build
     3    Start 53: mempool_tests
     41/1 Test [#53](/bitcoin-bitcoin/53/): mempool_tests ....................***Failed    0.01 sec
     5/bitcoin/build/src/test/test_bitcoin.exe: 1: MZ????@???: not found
     6/bitcoin/build/src/test/test_bitcoin.exe: 2: Syntax error: ")" unexpected
     7
     8
     90% tests passed, 1 tests failed out of 1
    10
    11Total Test time (real) =   0.02 sec
    12
    13The following tests FAILED:
    14	 53 - mempool_tests (Failed)
    15Errors while running CTest
    

    From https://cirrus-ci.com/task/6467960875778048 for the latest https://github.com/bitcoin/bitcoin/commit/a5fa90706addb3ebc4325b139cfebcf9d7ac3045:

    0 49/135 Test  [#54](/bitcoin-bitcoin/54/): mempool_tests ........................   Passed    2.92 sec
    

    hebasto commented at 1:22 pm on September 8, 2024:

    On my Ubuntu 24.04 system all test pass.

      0$ make -C depends -j 16 HOST=x86_64-w64-mingw32 NO_QT=1
      1$ cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
      2$ cmake --build build -j 16
      3$ ctest --test-dir build -j 16
      4Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
      5Test project /home/hebasto/git/bitcoin/build
      6        Start   1: util_test_runner
      7        Start   2: util_rpcauth_test
      8        Start   3: univalue_test
      9        Start   4: univalue_object_test
     10        Start   5: noverify_tests
     11        Start   6: tests
     12        Start   7: exhaustive_tests
     13        Start   8: addrman_tests
     14        Start   9: allocator_tests
     15        Start  10: amount_tests
     16        Start  11: argsman_tests
     17        Start  12: arith_uint256_tests
     18        Start  13: banman_tests
     19        Start  14: base32_tests
     20        Start  15: base58_tests
     21        Start  16: base64_tests
     22  1/134 Test   [#2](/bitcoin-bitcoin/2/): util_rpcauth_test ....................   Passed    0.06 sec
     23        Start  17: bech32_tests
     24  2/134 Test   [#4](/bitcoin-bitcoin/4/): univalue_object_test .................   Passed    0.67 sec
     25        Start  18: bip32_tests
     26  3/134 Test  [#10](/bitcoin-bitcoin/10/): amount_tests .........................   Passed    1.03 sec
     27        Start  19: bip324_tests
     28  4/134 Test  [#14](/bitcoin-bitcoin/14/): base32_tests .........................   Passed    1.03 sec
     29        Start  20: blockchain_tests
     30  5/134 Test  [#12](/bitcoin-bitcoin/12/): arith_uint256_tests ..................   Passed    1.03 sec
     31        Start  21: blockencodings_tests
     32  6/134 Test   [#9](/bitcoin-bitcoin/9/): allocator_tests ......................   Passed    1.03 sec
     33        Start  22: blockfilter_index_tests
     34  7/134 Test  [#17](/bitcoin-bitcoin/17/): bech32_tests .........................   Passed    0.98 sec
     35        Start  23: blockfilter_tests
     36  8/134 Test  [#13](/bitcoin-bitcoin/13/): banman_tests .........................   Passed    1.27 sec
     37        Start  24: blockmanager_tests
     38  9/134 Test  [#15](/bitcoin-bitcoin/15/): base58_tests .........................   Passed    1.37 sec
     39        Start  25: bloom_tests
     40 10/134 Test  [#18](/bitcoin-bitcoin/18/): bip32_tests ..........................   Passed    0.91 sec
     41        Start  26: bswap_tests
     42 11/134 Test   [#8](/bitcoin-bitcoin/8/): addrman_tests ........................   Passed    1.58 sec
     43        Start  27: checkqueue_tests
     44 12/134 Test  [#23](/bitcoin-bitcoin/23/): blockfilter_tests ....................   Passed    0.79 sec
     45        Start  28: cluster_linearize_tests
     46 13/134 Test  [#20](/bitcoin-bitcoin/20/): blockchain_tests .....................   Passed    0.94 sec
     47        Start  29: coins_tests
     48 14/134 Test  [#21](/bitcoin-bitcoin/21/): blockencodings_tests .................   Passed    1.01 sec
     49        Start  30: coinscachepair_tests
     50 15/134 Test  [#25](/bitcoin-bitcoin/25/): bloom_tests ..........................   Passed    0.74 sec
     51        Start  31: coinstatsindex_tests
     52 16/134 Test  [#26](/bitcoin-bitcoin/26/): bswap_tests ..........................   Passed    0.55 sec
     53        Start  32: common_url_tests
     54 17/134 Test  [#22](/bitcoin-bitcoin/22/): blockfilter_index_tests ..............   Passed    1.16 sec
     55        Start  33: compilerbug_tests
     56 18/134 Test  [#11](/bitcoin-bitcoin/11/): argsman_tests ........................   Passed    2.26 sec
     57        Start  34: compress_tests
     58 19/134 Test  [#24](/bitcoin-bitcoin/24/): blockmanager_tests ...................   Passed    1.12 sec
     59        Start  35: crypto_tests
     60 20/134 Test  [#28](/bitcoin-bitcoin/28/): cluster_linearize_tests ..............   Passed    0.83 sec
     61        Start  36: cuckoocache_tests
     62 21/134 Test  [#30](/bitcoin-bitcoin/30/): coinscachepair_tests .................   Passed    0.67 sec
     63        Start  37: dbwrapper_tests
     64 22/134 Test  [#19](/bitcoin-bitcoin/19/): bip324_tests .........................   Passed    1.69 sec
     65        Start  38: denialofservice_tests
     66 23/134 Test  [#32](/bitcoin-bitcoin/32/): common_url_tests .....................   Passed    0.70 sec
     67        Start  39: descriptor_tests
     68 24/134 Test  [#33](/bitcoin-bitcoin/33/): compilerbug_tests ....................   Passed    0.68 sec
     69        Start  40: disconnected_transactions
     70 25/134 Test  [#34](/bitcoin-bitcoin/34/): compress_tests .......................   Passed    1.09 sec
     71        Start  41: feefrac_tests
     72 26/134 Test  [#37](/bitcoin-bitcoin/37/): dbwrapper_tests ......................   Passed    0.81 sec
     73        Start  42: flatfile_tests
     74 27/134 Test  [#38](/bitcoin-bitcoin/38/): denialofservice_tests ................   Passed    0.87 sec
     75        Start  43: fs_tests
     76 28/134 Test  [#40](/bitcoin-bitcoin/40/): disconnected_transactions ............   Passed    0.80 sec
     77        Start  44: getarg_tests
     78 29/134 Test  [#41](/bitcoin-bitcoin/41/): feefrac_tests ........................   Passed    0.55 sec
     79        Start  45: hash_tests
     80 30/134 Test  [#39](/bitcoin-bitcoin/39/): descriptor_tests .....................   Passed    1.15 sec
     81        Start  46: headers_sync_chainwork_tests
     82 31/134 Test  [#36](/bitcoin-bitcoin/36/): cuckoocache_tests ....................   Passed    1.52 sec
     83        Start  47: httpserver_tests
     84 32/134 Test  [#27](/bitcoin-bitcoin/27/): checkqueue_tests .....................   Passed    2.63 sec
     85        Start  48: i2p_tests
     86 33/134 Test  [#42](/bitcoin-bitcoin/42/): flatfile_tests .......................   Passed    0.75 sec
     87        Start  49: interfaces_tests
     88 34/134 Test  [#43](/bitcoin-bitcoin/43/): fs_tests .............................   Passed    0.75 sec
     89        Start  50: key_io_tests
     90 35/134 Test  [#44](/bitcoin-bitcoin/44/): getarg_tests .........................   Passed    0.71 sec
     91        Start  51: key_tests
     92 36/134 Test  [#45](/bitcoin-bitcoin/45/): hash_tests ...........................   Passed    0.73 sec
     93        Start  52: logging_tests
     94 37/134 Test  [#46](/bitcoin-bitcoin/46/): headers_sync_chainwork_tests .........   Passed    0.83 sec
     95        Start  53: mempool_tests
     96 38/134 Test  [#48](/bitcoin-bitcoin/48/): i2p_tests ............................   Passed    0.69 sec
     97        Start  54: merkle_tests
     98 39/134 Test  [#47](/bitcoin-bitcoin/47/): httpserver_tests .....................   Passed    0.73 sec
     99        Start  55: merkleblock_tests
    100 40/134 Test  [#50](/bitcoin-bitcoin/50/): key_io_tests .........................   Passed    0.76 sec
    101        Start  56: miner_tests
    102 41/134 Test  [#35](/bitcoin-bitcoin/35/): crypto_tests .........................   Passed    2.86 sec
    103        Start  57: miniminer_tests
    104 42/134 Test  [#51](/bitcoin-bitcoin/51/): key_tests ............................   Passed    0.92 sec
    105        Start  58: miniscript_tests
    106 43/134 Test  [#52](/bitcoin-bitcoin/52/): logging_tests ........................   Passed    0.77 sec
    107        Start  59: minisketch_tests
    108 44/134 Test  [#55](/bitcoin-bitcoin/55/): merkleblock_tests ....................   Passed    0.57 sec
    109        Start  60: multisig_tests
    110 45/134 Test  [#31](/bitcoin-bitcoin/31/): coinstatsindex_tests .................   Passed    3.39 sec
    111        Start  61: net_peer_connection_tests
    112 46/134 Test  [#53](/bitcoin-bitcoin/53/): mempool_tests ........................   Passed    0.93 sec
    113        Start  62: net_peer_eviction_tests
    114 47/134 Test  [#49](/bitcoin-bitcoin/49/): interfaces_tests .....................   Passed    1.78 sec
    115        Start  63: net_tests
    116 48/134 Test  [#54](/bitcoin-bitcoin/54/): merkle_tests .........................   Passed    1.21 sec
    117        Start  64: netbase_tests
    118 49/134 Test  [#59](/bitcoin-bitcoin/59/): minisketch_tests .....................   Passed    0.77 sec
    119        Start  65: node_warnings_tests
    120 50/134 Test  [#61](/bitcoin-bitcoin/61/): net_peer_connection_tests ............   Passed    0.73 sec
    121        Start  66: orphanage_tests
    122 51/134 Test  [#60](/bitcoin-bitcoin/60/): multisig_tests .......................   Passed    0.75 sec
    123        Start  67: peerman_tests
    124 52/134 Test  [#62](/bitcoin-bitcoin/62/): net_peer_eviction_tests ..............   Passed    0.78 sec
    125        Start  68: pmt_tests
    126 53/134 Test  [#64](/bitcoin-bitcoin/64/): netbase_tests ........................   Passed    0.80 sec
    127        Start  69: policy_fee_tests
    128 54/134 Test  [#57](/bitcoin-bitcoin/57/): miniminer_tests ......................   Passed    1.66 sec
    129        Start  70: policyestimator_tests
    130 55/134 Test  [#65](/bitcoin-bitcoin/65/): node_warnings_tests ..................   Passed    0.78 sec
    131        Start  71: pool_tests
    132 56/134 Test  [#67](/bitcoin-bitcoin/67/): peerman_tests ........................   Passed    0.80 sec
    133        Start  72: pow_tests
    134 57/134 Test  [#66](/bitcoin-bitcoin/66/): orphanage_tests ......................   Passed    0.92 sec
    135        Start  73: prevector_tests
    136 58/134 Test  [#68](/bitcoin-bitcoin/68/): pmt_tests ............................   Passed    0.85 sec
    137        Start  74: raii_event_tests
    138 59/134 Test  [#69](/bitcoin-bitcoin/69/): policy_fee_tests .....................   Passed    0.58 sec
    139        Start  75: random_tests
    140 60/134 Test  [#58](/bitcoin-bitcoin/58/): miniscript_tests .....................   Passed    2.32 sec
    141        Start  76: rbf_tests
    142 61/134 Test  [#71](/bitcoin-bitcoin/71/): pool_tests ...........................   Passed    0.78 sec
    143        Start  77: rest_tests
    144 62/134 Test  [#72](/bitcoin-bitcoin/72/): pow_tests ............................   Passed    0.78 sec
    145        Start  78: result_tests
    146 63/134 Test  [#74](/bitcoin-bitcoin/74/): raii_event_tests .....................   Passed    0.72 sec
    147        Start  79: reverselock_tests
    148 64/134 Test  [#56](/bitcoin-bitcoin/56/): miner_tests ..........................   Passed    3.05 sec
    149        Start  80: rpc_tests
    150 65/134 Test  [#73](/bitcoin-bitcoin/73/): prevector_tests ......................   Passed    1.09 sec
    151        Start  81: sanity_tests
    152 66/134 Test  [#78](/bitcoin-bitcoin/78/): result_tests .........................   Passed    0.49 sec
    153        Start  82: scheduler_tests
    154 67/134 Test  [#77](/bitcoin-bitcoin/77/): rest_tests ...........................   Passed    0.67 sec
    155        Start  83: script_p2sh_tests
    156 68/134 Test  [#79](/bitcoin-bitcoin/79/): reverselock_tests ....................   Passed    0.60 sec
    157        Start  84: script_parse_tests
    158 69/134 Test  [#76](/bitcoin-bitcoin/76/): rbf_tests ............................   Passed    1.24 sec
    159        Start  85: script_segwit_tests
    160 70/134 Test  [#82](/bitcoin-bitcoin/82/): scheduler_tests ......................   Passed    0.68 sec
    161        Start  86: script_standard_tests
    162 71/134 Test  [#81](/bitcoin-bitcoin/81/): sanity_tests .........................   Passed    0.76 sec
    163        Start  87: script_tests
    164 72/134 Test  [#83](/bitcoin-bitcoin/83/): script_p2sh_tests ....................   Passed    0.85 sec
    165        Start  88: scriptnum_tests
    166 73/134 Test  [#84](/bitcoin-bitcoin/84/): script_parse_tests ...................   Passed    0.61 sec
    167        Start  89: serfloat_tests
    168 74/134 Test  [#80](/bitcoin-bitcoin/80/): rpc_tests ............................   Passed    1.19 sec
    169        Start  90: serialize_tests
    170 75/134 Test  [#85](/bitcoin-bitcoin/85/): script_segwit_tests ..................   Passed    0.74 sec
    171        Start  91: settings_tests
    172 76/134 Test  [#86](/bitcoin-bitcoin/86/): script_standard_tests ................   Passed    0.77 sec
    173        Start  92: sighash_tests
    174 77/134 Test  [#70](/bitcoin-bitcoin/70/): policyestimator_tests ................   Passed    2.88 sec
    175        Start  93: sigopcount_tests
    176 78/134 Test  [#88](/bitcoin-bitcoin/88/): scriptnum_tests ......................   Passed    0.71 sec
    177        Start  94: skiplist_tests
    178 79/134 Test  [#90](/bitcoin-bitcoin/90/): serialize_tests ......................   Passed    0.86 sec
    179        Start  95: sock_tests
    180 80/134 Test  [#63](/bitcoin-bitcoin/63/): net_tests ............................   Passed    4.15 sec
    181        Start  96: span_tests
    182 81/134 Test  [#91](/bitcoin-bitcoin/91/): settings_tests .......................   Passed    0.85 sec
    183        Start  97: streams_tests
    184 82/134 Test  [#87](/bitcoin-bitcoin/87/): script_tests .........................   Passed    1.54 sec
    185        Start  98: sync_tests
    186 83/134 Test  [#89](/bitcoin-bitcoin/89/): serfloat_tests .......................   Passed    1.31 sec
    187        Start  99: system_tests
    188 84/134 Test  [#93](/bitcoin-bitcoin/93/): sigopcount_tests .....................   Passed    0.82 sec
    189        Start 100: timeoffsets_tests
    190 85/134 Test  [#96](/bitcoin-bitcoin/96/): span_tests ...........................   Passed    0.62 sec
    191        Start 101: torcontrol_tests
    192 86/134 Test  [#92](/bitcoin-bitcoin/92/): sighash_tests ........................   Passed    1.08 sec
    193        Start 102: transaction_tests
    194 87/134 Test  [#95](/bitcoin-bitcoin/95/): sock_tests ...........................   Passed    0.80 sec
    195        Start 103: translation_tests
    196 88/134 Test  [#98](/bitcoin-bitcoin/98/): sync_tests ...........................   Passed    0.61 sec
    197        Start 104: txindex_tests
    198 89/134 Test  [#94](/bitcoin-bitcoin/94/): skiplist_tests .......................   Passed    1.30 sec
    199        Start 105: txpackage_tests
    200 90/134 Test  [#97](/bitcoin-bitcoin/97/): streams_tests ........................   Passed    0.81 sec
    201        Start 106: txreconciliation_tests
    202 91/134 Test [#101](/bitcoin-bitcoin/101/): torcontrol_tests .....................   Passed    0.56 sec
    203        Start 107: txrequest_tests
    204 92/134 Test [#100](/bitcoin-bitcoin/100/): timeoffsets_tests ....................   Passed    0.72 sec
    205        Start 108: txvalidation_tests
    206 93/134 Test  [#99](/bitcoin-bitcoin/99/): system_tests .........................   Passed    0.78 sec
    207        Start 109: txvalidationcache_tests
    208 94/134 Test [#103](/bitcoin-bitcoin/103/): translation_tests ....................   Passed    0.55 sec
    209        Start 110: uint256_tests
    210 95/134 Test [#106](/bitcoin-bitcoin/106/): txreconciliation_tests ...............   Passed    0.76 sec
    211        Start 111: util_tests
    212 96/134 Test [#110](/bitcoin-bitcoin/110/): uint256_tests ........................   Passed    0.56 sec
    213        Start 112: util_threadnames_tests
    214 97/134 Test [#104](/bitcoin-bitcoin/104/): txindex_tests ........................   Passed    1.06 sec
    215        Start 113: validation_block_tests
    216 98/134 Test [#108](/bitcoin-bitcoin/108/): txvalidation_tests ...................   Passed    0.98 sec
    217        Start 114: validation_chainstate_tests
    218 99/134 Test [#112](/bitcoin-bitcoin/112/): util_threadnames_tests ...............   Passed    0.56 sec
    219        Start 115: validation_chainstatemanager_tests
    220100/134 Test [#111](/bitcoin-bitcoin/111/): util_tests ...........................   Passed    0.78 sec
    221        Start 116: validation_flush_tests
    222101/134 Test [#114](/bitcoin-bitcoin/114/): validation_chainstate_tests ..........   Passed    0.88 sec
    223        Start 117: validation_tests
    224102/134 Test [#105](/bitcoin-bitcoin/105/): txpackage_tests ......................   Passed    2.14 sec
    225        Start 118: validationinterface_tests
    226103/134 Test [#116](/bitcoin-bitcoin/116/): validation_flush_tests ...............   Passed    0.70 sec
    227        Start 119: versionbits_tests
    228104/134 Test  [#75](/bitcoin-bitcoin/75/): random_tests .........................   Passed    6.26 sec
    229        Start 120: coinselector_tests
    230105/134 Test [#117](/bitcoin-bitcoin/117/): validation_tests .....................   Passed    0.79 sec
    231        Start 121: feebumper_tests
    232106/134 Test [#107](/bitcoin-bitcoin/107/): txrequest_tests ......................   Passed    2.85 sec
    233        Start 122: group_outputs_tests
    234107/134 Test [#118](/bitcoin-bitcoin/118/): validationinterface_tests ............   Passed    0.91 sec
    235        Start 123: init_tests
    236108/134 Test [#121](/bitcoin-bitcoin/121/): feebumper_tests ......................   Passed    0.74 sec
    237        Start 124: ismine_tests
    238109/134 Test   [#7](/bitcoin-bitcoin/7/): exhaustive_tests .....................   Passed   15.05 sec
    239        Start 125: psbt_wallet_tests
    240110/134 Test [#123](/bitcoin-bitcoin/123/): init_tests ...........................   Passed    0.72 sec
    241        Start 126: wallet_util_tests
    242111/134 Test [#109](/bitcoin-bitcoin/109/): txvalidationcache_tests ..............   Passed    3.71 sec
    243        Start 127: scriptpubkeyman_tests
    244112/134 Test [#119](/bitcoin-bitcoin/119/): versionbits_tests ....................   Passed    1.76 sec
    245        Start 128: spend_tests
    246113/134 Test [#124](/bitcoin-bitcoin/124/): ismine_tests .........................   Passed    0.72 sec
    247        Start 129: wallet_crypto_tests
    248114/134 Test [#126](/bitcoin-bitcoin/126/): wallet_util_tests ....................   Passed    0.53 sec
    249        Start 130: wallet_tests
    250115/134 Test [#122](/bitcoin-bitcoin/122/): group_outputs_tests ..................   Passed    1.42 sec
    251        Start 131: wallet_transaction_tests
    252116/134 Test [#127](/bitcoin-bitcoin/127/): scriptpubkeyman_tests ................   Passed    0.69 sec
    253        Start 132: walletdb_tests
    254117/134 Test  [#29](/bitcoin-bitcoin/29/): coins_tests ..........................   Passed   13.92 sec
    255        Start 133: walletload_tests
    256118/134 Test [#125](/bitcoin-bitcoin/125/): psbt_wallet_tests ....................   Passed    0.99 sec
    257        Start 134: db_tests
    258119/134 Test [#102](/bitcoin-bitcoin/102/): transaction_tests ....................   Passed    5.40 sec
    259120/134 Test [#131](/bitcoin-bitcoin/131/): wallet_transaction_tests .............   Passed    0.80 sec
    260121/134 Test [#115](/bitcoin-bitcoin/115/): validation_chainstatemanager_tests ...   Passed    3.91 sec
    261122/134 Test [#132](/bitcoin-bitcoin/132/): walletdb_tests .......................   Passed    0.83 sec
    262123/134 Test [#113](/bitcoin-bitcoin/113/): validation_block_tests ...............   Passed    4.51 sec
    263124/134 Test [#134](/bitcoin-bitcoin/134/): db_tests .............................   Passed    1.13 sec
    264125/134 Test [#128](/bitcoin-bitcoin/128/): spend_tests ..........................   Passed    2.06 sec
    265126/134 Test [#129](/bitcoin-bitcoin/129/): wallet_crypto_tests ..................   Passed    1.99 sec
    266127/134 Test [#133](/bitcoin-bitcoin/133/): walletload_tests .....................   Passed    2.69 sec
    267128/134 Test [#130](/bitcoin-bitcoin/130/): wallet_tests .........................   Passed    5.94 sec
    268129/134 Test [#120](/bitcoin-bitcoin/120/): coinselector_tests ...................   Passed   15.95 sec
    269130/134 Test   [#1](/bitcoin-bitcoin/1/): util_test_runner .....................   Passed   30.28 sec
    270131/134 Test   [#5](/bitcoin-bitcoin/5/): noverify_tests .......................   Passed   73.37 sec
    271132/134 Test   [#6](/bitcoin-bitcoin/6/): tests ................................   Passed  143.15 sec
    272133/134 Test  [#16](/bitcoin-bitcoin/16/): base64_tests .........................   Passed  144.17 sec
    273134/134 Test   [#3](/bitcoin-bitcoin/3/): univalue_test ........................   Passed  144.20 sec
    274
    275100% tests passed, 0 tests failed out of 134
    276
    277Total Test time (real) = 144.21 sec
    

    fanquake commented at 1:29 pm on September 8, 2024:
    Can you list the steps to make that happen, on a vanilla system. Given the steps in our instructions aren’t currently sufficient.

    hebasto commented at 3:29 pm on September 8, 2024:

    Sure! Here are the steps to cross-compile for Windows and run unit test on a fresh Ubuntu 24.04 installation:

    0$ sudo apt install binfmt-support cmake curl git g++-mingw-w64-x86-64-posix make pkg-config wine-binfmt
    1$ sudo update-binfmts --enable wine
    2$ git clone https://github.com/bitcoin/bitcoin.git
    3$ cd bitcoin
    4$ gmake -C depends -j $(nproc) HOST=x86_64-w64-mingw32 NO_QT=1
    5$ cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
    6$ cmake --build build -j $(nproc)
    7$ ctest --test-dir build -j $(nproc)
    

    fanquake commented at 3:32 pm on September 8, 2024:
    Ok. That is what I did; so I’ll have to retest. Although, above you said we’d have to do wine configuration etc, and that was a reason to not add it to the docs. However I don’t see anything like that here?

    hebasto commented at 3:35 pm on September 8, 2024:

    Although, above you said we’d have to do wine configuration etc, and that was a reason to not add it to the docs. However I don’t see anything like that here?

    0$ sudo update-binfmts --enable wine
    

    fanquake commented at 3:37 pm on September 8, 2024:
    I was under the impression that happened automatically on wine installation. Will also re-test.

    hebasto commented at 3:41 pm on September 8, 2024:

    I was under the impression that happened automatically on wine installation.

    I’m not sure, but it can depend on a distro.

  237. maflcko commented at 7:00 pm on August 22, 2024: member

    I reviewed everything from scratch commit-by-commit. I couldn’t find any massive holes, only a few instances of missing stuff of severity such as #30454 (review), which is trivial and quick to follow-up on and not a blocker. (My plan is to submit the review comments after merge, in one batch, so as to not distract this pull request right now with noise and comment-cycles)

    However, I did not review:

    • cmake/module/Find{*}.cmake
    • The translate target
    • Anything MSVC, VCPKG, Apple, Darwin
    • MULTI_CONFIG

    Also, I am not a build-system expert, so I may have overlooked stuff.

    Also, I haven’t done any tests. I’ll do them in the coming days and update the drafted review comments as needed.

    review ACK 41051290ab3b6c36312cec26a27f787cea9961b4 🐥

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: review ACK 41051290ab3b6c36312cec26a27f787cea9961b4 🐥
    3hU9e0FnrniROs75oW/F5Op9SsvAT+AkF01s2a7qdVi2AZEs0wbc0rSK7RF5eCFpOXQ3HdqCXbQc1R/Pr1dOMDQ==
    
  238. sipsorcery commented at 8:33 am on August 23, 2024: member

    ACK 41051290ab3b6c36312cec26a27f787cea9961b4.

    Tested successful builds on:

    • Win11 + msvc x64 dynamic and static
    • Win11 + WSL Ubuntu 24.04 + mingw32

    I couldn’t perform the build on Win11 + WSL + Ubuntu 20.04 as the g++-mingw toolset not available. I don’t think this is a problem since it’s trivial to install a newer Ubuntu version on WSL.

  239. in src/test/CMakeLists.txt:173 in ab2e99b0d9 outdated
    168+    REPLACE "(BOOST_FIXTURE_TEST_SUITE|BOOST_AUTO_TEST_SUITE)\\(" ""
    169+    test_suite_name "${test_suite_macro}"
    170+  )
    171+  if(test_suite_name)
    172+    add_test(NAME ${test_suite_name}
    173+      COMMAND test_bitcoin --run_test=${test_suite_name} --catch_system_error=no
    


    maflcko commented at 9:42 am on August 23, 2024:
    ab2e99b0d95714e16a7d1a1313d7da938b0485cb: This is missing -l test_suite and DEBUG_LOG_OUT

    hebasto commented at 5:23 pm on September 3, 2024:
    Fixed in #30803.
  240. in depends/README.md:24 in 41051290ab
    28-The default install prefix when using `config.site` is `--prefix=depends/<host-platform-triplet>`,
    29-so depends build outputs will be installed in that location.
    30+    cmake -B build --toolchain depends/x86_64-w64-mingw32/toolchain.cmake
    31 
    32 Common `host-platform-triplet`s for cross compilation are:
    33 
    


    pablomartin4btc commented at 3:56 pm on August 23, 2024:
    Building with depends on macOS setting the host as arm64-apple-darwin (following the notes in the readme file) I got confused noticing that the cross compilation was activated in the configuration (which was solved originally in https://github.com/hebasto/bitcoin/pull/125), then I checked in Ubuntu 22.04 which works fine and rerun the command on macOS without specifying HOST= and I found that in M3 the actual triplet is aarch64-apple-darwin, should we add it? Perhaps in a follow-up).
  241. pablomartin4btc commented at 4:05 pm on August 23, 2024: member

    re-ACK 41051290ab3b6c36312cec26a27f787cea9961b4

    Built successfully on macOS 14.4 with/ without GUI and legacy wallet. Noticed I didn’t have ccache installed so I also played with/ without it and reviewed the productivity.md doc.

    Also ran the tests with ctest and performed the deploy that had a in issue when compiling with --target identified by @fanquake earlier and I’ve verified that works perfect now.

    I’ll check cross building on WSL 24.04 soon.

  242. l0rinc commented at 6:21 pm on August 23, 2024: contributor

    After yesterday’s OS update (might be unrelated, not sure), I started getting:

     0% cmake -B build && cmake --build build -j8 && build/src/test/test_bitcoin --run_test=coins_tests
     1...
     2
     3[ 70%] Linking CXX executable test_bitcoin
     4ld: warning: ignoring duplicate libraries: '../secp256k1/src/libsecp256k1.a'
     5duplicate symbol '__ZN16sigopcount_tests13GetSigOpCount11test_methodEv' in:
     6    build/src/test/CMakeFiles/test_bitcoin.dir/sigopcount_tests.cpp.o
     7    build/src/test/CMakeFiles/test_bitcoin.dir/validation_chainstate_tests.cpp.o
     8duplicate symbol '__ZN16sigopcount_tests14GetTxSigOpCost11test_methodEv' in:
     9    build/src/test/CMakeFiles/test_bitcoin.dir/sigopcount_tests.cpp.o
    10    build/src/test/CMakeFiles/test_bitcoin.dir/validation_chainstate_tests.cpp.o
    11ld: 2 duplicate symbols
    

    which was weird, since sigopcount_tests.cpp and validation_chainstate_tests.cpp weren’t modified recently and compilation was working correctly with autotools and with cmake -B build -DWITH_CCACHE=OFF.

    Comparing the objdump of sigopcount_tests.cpp.o and validation_chainstate_tests.cpp.o revealed that for some reason ccache returned the content of sigopcount_tests for validation_chainstate_tests (exact match, 0 difference between the two).

    Not exactly sure if this is a CMake error or not, but given that it’s working with autotools, thought I’ll report it here. Note that I was merging this branch with every PR I checked out locally and built without clearing the cache in-between. That should work, right?

     0% sw_vers
     1ProductName:            macOS
     2ProductVersion:         14.6.1
     3BuildVersion:           23G93
     4
     5% ccache --version
     6ccache version 4.10.2
     7
     8% cmake --version
     9cmake version 3.30.2
    10
    11% clang --version
    12Homebrew clang version 18.1.8
    13Target: arm64-apple-darwin23.6.0
    14Thread model: posix
    

    I’m also trying to compare the build artefacts before/after the migration to make sure the build tool doesn’t change our deliverables. Tried using https://github.com/fanquake/core-review/blob/master/docker/debian.dockerfile, but the binaries seemed very different, can someone please point me to a doc on how we usually compare them?

  243. theuni commented at 7:44 pm on August 23, 2024: member
    @hebasto Didn’t we decide in last week’s IRC meeting to delete configure.ac + Makefile.* in this PR, then doing a full removal (#30664) as a follow-up? That way the conflicts are apparent as soon as it’s merged?
  244. hebasto commented at 7:58 pm on August 23, 2024: member

    @hebasto Didn’t we decide in last week’s IRC meeting to delete configure.ac + Makefile.* in this PR, then doing a full removal (#30664) as a follow-up? That way the conflicts are apparent as soon as it’s merged?

    I understood it as an intention to reveal conflicts. A follow up that just removes the code should be a no-brainer, right?

  245. pablomartin4btc approved
  246. pablomartin4btc commented at 10:19 pm on August 23, 2024: member

    tACK 41051290ab3b6c36312cec26a27f787cea9961b4

    Cross built for Win11 on WSL 24.04, performed install, deploy, run ctest and test_bitcoin on both WSL and WIndows.

    Small details regarding the docs:

    • In “Building for 64-bit Windows” doc, building depends got changed to gmake, but not in depends doc.
    • Also there’s no reference to ctest in build-unix.md except at the end as part of the “Setup and Build Example: Arch Linux” , should not be at the beginning in the “To Build” section as well?
  247. l0rinc commented at 12:02 pm on August 25, 2024: contributor

    Comparing the objdump of sigopcount_tests.cpp.o and validation_chainstate_tests.cpp.o revealed that for some reason ccache returned the content of sigopcount_tests for validation_chainstate_tests (exact match, 0 difference between the two).

    I investigated further, and if I understand correctly, it appears that ccache is indeed returning sigopcount_tests binaries for validation_chainstate_tests.

    I generated build debug logs using the following commands:

    0export CCACHE_DEBUG=1
    1export CCACHE_DEBUGDIR=~/ccache-debug-fail
    2git clean -fxd && cmake -B build && cmake --build build -j10
    

    When checking the corresponding build commands and results for validation_chainstate_tests.cpp and sigopcount_tests.cpp, I found the following:

    build/src/test/CMakeFiles/test_bitcoin.dir/validation_chainstate_tests.cpp.o.20240825_123816_090964.ccache-log:

    0Manifest key: 6af4hv9nbb0dhlmrck6nb8hr8llmdrmps
    1Retrieved 6af4hv9nbb0dhlmrck6nb8hr8llmdrmps from local storage (/Users/lorinc/.ccache/6/a/f4hv9nbb0dhlmrck6nb8hr8llmdrmpsM)
    2Got result key from manifest
    3Result key: 04f9ih83ggvf132nto09kl2ljemb7onbc
    4Retrieved 04f9ih83ggvf132nto09kl2ljemb7onbc from local storage (/Users/lorinc/.ccache/0/4/f9ih83ggvf132nto09kl2ljemb7onbcR)
    

    vs

    build/src/test/CMakeFiles/test_bitcoin.dir/sigopcount_tests.cpp.o.20240825_123815_909195.ccache-log:

    0Manifest key: 387dad41g4hv0p4jt4tvjlsc7bshr77gg
    1Retrieved 387dad41g4hv0p4jt4tvjlsc7bshr77gg from local storage (/Users/lorinc/.ccache/3/8/7dad41g4hv0p4jt4tvjlsc7bshr77ggM)
    2Got result key from manifest
    3Result key: 04f9ih83ggvf132nto09kl2ljemb7onbc
    4Retrieved 04f9ih83ggvf132nto09kl2ljemb7onbc from local storage (/Users/lorinc/.ccache/0/4/f9ih83ggvf132nto09kl2ljemb7onbcR)
    

    I.e. both are resolved to the same cache entry:

    004f9ih83ggvf132nto09kl2ljemb7onbc
    

    stored in

    0.ccache/0/4/f9ih83ggvf132nto09kl2ljemb7onbcR
    

    Extracting the corresponding cache entry proves that it’s sigopcount_tests.cpp related:

    0% ccache --extract-result ~/.ccache/0/4/f9ih83ggvf132nto09kl2ljemb7onbcR
    1% grep GetSigOpCount ccache-result.o
    2Binary file ccache-result.o matches
    3% grep sigopcount_tests ccache-result.d
    4src/test/CMakeFiles/test_bitcoin.dir/sigopcount_tests.cpp.o: \
    5  /Users/lorinc/IdeaProjects/bitcoin/src/test/sigopcount_tests.cpp \
    

    Removing the .ccache folder and rebuilding everything results in a different cache key and correct looking build. I’m not yet sure if it’s CMake related or not (could it be caused by switching branches often?), but I have sent the cache entries to @hebasto.

  248. i-am-yuvi commented at 8:37 pm on August 25, 2024: none

    tACK 4105129

    Tested and Built successfully on:

    • MacOS 14.6.1 without GUI and Wallet
    • Ubuntu 24.04 without GUI and Wallet

    Also ran tests with ctest.

  249. MarnixCroes commented at 1:53 pm on August 26, 2024: contributor

    41051290ab3b6c36312cec26a27f787cea9961b4 succesfully built on debian12 and fedora40

    • the build requirements for Fedora at doc/build.unix.md are incorrect/not updated
    • disable-wallet mode is not working without sqlite dependency installed? Using cmake -B build -DENABLE_WALLET=OFF, as per the instructions, will not work without sqlite installed. (error that sqlite is missing)
  250. hebasto commented at 2:02 pm on August 26, 2024: member
    • the build requirements for Fedora at doc/build.unix.md are incorrect/not updated

    Thanks! Will update them in a follow-up along with other doc amendments.

  251. hebasto commented at 2:13 pm on August 26, 2024: member

    4105129 succesfully built on debian12 and fedora40

    • disable-wallet mode is not working without sqlite dependency installed? Using cmake -B build -DENABLE_WALLET=OFF, as per the instructions, will not work without sqlite installed. (error that sqlite is missing)

    I cannot reproduce the issue on Fedora 40:

    0$ dnf list installed sqlite-devel
    1Error: No matching Packages to list
    2$ cmake -B build -DENABLE_WALLET=OFF
    3< snip >
    4-- Configuring done (24.7s)
    5-- Generating done (0.1s)
    6-- Build files have been written to: /home/hebasto/git/bitcoin/build
    

    Is your build directory clean?

  252. MarnixCroes commented at 2:59 pm on August 26, 2024: contributor

    4105129 succesfully built on debian12 and fedora40

    • disable-wallet mode is not working without sqlite dependency installed? Using cmake -B build -DENABLE_WALLET=OFF, as per the instructions, will not work without sqlite installed. (error that sqlite is missing)

    I cannot reproduce the issue on Fedora 40:

    0$ dnf list installed sqlite-devel
    1Error: No matching Packages to list
    2$ cmake -B build -DENABLE_WALLET=OFF
    3< snip >
    4-- Configuring done (24.7s)
    5-- Generating done (0.1s)
    6-- Build files have been written to: /home/hebasto/git/bitcoin/build
    

    Is your build directory clean?

    I cannot repro anymore. sorry for the pollution

  253. in CMakeLists.txt:26 in 41051290ab
    21+
    22+#=============================
    23+# Project / Package metadata
    24+#=============================
    25+set(PACKAGE_NAME "Bitcoin Core")
    26+set(CLIENT_VERSION_MAJOR 27)
    


    achow101 commented at 8:04 pm on August 26, 2024:

    In a2317e27b7fb86df4e32cd1674c06e09cb808248 “cmake: Add root CMakeLists.txt file”

    The plan is to have this merged post 28.x branch off, so I think this should be set to 28.


    hebasto commented at 8:22 am on August 27, 2024:
    Correct. Will be updated in a follow up PR, along with a few other amendments such as #30712.
  254. theuni commented at 8:32 pm on August 26, 2024: member

    x86-64 Guix build:

     0bfce0ebd120bd45576586691c46b47e66bb0031f7540772a924d4cb1c2602576  aarch64-linux-gnu/bitcoin-41051290ab3b-aarch64-linux-gnu-debug.tar.gz
     1615aae82e503fc07ee1053649b3a45ad2f0dd9c02d5a85394f8ba33b3bd5be88  aarch64-linux-gnu/bitcoin-41051290ab3b-aarch64-linux-gnu.tar.gz
     2c4e0333106b259d1ab11066677fc32fba8dd380afc2f9c334d9eff2dccccba39  aarch64-linux-gnu/SHA256SUMS.part
     3152a84bd68011d9af6e30830f4da1a72ebee401160c1fefd46e9de0cae77c94a  arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin.tar.gz
     4db755f2f4321ed1ba930ffd73aaa6d3f30ce028c1cbcbc120e3a4f9729d44aeb  arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin-unsigned.tar.gz
     57375d138f0351c9f41b25c02b3628894b2fdb8add8783952468d71afbf3fbb47  arm64-apple-darwin/bitcoin-41051290ab3b-arm64-apple-darwin-unsigned.zip
     63c8fc1d8f4bbf87e7106dfcd6fb9ac89fdc98c47ccfd27f791e85396458acf6d  arm64-apple-darwin/SHA256SUMS.part
     7fe9b3740ddcf221ee6b2a16c1fcc51f0f1276226a6f0d5ede4ec9a5ec8630473  arm-linux-gnueabihf/bitcoin-41051290ab3b-arm-linux-gnueabihf-debug.tar.gz
     8459f02e850925be6c7c7e04f2c5ce851838505c1fa3fb86ad796e3892b62b561  arm-linux-gnueabihf/bitcoin-41051290ab3b-arm-linux-gnueabihf.tar.gz
     92a8ef06d7795513a3d53c43560c3810c94cbe1072a71780ab32ffbda53f7be70  arm-linux-gnueabihf/SHA256SUMS.part
    1097c437130ae60c72400ba9738fe2ef03dbd7bc25e303185061b426ac8f0244ea  dist-archive/bitcoin-41051290ab3b.tar.gz
    11c02e3d07355b36481a9c442085a2d3370d19031defa0c1831b39f443abc650d1  powerpc64-linux-gnu/bitcoin-41051290ab3b-powerpc64-linux-gnu-debug.tar.gz
    126a1e7e6eedc15ed13be15c526c4c9145241e012cc7b23ecf23bbde455eb0e74e  powerpc64-linux-gnu/bitcoin-41051290ab3b-powerpc64-linux-gnu.tar.gz
    133f0961cf0912d9469c279c76987d81c217e755b7b2593fcf04171663f453ef1d  powerpc64-linux-gnu/SHA256SUMS.part
    14d5bd95a0f718e72b01732e37de33a9e63ae26182e67d4458b47ca6051fc2ce9d  riscv64-linux-gnu/bitcoin-41051290ab3b-riscv64-linux-gnu-debug.tar.gz
    15fbdd2ca094b9f69c17fdebedc9d790eab0cd58a2ce12833efec602d210a39a42  riscv64-linux-gnu/bitcoin-41051290ab3b-riscv64-linux-gnu.tar.gz
    16e99090bd1371547fae416e70fe0977ab8679e4d710446ee7b1df980618a728b6  riscv64-linux-gnu/SHA256SUMS.part
    17a52f9e74d790f71f8ec89a23338df518053dce6ad5c37338ba13ba4438a367cd  x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin.tar.gz
    180a7214b6617da6f36cd632a9dc1edbdeda25a5d3049cd2b597634164d906abe8  x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin-unsigned.tar.gz
    19c266669f9e663c7f35060adeba9148e8b5cda45376ae90ee917eb411d6890e46  x86_64-apple-darwin/bitcoin-41051290ab3b-x86_64-apple-darwin-unsigned.zip
    20aeeb2f5ab3a3197481f0c6c2aea1837026dc322b0066c229b365ba8747d829f2  x86_64-apple-darwin/SHA256SUMS.part
    2191da98db1e4eea4738b839ed3b425dcf6ffdccd2177debe46314cceb785f0728  x86_64-linux-gnu/bitcoin-41051290ab3b-x86_64-linux-gnu-debug.tar.gz
    22f8bc57976b26c4a378dc7a3be5e6f60de9e7b1d73fae6a2a96505801194f1420  x86_64-linux-gnu/bitcoin-41051290ab3b-x86_64-linux-gnu.tar.gz
    231c1dcc180e415c893a253315786397b6dbbed7a10b40fa9fc4b85f3ae102d370  x86_64-linux-gnu/SHA256SUMS.part
    24572691a757abc27a32a37b0f10c84a361809f4de76fb4a39e39815ded836a26a  x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-debug.zip
    25d02a1bc08fb97a2c4e726fabb3166293061834f0f32dd488a4238304cfaefe28  x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-setup-unsigned.exe
    26963836e69f2382ccd48fbbf68146cd29a78e79b6784297c76b415824ed3e4175  x86_64-w64-mingw32/bitcoin-41051290ab3b-win64-unsigned.tar.gz
    27f42b645f14fd2d9fe88ce44e03f0b175fce4a703961e9ef5dd1f6cf4ad986de1  x86_64-w64-mingw32/bitcoin-41051290ab3b-win64.zip
    28c7642ef6d7529c6c3b3d929de6172ab594ace611c3ab04b11a6c953bcd48823c  x86_64-w64-mingw32/SHA256SUMS.part
    
  255. achow101 commented at 8:51 pm on August 26, 2024: member
    doc/developer-notes.md needs to be updated too.
  256. luke-jr commented at 11:05 pm on August 26, 2024: member

    Initial testing: Seems to work fine. I was surprised Qt wasn’t at least enabled by default.

    The “auto” value is no longer available; non-default values must be specified explicitly.

    I assume this is a CMake standard practice? Or is there some other reason to not autodetect bdb/portmapping/qt/zmq?

  257. hebasto commented at 8:20 am on August 27, 2024: member

    doc/developer-notes.md needs to be updated too. @l0rinc has already started to work on it in https://github.com/hebasto/bitcoin/pull/337. It will be converted in a follow up PR once this PR is merged.

  258. hebasto commented at 8:42 am on August 27, 2024: member

    I was surprised Qt wasn’t at least enabled by default.

    The default build options were tailored to meet the needs of the majority of this repository’s contributors.

    The “auto” value is no longer available; non-default values must be specified explicitly.

    I assume this is a CMake standard practice?

    It is.

    Or is there some other reason to not autodetect bdb/portmapping/qt/zmq?

    Package autodetection can introduce unsolicited changes in the build configuration, which is considered suboptimal practice.

    FWIW, the “AUTO” value was implemented in the staging branch to ensure that the new system mirrors the old one. It was deleted in https://github.com/hebasto/bitcoin/pull/215/commits/4cd15f1c64537fc9964dcdb7ff763884ff4e9278.

  259. theuni approved
  260. theuni commented at 6:55 pm on August 27, 2024: member

    I haven’t reviewed this exact series in detail, but I believe I reviewed it all as it passed through @heabsto’s staging branch.

    Quickly tested this PR merged into master, with depends on Linux x86-64.

    With the understanding that there will be a flurry of follow-up PRs to clean up some things requested here, along with the deletion of autotools:

    ACK 41051290ab3b6c36312cec26a27f787cea9961b4.

  261. fanquake approved
  262. fanquake commented at 9:45 am on August 28, 2024: member

    ACK 41051290ab3b6c36312cec26a27f787cea9961b4

    Going to merge this now. Non-exhaustive list of things that need fixing / following up:

  263. fanquake merged this on Aug 28, 2024
  264. fanquake closed this on Aug 28, 2024

  265. maflcko commented at 9:55 am on August 28, 2024: member

    Autotools removal (ideally today).

    I understand that it should be done quickly. However, given that some things don’t work out-of-the-box anymore, such as clang coverage, it would be good to give a few more days (maybe a week) to give everyone a bit time to adapt their scripts, which there may be many of.

    This also has the benefit that any quick follow-ups and bugfixes can still be tested for autotools parity.

    In any case, I am not going to block the autotools removal, if others agree it should be done faster.

  266. fanquake referenced this in commit e45913ea8b on Aug 28, 2024
  267. maflcko commented at 2:53 pm on August 28, 2024: member
  268. in src/test/README.md:20 in 41051290ab
    18 and tests weren't explicitly disabled.
    19 
    20-After configuring, they can be run with `make check`, which includes unit tests from
    21-subtrees, or `make && make -C src check-unit` for just the unit tests.
    22+Assuming the build directory is named `build`, the unit tests can be run
    23+with `ctests --test-dir build`, which includes unit tests from subtrees.
    


    sdaftuar commented at 4:29 pm on August 28, 2024:
    I think this is supposed to be ctest, not ctests?

    l0rinc commented at 4:30 pm on August 28, 2024:
    Yes, already fixed in a differt pr

    fanquake commented at 4:30 pm on August 28, 2024:
    See #30734. Docs should be improving (generally) over the next day or so.
  269. in doc/build-unix.md:37 in 41051290ab
    38+    cmake -B build -DCMAKE_CXX_FLAGS_RELWITHDEBINFO="-O2 -g0"
    39 
    40 Finally, clang (often less resource hungry) can be used instead of gcc, which is used by default:
    41 
    42-    ./configure CXX=clang++ CC=clang
    43+    cmake -B build -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_CC_COMPILER=clang
    


    sdaftuar commented at 7:58 pm on August 28, 2024:

    Should this say: cmake -B build -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang instead?

    When I follow these instructions as written, I get a Cmake Warning that the CMAKE_CC_COMPILER variable was not used by the project.


    l0rinc commented at 8:04 pm on August 28, 2024:

    hebasto commented at 8:06 pm on August 28, 2024:
    Thanks! Fixed in #30744.
  270. hebasto deleted the branch on Aug 28, 2024
  271. fanquake referenced this in commit 4ae3be772d on Aug 29, 2024
  272. in CMakeLists.txt:67 in a2317e27b7 outdated
    62+set(configure_warnings)
    63+
    64+message("\n")
    65+message("Configure summary")
    66+message("=================")
    67+message("C++ compiler .......................... ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION}, ${CMAKE_CXX_COMPILER}")
    


    maflcko commented at 12:28 pm on August 29, 2024:

    in commit a2317e27b7fb86df4e32cd1674c06e09cb808248: This will only print the compiler name, not the flags.

    E.g. -DCMAKE_CXX_COMPILER='clang++;-stdlib=libc++;-m32' will print the flags nowhere. (This is used in the ci/test/00_setup_env_i686_multiprocess.sh task)


    hebasto commented at 5:23 pm on September 3, 2024:
    Fixed in #30803.
  273. maflcko commented at 9:43 am on August 30, 2024: member
    ci msan fuzz looks broken, see https://github.com/bitcoin/bitcoin/issues/30760
  274. ismaelsadeeq commented at 11:24 am on September 2, 2024: member

    Post-merge Tested ACK 41051290ab3b6c36312cec26a27f787cea9961b4

    Tested on M2 Pro, Sonoma 14.1.1

    Clang version 18.1.7

    CMake version 3.29.5

    Configuration and compilation completed smoothly without any issues.

    Started bitcoind without any issue and tested a few RPCs.

    I also ran unit tests and benchmarks, all ran smoothly.

  275. maflcko commented at 3:32 pm on September 3, 2024: member
    Submitting my review comments, now that this has been merged.
  276. ryanofsky commented at 4:53 pm on September 10, 2024: contributor
    Are there release notes for cmake transition? Seems like the autoconf -> cmake mapping table https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e would be especially helpful to have in release notes to help users and packagers convert previous configurations to new ones.
  277. maflcko added the label Needs release note on Sep 11, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-25 18:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me