fac00d4ed361e5e8c8989b2bb5a4a22dd54e2c72 doc: Move CI-must-pass requirement into readme section (MarcoFalke)
fab79c1a250db252baa206f59d7e46986e21a57c doc: Clarify and move "hygienic commit" note (MarcoFalke)
fac8b051979911f036f9156b2d4e7afbe2853336 doc: Clarify strprintf size specifier note (MarcoFalke)
faaf34ad7253e3d347af986305e405e6ef35f459 doc: Remove section about RPC alias via function pointer (MarcoFalke)
2222d61e1ce5c9efd099f33b5dda934bc9d2d57f doc: Remove section about RPC arg names in table (MarcoFalke)
fa00b8c02c9de6dca833b83dc6447ba31d72ca57 doc: Remove section about include guards (MarcoFalke)
fad6cd739b638cf7e8c86cdccf98df070e64a0f9 doc: Remove dev note section on includes (MarcoFalke)
fa6623d85af192c8aa1d0380d4a5452e0779c64e doc: Remove file name section (MarcoFalke)
7777fb8bc749e18c178ef460b65219187e676128 doc: Remove shebang section (MarcoFalke)
faf65f05312be7647f485f088ba00fef97f47bf4 doc: Remove .gitignore section (MarcoFalke)
faf2094f2511a5322d68d2352f244b5bc89d5fee doc: Remove note about removed ParsePrechecks (MarcoFalke)
fa69c5b170f56d554fcb0d0887bd27f961fe3e74 doc: Remove -disablewallet from dev notes (MarcoFalke)
Pull request description:
This removes sections that I've been collecting as stale or overly redundant over the years. The rationale for each removal is in the commit message.
ACKs for top commit:
yuvicc:
ACK fac00d4ed361e5e8c8989b2bb5a4a22dd54e2c72
janb84:
LGTM ACK fac00d4ed361e5e8c8989b2bb5a4a22dd54e2c72
glozow:
ACK fac00d4ed361e5e8c8989b2bb5a4a22dd54e2c72, all lgtm
Tree-SHA512: 17a5b4277fb30d265959d1230a705b36d8501a64c0f4a7f272ea5d9c22031421f95c491144f6d6f714dc7927df667d96ece9ceb43e0a07317d76fdcc4769aaa7
One of the benefits of using a compilation database, which is available
after the CMake build system generation step, is that it is not
necessary to actually build the code in order to run `clang-tidy`.
This is already checked by test/lint/lint-files.py
There is no need to reword all linters into the dev notes.
Also, allow scripts in Rust (there are already some).
This was added to the gitignore file itself in commit
fada115cbeaa8c0ca3d19178499079b66ee5f499. The past violations are
evidence that the note in the dev notes was missed anyway.
In the dev notes, remove the whole section, because:
* ParseDouble was removed in commit
fa9d72a7947d2cff541794e21e0040c3c1d43b32
* The locale-dependent atoi is already checked by
test/lint/lint-locale-dependence.py
Co-authored-by: Fabian Jahr <fjahr@protonmail.com>
The string exclusion would fail to detect `"bla" + wrong()`.
Also, remove /* */ comment exclusion, which would fail to detect stuff
like `/* bla */ wrong()`.
Instead, require the function to be called by adding \\( to the regex.
Finally, also remove the section in the dev notes, because:
* It was outdated and missing some functions such as std::to_string in
the list.
* The maintenance overhead of having to update two places is fragile and
questionable.
* Many other linters are also not mentioned in the dev notes, even
though they are important.
* A dev (and CI) is more likely to run the linters than to read the dev
notes.
* The dev notes are more than 1000 lines of dense information. It would
be easier to digest if they focused on the important stuff that is not
checked by automated tools.
babb9f5db64145a4adec07dee1c8328195363777 depends: remove non-native libmultiprocess build (Cory Fields)
5d105fb8c3ffa39c3e716c3147ee74795b129113 depends: Switch libmultiprocess packages to use local git subtree (Ryan Ofsky)
9b35518d2f3ff67cbc87d30bd3b882fe72fd5d87 depends, moveonly: split up int_get_build_id function (Ryan Ofsky)
2d373e27071f54e04f8aaf6753e5481ccbc9ed3f lint: Add exclusions for libmultiprocess subtree (Ryan Ofsky)
e88ab394c163a8ebe59ee240b09430d788ef274e doc: Update documentation to explain libmultiprocess subtree (Ryan Ofsky)
d4bc5639829f0c98bd2faeddba9be4e8c6667875 cmake: Fix clang-tidy "no input files" errors (Ryan Ofsky)
abdf3cb6456f3f79b3130d2001bf780deadccf87 cmake: Fix warnings from boost headers (Ryan Ofsky)
8532fcb1c30d6556c273bd77a2a7daa6b0a2448d cmake: Fix ctest mptest "Unable to find executable" errors (Ryan Ofsky)
d597ab1dee6b3541c827e6be61fc8a9b61d1e23d cmake: Support building with libmultiprocess subtree (Ryan Ofsky)
69f0d4adb72c1dc5c261517899d87b0fe8b88304 scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake (Ryan Ofsky)
a2f28e4be96e92079a219567cf20214996aefc53 Squashed 'src/ipc/libmultiprocess/' content from commit 35944ffd23fa (Ryan Ofsky)
d6244f85c509d7644a57def892298a4dde988c3b depends: Update libmultiprocess library to simplify cmake subtree build (Ryan Ofsky)
Pull request description:
This adds the [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) library and code generator as a subtree in `src/ipc/libmultiprocess` and allows it to be built with the cmake `-DENABLE_IPC` option, which is disabled by default.
This PR does not entirely remove the depends system [libmultiprocess package](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/native_libmultiprocess.mk) because the package is useful when cross compiling. (A cross-compiling cmake build cannot easily build and run a native code generation tool.) However, it does update the depends package to build from the new git subtree, instead of being downloaded separately from github, so the same sources are used to build both the runtime library and the code generator.
This PR includes the following manual changes (not created automatically with `git subtree add`) which just update the build system and documentation:
- [`d6244f85c509` depends: Update libmultiprocess library to simplify cmake subtree build](d6244f85c5)
- [`69f0d4adb72c` scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake](69f0d4adb7)
- [`d597ab1dee6b` cmake: Support building with libmultiprocess subtree](d597ab1dee)
- [`8532fcb1c30d` cmake: Fix ctest mptest "Unable to find executable" errors](8532fcb1c3)
- [`abdf3cb6456f` cmake: Fix warnings from boost headers](abdf3cb645)
- [`d4bc5639829f` cmake: Fix clang-tidy "no input files" errors](d4bc563982)
- [`e88ab394c163` doc: Update documentation to explain libmultiprocess subtree](e88ab394c1)
- [`2d373e27071f` lint: Add exclusions for libmultiprocess subtree](2d373e2707)
- [`9b35518d2f3f` depends, moveonly: split up int_get_build_id function](9b35518d2f)
- [`5d105fb8c3ff` depends: Switch libmultiprocess packages to use local git subtree](5d105fb8c3)
- [`babb9f5db641` depends: remove non-native libmultiprocess build](babb9f5db6)
---
Previous minisketch subtree PR #23114 may be useful for comparison
Instructions for subtree verification can be found:
- https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees
- https://github.com/bitcoin/bitcoin/tree/master/test/lint#git-subtree-checksh
TL&DR:
```sh
git remote add --fetch libmultiprocess https://github.com/chaincodelabs/libmultiprocess.git
test/lint/git-subtree-check.sh -r src/ipc/libmultiprocess
```
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
Sjors:
re-ACK babb9f5db64145a4adec07dee1c8328195363777
TheCharlatan:
tACK babb9f5db64145a4adec07dee1c8328195363777
vasild:
ACK babb9f5db64145a4adec07dee1c8328195363777
Tree-SHA512: 43d4eecca5aab63e55c613de935965666eaced327f9fe859a0e9c9b85f7685dc16c5c8d6e03e09ca998628c5d468633f4f743529930b037049abe8e0101e0143
b96f1a696aa792068b5a4fa16e2d4a342e4f55b8 add clang/llvm based coverage report generation (Prabhat Verma)
Pull request description:
Followed up from the [comment](https://github.com/bitcoin/bitcoin/issues/31927#issuecomment-2674522975) on the issue [#31927](https://github.com/bitcoin/bitcoin/issues/31927) , issues have been observed building coverage reports with `gcov` in MacOs and NixOs. This PR adds the steps to generate a coverage report based on the default llvm/clang tooling.
ACKs for top commit:
Crypt-iQ:
tACK b96f1a696aa792068b5a4fa16e2d4a342e4f55b8
hodlinator:
re-ACK b96f1a696aa792068b5a4fa16e2d4a342e4f55b8
janb84:
Re ACK [b96f1a6](b96f1a696a)
Tree-SHA512: bc54f170e84bb76b3eba7285bd49f051c0b99b784d583a550d8e51511497bcc4df8964bbe3991777648d2f829809db8eabb0cbf0d25f9da5e49e1cfc62f6d8d0
aa7a898c236eb519aaf546afee2b9c2b6dfdea1f doc: use testnet4 in developer docs (Sjors Provoost)
6c217d22fdc978cac0f970cf2296a9fa1e00ee97 test: use testnet4 in argsman test (Sjors Provoost)
7c200ece80575d399a552f5757c07ac2c8c7ec6c test: use testnet4 in key_io_valid.json (Sjors Provoost)
d424bd59413c8ffc7a263635e5b9882497d39fed test: drop unused testnet3 magic bytes (Sjors Provoost)
8cfc09fafe59adae4904a21589736de93a00ad2d test: cover testnet4 magic in assumeutxo.py (Sjors Provoost)
4281e3603a2eadefc8535b863128a05cf3a5a75f zmq: use testnet4 in zmq_sub.py example (Sjors Provoost)
Pull request description:
In preparation for dropping testnet3 entirely in #31974 this PR migrates a few things to testnet4:
* the ZMQ examples
* developer docs
* various unit tests
* the snapshot magic byte check in `feature_assumeutxo.py`
It drops `testnet3` from `MAGIC_BYTES` in the test framework, since no test uses it.
ACKs for top commit:
fjahr:
re-ACK aa7a898c236eb519aaf546afee2b9c2b6dfdea1f
maflcko:
lgtm ACK aa7a898c236eb519aaf546afee2b9c2b6dfdea1f 🔊
hodlinator:
re-ACK aa7a898c236eb519aaf546afee2b9c2b6dfdea1f
Tree-SHA512: 235f74273234e8fb2aedf0017dea5c16bb9813ec7a1f89a51abe85691f09830a5ead834115d7db0936e12e55a40bc81888856a8002fe507c1474407e77f8b9fb
329a0dcdafe05002f662e8737a76bfdeaba9a3ed doc: clarify the documentation of `Assume` (ismaelsadeeq)
Pull request description:
An Expression inside `Assume` may be optimized away in production builds when the compiler proves they are side-effect-free.
This use case is demonstrated in #31363 and is suggested to be documented in https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2736410023.
ACKs for top commit:
l0rinc:
ACK 329a0dcdafe05002f662e8737a76bfdeaba9a3ed
hodlinator:
re-ACK 329a0dcdafe05002f662e8737a76bfdeaba9a3ed
jonatack:
ACK 329a0dcdafe05002f662e8737a76bfdeaba9a3ed
rkrux:
re-ACK 329a0dcdafe05002f662e8737a76bfdeaba9a3ed
Tree-SHA512: 4bbb807a1e632694863c1a1fa2e93cc5a756b19f8d78f0642ebe7ffafb01835765fa66c76a680dc6f3c412a5abb0c4a33fb7212c26b4b2d80b6b3b7ee8284b2e
This uses a macro, which can be a bit more brittle than an alias
template. However, class template argument deduction for alias templates
is only implemented in clang-19.
568fcdddaec2cc8decba5a098257f31729cc1caa scripted-diff: Adjust documentation per top-level target output location (Hennadii Stepanov)
026bb226e96919603af829d0b677779a234a0f6e cmake: Set top-level target output locations (Hennadii Stepanov)
Pull request description:
This PR sets the target output locations to the `bin` and `lib` subdirectories within the build tree, creating a directory structure that mirrors that of the installed targets.
This approach is widely adopted by the large projects, such as [LLVM](e146c1867e/lldb/cmake/modules/LLDBStandalone.cmake (L128-L130)):
```cmake
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
```
The `libsecp256k1` project has also recently [adopted](https://github.com/bitcoin-core/secp256k1/pull/1553) this approach.
With this PR, all binaries are conveniently located. For example, run:
```
$ ./build/bin/fuzz
```
instead of:
```
$ ./build/src/test/fuzz/fuzz
```
On Windows, all required DLLs are now located in the same directory as the executables, allowing to run `bitcoin-chainstate.exe` (which loads `bitcoinkernel.dll`) without the need to copy DLLs or modify the `PATH` variable.
The idea was briefly discussed among the build team during the recent CoreDev meeting.
---
**Warning**: This PR changes build locations of newly built executables like `bitcoind` and `test_bitcoin` from `src/` to `bin/` without deleting previously built executables. A clean build is recommended to avoid accidentally running old binaries.
ACKs for top commit:
theStack:
Light re-ACK 568fcdddaec2cc8decba5a098257f31729cc1caa
ryanofsky:
Code review ACK 568fcdddaec2cc8decba5a098257f31729cc1caa. Only change since last review was rebasing. I'm ok with this PR in its current form if other developers are happy with it. I just personally think it is inappropriate to \*silently\* break an everyday developer workflow like `git pull; make bitcoind`. I wouldn't have a problem with this PR if it triggered an explicit error, or if the problem was limited to less common workflows like changing cmake options in an existing build.
TheCharlatan:
Re-ACK 568fcdddaec2cc8decba5a098257f31729cc1caa
theuni:
ACK 568fcdddaec2cc8decba5a098257f31729cc1caa
Tree-SHA512: 1aa5ecd3cd49bd82f1dcc96c8e171d2d19c58aec8dade4bc329df89311f9e50cbf6cf021d004c58a0e1016c375b0fa348ccd52761bcdd179c2d1e61c105e3b9f
This pull request fixes a word order error in developer-notes.md.
Before:
"In cases where do you call .c_str(), you might want to additionally check that the string does not contain embedded '\0' characters..."
After:
"In cases where you do call .c_str(), you might want to additionally check that the string does not contain embedded '\0' characters..."
Explanation:
The sentence had incorrect word order, making it grammatically incorrect. Rearranging "do you" to "you do" corrects the sentence, improving the readability and clarity of the documentation.
replaced --enable-debug with -DCMAKE_BUILD_TYPE=Debug in developer-notes
replaced --enable-multiprocess with -DWITH_MULTIPROCESS=ON
replaced --disable-zmq with -DWITH_ZMQ=OFF
0b003e1ff7e09b56cd013b15f1998495994b7211 docs: Updated debug build instructions for cmake (ion-)
Pull request description:
In the [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md) the section on [compiling for debug](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-debugging) displays outdated instructions that were applicable before the move to cmake build system.
This PR just gives instructions on how to build for debugging in the context of cmake.
ACKs for top commit:
kevkevinpal:
ACK [0b003e1](0b003e1ff7)
achow101:
ACK 0b003e1ff7e09b56cd013b15f1998495994b7211
tdb3:
ACK 0b003e1ff7e09b56cd013b15f1998495994b7211
Tree-SHA512: fc8b4824d68e47b3e64320b4ce728df4242d00c2e8c75dcf5daa60af3050c62d94fa9741e447c7d81e72194470ed68a4e19bbee8cf6a1412d6566c7b699cf242
6a68343ffbf3291eb243d90c00df50e672ff3944 doc: Prepend 'build/' to binary paths under 'src/' in docs (Lőrinc)
91b3bc2b9c572d9244e1cf542655c1b32e87aaf7 doc: Update documentation generation example in developer-notes.md (Lőrinc)
Pull request description:
In [the other readmes](6ce50fd9d0/src/test/README.md (L19)) we've provided a default build directory instead, unified the `developer-notes.md` to specify it explicitly.
In the next commit I've used this default to go over each reference to our binaries and changed their in-source references to the build directory.
Some of these changes were in example outputs - I haven't validated that the outputs are still the same.
I haven't modified the build folders in the devtools.
ACKs for top commit:
maflcko:
review ACK 6a68343ffbf3291eb243d90c00df50e672ff3944
pablomartin4btc:
ACK 6a68343ffbf3291eb243d90c00df50e672ff3944
fanquake:
ACK 6a68343ffbf3291eb243d90c00df50e672ff3944 - we still need to followup with other scripts/devtools, and likely unify what we are doing in some way, but this is an improvement.
Tree-SHA512: 905d9c68cafe1e405e98d6aa089d7a36a34c9e03403df5c67ac2c9a98cfa54a0305b647cb92247dcb9f49e9b509a8ba88367392b95618c67059684c67b6c36fb
