doc: update dev note examples for CMake

fanquake commented at 2:17 pm on August 28, 2024: member

Update the examples in the developer notes to work with CMake. Also added an explicit -DCMAKE_EXPORT_COMPILE_COMMANDS=ON for clarity.

DrahtBot commented at 2:17 pm on August 28, 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	TheCharlatan, jonatack, davidgumberg

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

DrahtBot added the label Docs on Aug 28, 2024

doc: update dev note examples for CMake 7de0c99804

fanquake force-pushed on Aug 28, 2024

TheCharlatan approved

TheCharlatan commented at 4:18 pm on August 28, 2024: contributor

ACK 7de0c99804bca98ef159b7b778e6f5b602507d2c

jonatack commented at 5:05 pm on August 28, 2024: member

Tested ACK 7de0c99804bca98ef159b7b778e6f5b602507d2c on arm64 macOS 14.6.1

(modulo local issue with the third incantation that is unrelated to this change, with my machine not finding clang-tidy-diff zsh: command not found: clang-tidy-diff)

TheCharlatan commented at 8:31 pm on August 28, 2024: contributor

Do you want to add https://github.com/TheCharlatan/bitcoin/commit/17f6ecd1470c07a3bf26a08e6d1a55ed2f8486ca? I guess we should update all scripts and examples to use the build directory for executing the binaries? Edit: Ah, nevermind I missed #30741

in doc/developer-notes.md:238 in 7de0c99804

