Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 theuni commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#discussion_r1892519435)
😬
💬 theuni commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#discussion_r1892536668)
Why drop the assert? Though I have no idea who/what tests with `-DDEBUG` anyway.
📝 hebasto opened a pull request: "qa: Use `sys.executable` when invoking other Python scripts"
(https://github.com/bitcoin/bitcoin/pull/31541)
This PR fixes the `rpc_signer.py` and `wallet_signer.py` functional tests on systems where `python3` is not available in the `PATH`, causing the shebang `#!/usr/bin/env python3` to fail.

Here are logs on NetBSD 10.0:
- without this PR (only https://github.com/bitcoin/bitcoin/pull/31537 branch):
```
$ python3.12 ./build/test/functional/test_runner.py rpc_signer.py wallet_signer.py
Temporary test directory at /tmp/test_runner_₿_🏃_20241219_160538
Remaining jobs: [rpc_signer.py, wallet_sign
...
💬 maflcko commented on pull request "refactor: std::span compat fixes":
(https://github.com/bitcoin/bitcoin/pull/31540#discussion_r1892617595)
The assert is not dropped. In fact, there are now two asserts:

* https://github.com/bitcoin/bitcoin/blob/fa494a1d53f3f030fafe7b533d72b2200428a0fd/src/span.h#L184
* https://github.com/bitcoin/bitcoin/blob/fa494a1d53f3f030fafe7b533d72b2200428a0fd/src/span.h#L207

The first one is the one you left a comment on. And the second one is left for the compiler to drop, if it wants to.
💬 hebasto commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2554942049)
A CI log from https://github.com/hebasto/bitcoin-core-nightly has been added to the PR description.
💬 theuni commented on pull request "depends: Fix compiling `libevent` package on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31500#issuecomment-2555160770)
LGTM, but yeah, it'd be helpful to upstream this.

FWIW c23 adds `typeof()`, so technically guards for that could be added. Not worth the complexity though.
💬 maflcko commented on pull request "qa: Use `sys.executable` when invoking other Python scripts":
(https://github.com/bitcoin/bitcoin/pull/31541#issuecomment-2555161398)
lgtm. even without netbsd, this is required to pass down the python version that was selected to run the tests. This is also used in all other places in the tests, like https://github.com/bitcoin/bitcoin/blob/bb57017b2945d5e0bbd95c7f1a9369a8ab7c6fcd/test/functional/test_runner.py#L632
👋 hebasto's pull request is ready for review: "qa: Use `sys.executable` when invoking other Python scripts"
(https://github.com/bitcoin/bitcoin/pull/31541)
💬 hebasto commented on pull request "qa: Use `sys.executable` when invoking other Python scripts":
(https://github.com/bitcoin/bitcoin/pull/31541#issuecomment-2555184010)
> lgtm. even without netbsd

Thanks! Rebased on the master branch.
💬 maflcko commented on pull request "fuzz: Fix misplaced SeedRand::ZEROS":
(https://github.com/bitcoin/bitcoin/pull/31521#issuecomment-2555198249)
> But (unfortunately?) the checker doesn't really check that. It only checks that if a fuzz test uses the RNG, then it must seed it with zeros at some point during the test. The checker doesn't check that seeding happens **before** using the RNG.

Sure, makes sense to add this assert as well
👍 theuni approved a pull request: "guix: latest 2.31 glibc"
(https://github.com/bitcoin/bitcoin/pull/31529#pullrequestreview-2515661465)
utACK b8710201fbd01bf2cba3c3b1fd312316b1ae22af
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1892975685)
> wen `s/Span/std::span/` all over the place?

now: https://github.com/bitcoin/bitcoin/pull/31519
💬 vasild commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#issuecomment-2555525700)
Concept ACK
📝 maflcko opened a pull request: " test: Embed univalue json tests in binary "
(https://github.com/bitcoin/bitcoin/pull/31542)
All other benchmarks and tests have their data embedded, except for the univalue json tests.

This is not only confusing, but also problematic, when the test binary is moved to a different system for testing, because one has to put the test files in the source dir that was used at compile-time.

Fix all issues by embedding them. Also, re-enable a disabled test. Also, fix an issue in the GenerateHeaderFromJson.cmake.

Requested in https://github.com/bitcoin/bitcoin/pull/31434/files#r1876000
...
📝 hebasto opened a pull request: "cmake: Always provide `RPATH` on NetBSD"
(https://github.com/bitcoin/bitcoin/pull/31543)
Apparently, runtime paths cannot be skipped on NetBSD, even for system-wide packages.

On NetBSD 10.0:
- on the master branch @ bb57017b2945d5e0bbd95c7f1a9369a8ab7c6fcd:
```
$ cmake -B build -DCMAKE_C_COMPILER="/usr/pkg/gcc14/bin/gcc" -DCMAKE_CXX_COMPILER="/usr/pkg/gcc14/bin/g++"
$ cmake --build build
$ ./build/src/bitcoin-wallet -version
./build/src/bitcoin-wallet: Shared object "libsqlite3.so.0" not found
$ cmake --install build --prefix /home/hebasto/INSTALL
$ /home/hebasto/INSTALL/
...
💬 hebasto commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2555661793)
Tracked in https://github.com/hebasto/bitcoin-core-nightly/issues/6.
💬 hebasto commented on pull request "qa: Use `sys.executable` when invoking other Python scripts":
(https://github.com/bitcoin/bitcoin/pull/31541#issuecomment-2555662120)
Tracked in https://github.com/hebasto/bitcoin-core-nightly/issues/6.
💬 hebasto commented on pull request "cmake: Always provide `RPATH` on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2555662426)
Tracked in https://github.com/hebasto/bitcoin-core-nightly/issues/6.
💬 theuni commented on pull request "cmake: Always provide `RPATH` on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31543#issuecomment-2555679006)
What makes NetBSD special in this regard?