💬 theuni commented on issue "build: `cmake --install` fails if only select targets are built":
(https://github.com/bitcoin/bitcoin/issues/31745#issuecomment-2619684279)
This is frustrating, but it seems to be how CMake works. `cmake --install build` doesn't force any builds, but `make -C build install` does.

I think what you're really looking for is a way to only build/install certain binaries. For that, the workaround would be to name each one as a separate component. That would allow for:
```cmake
cmake -B build
cmake --build build --target bitcoind bitcoin-cli
cmake --install build --component bitcoind bitcoin-cli
```

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#issuecomment-2619701583)
The idea that an executable placed in `libexec/` should be called by other executables is a good rule of thumb but not a requirement of some kind. (And if it were a requirement, it would be satisfied by #31375.)

The technical difference between binaries installed to `bin/` and binaries installed to `libexec/` is that binaries in `bin/` get exposed on the `PATH` while binaries in `libexec/` don't.

There is nothing wrong with binaries in `libexec/` being documented and called used by users a
...

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1932643967)
I changed the comment to "Invalid blocks and their descendants must be marked as invalid" - doesn't really make sense to me to make an exception for the block itself.

💬 ryanofsky commented on issue "build: depends cros-compile using Clang fails":
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2619724098)
Maybe dumb question but are you building on a x86_64 system where `clang` and `clang++` executables produce x86_64 binaries? If so I would guess this is caused by your `CC=clang CXX=clang++` variables.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1932645135)
Done, made it a bit shorter:
`it->second->nStatus = (it->second->nStatus & ~BLOCK_FAILED_VALID) | BLOCK_FAILED_CHILD;`

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2619726595)
[b2136da](https://github.com/bitcoin/bitcoin/commit/b2136da98df61f10ff8283d42f112f69f041fa7a) to [4ba2e48](https://github.com/bitcoin/bitcoin/commit/4ba2e480ffa0b77113953bee4ff5c9349e277e7e):
Addressed review comments by @stratospher (Thanks!) and rebased.

💬 theuni commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2619837571)
> > @ryanofsky Please have a look here: [theuni@f2d7447](https://github.com/theuni/bitcoin/commit/f2d74475433528c81c42af3232c257c443c1fc94)
> > It's a quick hack to demonstrate the concept, please ignore the rough edges and consider the concept alone. I'll clean it up if you're interested.
>
> I feel like I'm missing a lot of context which led up to that implemention, and also I wonder if files are missing from that commit because just shows a new `CopyOrGenerate.cmake` script that does not
...

💬 theuni commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2619854239)
> I think the depends source-package thing is elegant and neat. But in the cross-compiled case, it effectively nullifies the effect of vendoring the code. Changes to the local libmultiprocess won't be picked up without rebuilding depends. I think this could be improved by always using our internal target libmultiprocess, but selectively with an internel/external native one? In that case, I don't think the non-native libmultiprocess in depends needs to exist?

Basically (unless I'm missing some
...

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1932753931)
> `@param[out] cancel_recv Should always be set upon return`

I think the problem was that `HTTPServer::EventReadyToSend()` could end up not setting `cancel_recv` in which case it remains uninitialized, containing a "random" value from the stack at the caller (which was probably `true`, so it canceled all receives from the client, never receiving anything causing an infinite loop):

```cpp
bool cancel_recv;

EventReadyToSend(node_id, cancel_recv);


...

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1932762333)
rpcwhitelistdefault=1 revokes permissions for all users, including the __cookie__ user. Since the cookie user is required during the node restart process (for getblockcount and getmempoolinfo), I had to explicitly whitelist

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1932768623)
I've just realized that I need to remove` getblockchaininfo `from the whitelist, it's unused.

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#discussion_r1932801596)
this function will fail without the whitelist https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_framework/test_node.py#L264

💬 pinheadmz commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1932812651)
awesome thank you

💬 pinheadmz commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2619994736)
Something else I see using Sockman is in the `mempool_limit` functional test, we call `getrawtransaction` and expect around 540,000 bytes back. The call to `send()` returns the total amount of bytes immediately, but it requires 60-70 TCP packets to actually transmit the response. I think what I'm seeing in wireshark is that this transmission is incomplete when I close the HTTP connection with `CloseConnection()` which closes the underlying socket. The client waits 120 seconds for the rest of its
...

💬 darosior commented on pull request "rpc: fix mintime field testnet4":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2619996977)
What's the state of this? I'd love this to not miss the 29.0 deadline, in order to be as widely available to miners if a timewarp fix ever gets proposed, and given that miners may take years to upgrade.

@Sjors's last message reads like he agrees with me and was going to make the change, so i was waiting on that. Pinged him and turns out he was waiting to see if anybody was going to chime in about his last message. @fjahr @tdb3 since you previously ACKed this do you have an opinion? @luke-jr a
...

💬 naiyoma commented on pull request "test: Add test for rpcwhitelistdefault":
(https://github.com/bitcoin/bitcoin/pull/29858#issuecomment-2620001641)
hey @ismaelsadeeq and @rkrux thanks for the reviews! I'll be sure to incorporate the suggestion in the follow-up.

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2620013263)
Thanks for the feedback and clearing things up. Will be interested to see the EXTERNAL_MPGEN option and maybe eliminate one of the depends packages.

> I think the depends source-package thing is elegant and neat. But in the cross-compiled case, it effectively nullifies the effect of vendoring the code. Changes to the local libmultiprocess won't be picked up without rebuilding depends.

I don't think "effectively nullifies the effect of vendoring" is true at all, unless you have some differe
...

💬 luke-jr commented on pull request "rpc: fix mintime field testnet4":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2620052882)
"allowed" isn't necessarily consensus rules. It would be strictly BIP 23 compatible to just reject rpc submissions with an earlier time. I don't think we need to go that far though - probably fine to tolerate it

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2620090133)
Few other things I wanted to reply to.

re: https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2619837571

> Correct. Even if it's not desirable to check these into master, it would be _very_ useful to have them in release branches so that they end up in release tarballs. It's very common for source packages to be pre-bootstrapped, and that's exactly what this would do.

Honestly as a long time gentoo and nixos user, this sounds pretty bad to me. I want my package manager to build
...

⚠️ texasspurstar opened an issue: "build: broken CMake *flags output"
(https://github.com/bitcoin/bitcoin/issues/31749)
### Please describe the feature you'd like to see added.

Seen in CI runs, i.e https://cirrus-ci.com/task/5616358949388288?logs=configure#L309:

C++ compiler flags .................... -O2 -g -std=c++20 -fPIC -fdebug-prefix-map=/tmp/cirrus-ci-build/bitcoin-core/src=. -fmacro-prefix-map=/tmp/cirrus-ci-build/bitcoin-core/src=. -Werror $<$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>:SHELL:-Xcompiler -pthread> $<$<AND:$<NOT:$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>>,$<NOT:$<COMPILE_LANGUAGE:Swift>>>:-pthread> -Wall -
...