🤔 paplorinc reviewed a pull request: "build: Introduce CMake-based build system"
(https://github.com/bitcoin/bitcoin/pull/30454#pullrequestreview-2231417621)
👍 left a few things I've noticed, ignore the nits of out of scope

💬 Sjors commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2281017182)
@maflcko you theory might somewhat right.

When my `/usr/local/bin/guix` was at 0.4.0 the first thing it wanted to build was guile-3.0.7.

Now that I built the most recent master commit, it starts out building guile-3.0.9.

However under `--list-generations` I'm only seeing the version I pulled, not the one I built from source. I then did a guix pull on that commit, which ended up building a few more derivations.

So perhaps it's only building guile or some other code component? cc @theu
...

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712625608)
Could you elaborate please? What are the benefits of maintaining the build system in separate projects? Is this practice used by other open-source projects?

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100)
Rebased due to the [conflicts](https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2277190642). More feedback has been addressed.

> (The drahtbot guix build failed due to a silent merge conflict, I presume)

Fixed.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626562)
Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626637)
Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626646)
Might be controversial, but other Bitcoin clones seem to split into multiple projects after their CMake migrations:
* https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/src/bench/CMakeLists.txt#L3
* https://github.com/bitcoin-cash-node/bitcoin-cash-node/blob/master/src/bench/CMakeLists.txt#L3
* https://github.com/bitcoin-sv/bitcoin-sv/blob/master/src/test/CMakeLists.txt#L5

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626661)
Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712626746)
Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712627020)
> I wasn't able to run the fuzz tests with cmake

Resolved in https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2281111100.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712627346)
Still curios about:
> What are the benefits of maintaining the build system in separate projects?

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712628124)
I'm also curious, that's what I'm asking basically, why did the other clones decide to do this differently.

I could only find general `Modularity and Reusability/Scalability/Improved Dependency Management/Isolation of Build Configurations` arguments online, but not sure any applies here, hence my question.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712633831)
> Might be controversial, but other Bitcoin clones seem to split into multiple projects after their CMake migrations:
>
> * https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/src/bench/CMakeLists.txt#L3

It is not controversial, just broken :)

The `cmake -S src/bench -B build` command fails.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712637404)
See https://github.com/hebasto/bitcoin/pull/320.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712637433)
See https://github.com/hebasto/bitcoin/pull/320.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712637443)
See https://github.com/hebasto/bitcoin/pull/320.

💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#issuecomment-2281899749)
This change didn't appear to have any speed impact for a reindex-chainstate using max dbcache. Ran the following benchmark:
```
nohup hyperfine \
--export-markdown ~/bench.md \
--show-output \
--parameter-list commit 81508954f1f89f95b6fdca10c3c471cdb6566c80,27a770b34b8f1dbb84760f442edb3e23a0c2420b \
--prepare 'git checkout {commit} && \
make -j$(nproc) src/bitcoind; \
sync; sudo /sbin/sysctl vm.drop_caches=3;' \
-M 2 \
'echo '{commit}' && \
./src/bitcoind -datadir=/home/user/.bitcoin
...

💬 1440000bytes commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2282178130)
Concept ACK

📝 maflcko opened a pull request: "doc: Remove outdated nTx faking comment"
(https://github.com/bitcoin/bitcoin/pull/30624)
This problematic `nTx` "faking" was removed in commit b50554babdddf452acaa51bac757736766c70e81.

So fix the wrong comment.

Also, address the typo nits from:

* https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1531789314
* https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1711982543

💬 maflcko commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1712660234)
Done as part of https://github.com/bitcoin/bitcoin/pull/30624