239 
240 To run clang-tidy on the changed source lines:
241 
242 ```sh
243-git diff | ( cd ./src/ && clang-tidy-diff -p2 -j $(nproc) )
244+git diff | ( cd ./src/ && clang-tidy-diff -p2 -path ../build -j $(nproc) )

davidgumberg commented at 0:03 am on August 29, 2024:

Out-of-scope-feel-free-to-disregard:

On Fedora 40, I don’t have the binary clang-tidy-diff, but I tested that the invocation added here works if I do:

0git diff | ( cd ./src/ && /usr/share/clang/clang-tidy-diff.py -p2 -path ../build -j $(nproc) )

maflcko commented at 4:05 am on August 29, 2024:

There will always be a distro where it doesn’t work and other command may be needed. Generally, I think the commands target Debian/Ubuntu. If they happen to work on all distros, great, and I am sure developers are able to figure out their distro’s way of calling the command.

in doc/developer-notes.md:225 in 7de0c99804

224 ```sh
225-./autogen.sh && ./configure CC=clang CXX=clang++
226-make clean && bear --config src/.bear-tidy-config -- make -j $(nproc)
227+cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
228+cmake --build build -j $(nproc)
229 ```

davidgumberg commented at 0:21 am on August 29, 2024:

Feel free to disregard alternative:

0Configure with clang as the compiler to produce the `compile_commands.json` that
1`clang-tidy` needs:
2
3```sh
4cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++
5cmake --build build -j $(nproc)
6```

davidgumberg commented at 0:36 am on August 29, 2024: contributor

Tested ACK https://github.com/bitcoin/bitcoin/commit/7de0c99804bca98ef159b7b778e6f5b602507d2c

Used the invocation provided here on a clean git clone of bitcoin core, and these instructions worked for generating the needed compile_commands.json for clang-tidy, and the arguments provided here help clang-tidy find the compile commands.

This bit of the docs is also helpful for anyone wanting to use their ide/editor’s lsp client with the clangd server which also requires compile_commands.json.

clang-tidy-diff

I had the same issue with clang-tidy-diff as @jonatack, but I tested that the invocation provided here works with the out-of-$PATH-for-me clang-tidy-diff.py:

0git diff | ( cd ./src/ && /usr/share/clang/clang-tidy-diff.py -p2 -path ../build -j $(nproc) )

This actually seems like it might be partly llvm’s issue, as there’s a ~10 year old FIXME comment asking for the clang-tidy-diff.py stuff to get rolled into run-clang-tidy.

tdb3 commented at 2:47 am on August 29, 2024: contributor

Initially running into issues building with Debian (12.6) default clang 14.0.6 and with 18.1.8. Fresh clone. Hope to revisit soon. No issues seen with gcc (following https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#to-build).

0cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
1cmake --build build -j 18

0-- Found Boost: /usr/include (found suitable version "1.74.0", minimum required is "1.73.0")
1...
2$ clang --version
3Debian clang version 18.1.8 (++20240731024826+3b5b5c1ec4a3-1~exp1~20240731144843.145)
4Target: x86_64-pc-linux-gnu

With 18.1.8

 0In file included from /usr/include/boost/mpl/integral_c.hpp:32:
 1/usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'udt_builtin_mixture_enum' [-Wenum-constexpr-conversion]
 2   73 |     typedef AUX_WRAPPER_INST( BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value - 1)) ) prior;
 3      |                               ^
 4/usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 'BOOST_MPL_AUX_STATIC_CAST'
 5   24 | #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
 6      |                                               ^
 7In file included from ../../../src/test/validation_chainstatemanager_tests.cpp:8:
 8In file included from ../../../src/node/chainstatemanager_args.h:9:
 9In file included from ../../../src/validation.h:28:
10In file included from ../../../src/txmempool.h:26:
11In file included from /usr/include/boost/multi_index/hashed_index.hpp:38:
12In file included from /usr/include/boost/multi_index/detail/node_handle.hpp:22:
13In file included from /usr/include/boost/multi_index_container_fwd.hpp:18:
14In file included from /usr/include/boost/multi_index/indexed_by.hpp:17:
15In file included from /usr/include/boost/mpl/vector.hpp:36:
16In file included from /usr/include/boost/mpl/vector/vector20.hpp:18:
17In file included from /usr/include/boost/mpl/vector/vector10.hpp:18:
18In file included from /usr/include/boost/mpl/vector/vector0.hpp:24:
19In file included from /usr/include/boost/mpl/vector/aux_/clear.hpp:18:
20In file included from /usr/include/boost/mpl/vector/aux_/vector0.hpp:22:
21In file included from /usr/include/boost/mpl/vector/aux_/iterator.hpp:19:
22In file included from /usr/include/boost/mpl/plus.hpp:19:
23In file included from /usr/include/boost/mpl/aux_/arithmetic_op.hpp:17:
24In file included from /usr/include/boost/mpl/integral_c.hpp:32:
25/usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'int_float_mixture_enum' [-Wenum-constexpr-conversion]
26/usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 'BOOST_MPL_AUX_STATIC_CAST'
27   24 | #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
28      |                                               ^
292 errors generated.

maflcko commented at 7:09 am on August 29, 2024: member

/usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type ‘udt_builtin_mixture_enum’ [-Wenum-constexpr-conversion]

That is just a boost issue, unrelated to cmake. It should happen with autotools as well. I think you can fix it by upgrading your distro (or boost)

fanquake commented at 9:51 am on August 29, 2024: member

Initially running into issues building

Opened an issue to track this: #30751 , as it needs fixing in some way (also for 28.x).

fanquake merged this on Aug 29, 2024

fanquake closed this on Aug 29, 2024

fanquake deleted the branch on Aug 29, 2024

Thompson1985 commented at 11:32 am on August 29, 2024: none

Thank you 👍

Thompson1985 commented at 4:10 am on August 30, 2024: none

Thank you

sins921 commented at 4:45 pm on February 17, 2025: none

purpleKarrot commented at 1:24 pm on February 18, 2025: contributor

It may not be clear to all readers of this document that -B will instruct cmake to create a directory and that the argument could be anything. This information may be useful for people that want to maintain multiple build directories.

cmake --build is useful in generic scripts that handle multiple build systems.

On the command line, invoking the underlying commands explicitly is both easier to understand and faster to type:

0mkdir build
1cd build
2cmake -GNinja ..
3ninja

Users that prefer using clang, may set CC=clang and CXX=clang++ as environment variables. Users that need the compile commands, can set the CMAKE_EXPORT_COMPILE_COMMANDS=1 environment variable. Other useful environment variables are CMAKE_C_COMPILER_LAUNCHER and CMAKE_CXX_COMPILER_LAUNCHER. (By the way, it is not a good idea that the project sets these, because it may interfere with user preferences).

hebasto commented at 2:35 pm on February 18, 2025: member

On the command line, invoking the underlying commands explicitly is both easier to understand and faster to type:
0mkdir build
1cd build
2cmake -GNinja ..
3ninja

I would advise against this approach, as it may inadvertently lead to the use of an incorrect build tool.

For instance, on macOS, when GNU Make is installed via Homebrew, two build tools become available:

 0% which -a make                 
 1/usr/bin/make
 2% make --version                                                                                                                
 3GNU Make 3.81
 4Copyright (C) 2006  Free Software Foundation, Inc.
 5This is free software; see the source for copying conditions.
 6There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
 7PARTICULAR PURPOSE.
 8
 9This program built for i386-apple-darwin11.3.0
10% which -a gmake                
11/opt/homebrew/bin/gmake
12% gmake --version               
13GNU Make 4.4.1
14Built for aarch64-apple-darwin24.0.0
15Copyright (C) 1988-2023 Free Software Foundation, Inc.
16License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
17This is free software: you are free to change and redistribute it.
18There is NO WARRANTY, to the extent permitted by law.

While CMake selects /opt/homebrew/bin/gmake, there remains a risk that the user might inadvertently execute /usr/bin/make manually.

doc: update dev note examples for CMake #30739

Code Coverage

Reviews