📝 LarryRuane opened a pull request: "doc: cmake: prepend "build" to functional/test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/30859)
This is a small follow-on to #30741. Also, improve the error message if someone runs the functional tests the old way (outside of the build directory).
(https://github.com/bitcoin/bitcoin/pull/30859)
This is a small follow-on to #30741. Also, improve the error message if someone runs the functional tests the old way (outside of the build directory).
💬 LarryRuane commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2339578313)
This patch would have helped me; I'm new to `cmake`; after building it for the first time, I tried to run the functional tests (the way I was used to), and it failed in a strange way:
```
$ test/functional/test_runner.py
Traceback (most recent call last):
File "/sd/g/bitcoin/test/functional/test_runner.py", line 950, in <module>
main()
File "/sd/g/bitcoin/test/functional/test_runner.py", line 465, in main
config.read_file(open(configfile, encoding="utf8"))
...
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2339578313)
This patch would have helped me; I'm new to `cmake`; after building it for the first time, I tried to run the functional tests (the way I was used to), and it failed in a strange way:
```
$ test/functional/test_runner.py
Traceback (most recent call last):
File "/sd/g/bitcoin/test/functional/test_runner.py", line 950, in <module>
main()
File "/sd/g/bitcoin/test/functional/test_runner.py", line 465, in main
config.read_file(open(configfile, encoding="utf8"))
...
🤔 tdb3 reviewed a pull request: "doc: cmake: prepend "build" to functional/test_runner.py"
(https://github.com/bitcoin/bitcoin/pull/30859#pullrequestreview-2291346092)
Thanks for noticing this. I'd support the documentation change without changing the python programs.
(https://github.com/bitcoin/bitcoin/pull/30859#pullrequestreview-2291346092)
Thanks for noticing this. I'd support the documentation change without changing the python programs.
💬 tdb3 commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1751241259)
This extra line seems like overkill if the proceeding line is being updated to include "build"
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1751241259)
This extra line seems like overkill if the proceeding line is being updated to include "build"
💬 tdb3 commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1751243701)
Seems like it might be enough to update the instructions rather than change `test_framework.py` or `test_runner.py`
(https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1751243701)
Seems like it might be enough to update the instructions rather than change `test_framework.py` or `test_runner.py`
💬 achow101 commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2339623497)
Not really that much faster: https://cirrus-ci.com/build/5557255590903808, but possibly also misconfigured as everything was setup to run on that one machine simultaneously, although it should enough cores and memory.
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2339623497)
Not really that much faster: https://cirrus-ci.com/build/5557255590903808, but possibly also misconfigured as everything was setup to run on that one machine simultaneously, although it should enough cores and memory.
💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2339651980)
Hmm, you could also try to re-run the tasks individually to see the best possible performance (which is what was tested above as well).
Otherwise, my suggestion would be to go with one of the AMD CPUs mentioned above. (I can give this a try in the coming days)
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2339651980)
Hmm, you could also try to re-run the tasks individually to see the best possible performance (which is what was tested above as well).
Otherwise, my suggestion would be to go with one of the AMD CPUs mentioned above. (I can give this a try in the coming days)
💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1751290182)
Yes. I am thinking that it could make sense to have a wrapper-header around tinyformat, so that each relevant tfm function can be called with `ConstevalFormatString`.
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1751290182)
Yes. I am thinking that it could make sense to have a wrapper-header around tinyformat, so that each relevant tfm function can be called with `ConstevalFormatString`.
💬 maflcko commented on issue "Trying to run bitcoin qt on Windows and getting an AV":
(https://github.com/bitcoin/bitcoin/issues/30825#issuecomment-2339720856)
It would also be good to confirm the exact commit hash where this happened, because `master` may have changed when it was pulled and when the issue was filed (and certainly has changed now).
(https://github.com/bitcoin/bitcoin/issues/30825#issuecomment-2339720856)
It would also be good to confirm the exact commit hash where this happened, because `master` may have changed when it was pulled and when the issue was filed (and certainly has changed now).
💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block":
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2339769505)
Trivial rebase after #30509.
(https://github.com/bitcoin/bitcoin/pull/30409#issuecomment-2339769505)
Trivial rebase after #30509.
💬 Sjors commented on pull request "multiprocess: Add -ipcbind option to bitcoin-node":
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2339814474)
Post-merge re-utACK 30073e6b3a24cbe417c45cd5df6a3a2de0251e9d
(https://github.com/bitcoin/bitcoin/pull/30509#issuecomment-2339814474)
Post-merge re-utACK 30073e6b3a24cbe417c45cd5df6a3a2de0251e9d
💬 stickies-v commented on pull request "test: support std::optional in BOOST_CHECK_* and increase FromUserHex fuzz feature coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#issuecomment-2339825378)
rebase re-ACK 6da47f6ba5450e7f2b2ab2a64c7cfd354f67a1a9, CI timeout failure seems unrelated
(https://github.com/bitcoin/bitcoin/pull/30618#issuecomment-2339825378)
rebase re-ACK 6da47f6ba5450e7f2b2ab2a64c7cfd354f67a1a9, CI timeout failure seems unrelated
💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1751353778)
988dacead4a9a6850b767a8ced0c08b47fece56d: can you add a comment for why this isn't in `src/test/CMakeFile.txt`? A tl&dr of https://github.com/bitcoin/bitcoin/pull/30510/files#r1747656292
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1751353778)
988dacead4a9a6850b767a8ced0c08b47fece56d: can you add a comment for why this isn't in `src/test/CMakeFile.txt`? A tl&dr of https://github.com/bitcoin/bitcoin/pull/30510/files#r1747656292
💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1751359880)
On Intel macOS 14.6.1 this doesn't work for me.
```
git clean -dfx
cmake -B build -DWITH_MULTIPROCESS=true
...
-- Configuring done (29.3s)
CMake Error at src/CMakeLists.txt:335 (add_dependencies):
The dependency target "bitcoin_ipc_headers" of target "bitcoin_ipc_test"
does not exist.
```
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1751359880)
On Intel macOS 14.6.1 this doesn't work for me.
```
git clean -dfx
cmake -B build -DWITH_MULTIPROCESS=true
...
-- Configuring done (29.3s)
CMake Error at src/CMakeLists.txt:335 (add_dependencies):
The dependency target "bitcoin_ipc_headers" of target "bitcoin_ipc_test"
does not exist.
```
💬 willcl-ark commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2339988301)
> Hmm, you could also try to re-run the tasks individually to see the best possible performance (which is what was tested above as well).
Correct, I used default `cirrus-cli` concurrency (of 1) on the server during testing. Some of these were second runs but ccache was not hit in all of them. E.g. it hit 0% in `depends, debug`, but 100% in `nowallet, libbitcoinkernel`. The run is here: https://cirrus-ci.com/build/6496417768800256
IMO that @achow101 didn't see a great speedup further hints
...
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2339988301)
> Hmm, you could also try to re-run the tasks individually to see the best possible performance (which is what was tested above as well).
Correct, I used default `cirrus-cli` concurrency (of 1) on the server during testing. Some of these were second runs but ccache was not hit in all of them. E.g. it hit 0% in `depends, debug`, but 100% in `nowallet, libbitcoinkernel`. The run is here: https://cirrus-ci.com/build/6496417768800256
IMO that @achow101 didn't see a great speedup further hints
...
📝 BrandonOdiwuor opened a pull request: "test: autogenerate bash completion"
(https://github.com/bitcoin/bitcoin/pull/30860)
Fixes https://github.com/bitcoin/bitcoin/issues/17289, and follows up on https://github.com/bitcoin/bitcoin/pull/18606
Adds a functional test that parses available RPC commands, generates the associated bitcoin-cli autocomplete file and checks that the current autocomplete file matches.
An outdated autocomplete file can be updated via the --overwrite test parameter.
(https://github.com/bitcoin/bitcoin/pull/30860)
Fixes https://github.com/bitcoin/bitcoin/issues/17289, and follows up on https://github.com/bitcoin/bitcoin/pull/18606
Adds a functional test that parses available RPC commands, generates the associated bitcoin-cli autocomplete file and checks that the current autocomplete file matches.
An outdated autocomplete file can be updated via the --overwrite test parameter.
💬 willcl-ark commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2340006739)
Another potential (and cheap) avenue we could explore is having a single massive shared ccache dir. This of course wouldn't speed up running tests themselves, but we could try for even more cache hits during compilation.
Hetzner has a "storage box" https://www.hetzner.com/storage/storage-box/ which, for 4€ per month gets unlimited internal traffic and 1TB of disk space. I don't much like the look of "10 concurrent connections" max, and unsure if it's SSD or spinning disk, but supposing the be
...
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2340006739)
Another potential (and cheap) avenue we could explore is having a single massive shared ccache dir. This of course wouldn't speed up running tests themselves, but we could try for even more cache hits during compilation.
Hetzner has a "storage box" https://www.hetzner.com/storage/storage-box/ which, for 4€ per month gets unlimited internal traffic and 1TB of disk space. I don't much like the look of "10 concurrent connections" max, and unsure if it's SSD or spinning disk, but supposing the be
...
💬 hebasto commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2340054600)
Shouldn't we check for the absence of unfortified versions of functions, rather than checking for the presence of fortified ones?
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2340054600)
Shouldn't we check for the absence of unfortified versions of functions, rather than checking for the presence of fortified ones?
💬 fanquake commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2340063318)
> Shouldn't we check for the absence of unfortified versions of functions, rather than checking for the presence of fortified ones?
Can you explain how that test would work? Not all functions are gauranteed to be fortified.
(https://github.com/bitcoin/bitcoin/pull/27038#issuecomment-2340063318)
> Shouldn't we check for the absence of unfortified versions of functions, rather than checking for the presence of fortified ones?
Can you explain how that test would work? Not all functions are gauranteed to be fortified.
💬 fanquake commented on pull request "security-check: test for `_FORTIFY_SOURCE` usage in release binaries":
(https://github.com/bitcoin/bitcoin/pull/27038#discussion_r1751550882)
Yea, it's a bit annoying. At least post-BDB, it'll just be "ignore the stack protector".
(https://github.com/bitcoin/bitcoin/pull/27038#discussion_r1751550882)
Yea, it's a bit annoying. At least post-BDB, it'll just be "ignore the stack protector".