💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2647374943)
It is better to post such suggestions not in the main thread of the PR but as a comment to some line of code, even if that is a random, unrelated line. That way replies will be grouped together with the questions instead of being scattered around in the main PR thread. In the main thread it is easier to forget to reply to some questions. As comments to some line of code, they can be tracked and eventually "resolved" to collapse them and reduce the main PR thread noise.

> #### WinSock
>
> W
...

💬 Sjors commented on pull request "refactor: Use std::span over Span":
(https://github.com/bitcoin/bitcoin/pull/31519#issuecomment-2647388325)
Concept ACK. I'll need to rebase some things, but I'm favor of getting that over with.

💬 hebasto commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1948687873)
1b3613505deb4baeacc3c4c2fc91ccca1a8ecec5

Usually, external dependencies follow the internal ones.

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948692321)
Done, will be in next push.

💬 hebasto commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2647421895)
> > Could the `CAPNP_EXECUTABLE`, `CAPNPC_CXX_EXECUTABLE` and all `CapnProto_*` CMake variables be marked as advanced to avoid cluttering the CMake cache view when inspected by the user?
>
> Sure, done in latest push.

The `CapnProto_*` CMake variables are still lingering:
```
$ cmake -B build -DENABLE_IPC=ON -L 2>&1 | grep CapnProto_
CapnProto_capnp-json_IMPORTED_LOCATION:FILEPATH=/usr/lib/x86_64-linux-gnu/libcapnp-json-1.0.1.so
CapnProto_capnp-rpc_IMPORTED_LOCATION:FILEPATH=/usr/lib/x
...

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948716391)
By default trying to pass `std::span<std::byte>` to a function that takes `std::span<uint8_t>` gives:

```
no known conversion from 'span<std::byte>' to 'span<uint8_t>'
```

If I really insist then I can convert it like:

```cpp
void f(std::span<uint8_t> s);
...
std::span<std::byte> a;
f(std::span<uint8_t>{reinterpret_cast<uint8_t*>(a.data()), a.size()});
```

I find it cleaner to use `uint8_t` because it is already used in the existent code (`ReceiveMsgBytes()`) and avoid such fo
...

💬 hebasto commented on pull request "depends: Use `CC_FOR_BUILD` for `config.guess `":
(https://github.com/bitcoin/bitcoin/pull/29963#issuecomment-2647448250)
@theuni

> > However, switching the CC variable context from Makefile to the shell environment breaks expectations
>
> Arguably that's because this wasn't intended to be supported :)
>
> I suppose this fix is reasonable, though supporting env vars like this seems quite brittle.

There are testing environments, such as OSS-Fuzz, that rely on a project's build system to support environment variables.

💬 hebasto commented on pull request "depends: Make default `host` and `build` comparable":
(https://github.com/bitcoin/bitcoin/pull/30584#issuecomment-2647452355)
Friendly ping @laanwj @josibake @theuni ;)

💬 hebasto commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2647463681)
Friendly ping @theuni :)

⚠️ Sjors opened an issue: "ci: intermittent p2p_tx_download.py timeout (test_preferred_tiebreaker_inv)"
(https://github.com/bitcoin/bitcoin/issues/31833)
Seen in: https://github.com/bitcoin/bitcoin/actions/runs/13236532677/job/36942441812?pr=30975 (#30975)

```
test 2025-02-10T09:21:22.592000Z TestFramework (INFO): Test that preferred peers are always selected over non-preferred when ready
test 2025-02-10T09:21:22.592000Z TestFramework.node0 (DEBUG): Stopping node
node0 2025-02-10T09:21:22.593102Z [http] [httpserver.cpp:306] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:57336
node0 2025-02-10T09:21:22.593212Z [http
...

💬 Sjors commented on issue "qa: Functional tests are intrinsic vulnerable to timeouts":
(https://github.com/bitcoin/bitcoin/issues/19732#issuecomment-2647473725)
Is #31833 a symptom of this?

💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2647477699)
Reported spurious CI failure in #31833. Otherwise it looks good.

💬 hebasto commented on pull request "test: Move `script_assets_tests` into its own suite":
(https://github.com/bitcoin/bitcoin/pull/31576#issuecomment-2647481085)
Friendly ping @maflcko ;)

🚀 fanquake merged a pull request: "build: consistently use `CLIENT_NAME` in libbitcoinkernel.pc.in"
(https://github.com/bitcoin/bitcoin/pull/31820)

💬 hodlinator commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1948755987)
nit: Could add a warning when skipping the test because of user permissions:
```suggestion
if platform.system() != 'Windows':
if os.geteuid() == 0:
self.log.warning('Skipping "permission denied"-test requiring non-root user.')
else:
```

(Ran into something similar on my own branch as sanitizer-tests apparently run with root permissions ([CI log](https://github.com/hodlinator/bitcoin/actions/runs/13219450078/job/36902673995#step:7:5018))).

🚀 fanquake merged a pull request: "TxOrphanage: account for size of orphans and count announcements"
(https://github.com/bitcoin/bitcoin/pull/31810)

💬 hebasto commented on pull request "guix: remove test-security/symbol-check scripts":
(https://github.com/bitcoin/bitcoin/pull/31818#discussion_r1948766776)
The block above, which sets the `exe_format` variable, can now be deleted entirely.

💬 hodlinator commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1948784579)
Certainly won't insist, but another possibility is to use `bit_cast` for trivially constructible types:
```c++
void f(std::span<uint8_t> s);
...
std::span<std::byte> a;
f(std::bit_cast<std::span<uint8_t>>(a));
```

💬 fanquake commented on pull request "guix: remove test-security/symbol-check scripts":
(https://github.com/bitcoin/bitcoin/pull/31818#discussion_r1948786940)
Dropped.

✅ fanquake closed a pull request: "build: fix MSVC ccache reporting"
(https://github.com/bitcoin/bitcoin/pull/31813)