💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712605045)
Q: given that FUZZ and BENCH are disabled by default, would it maybe make sense to make them separate projects? I'm not exactly familiar with every single second-order effect of that and don't have strong arguments either way, was just wondering when investigating some other cmake related issue.

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712621112)
Nit, feel free to ignore these if not important:
```suggestion
rpc/spend.cpp
rpc/signmessage.cpp
```

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712605154)
nit: alphabetic ordering?
```patch
diff --git a/src/crypto/CMakeLists.txt b/src/crypto/CMakeLists.txt
--- a/src/crypto/CMakeLists.txt (revision 92f7fc21768b0d898655d68b5d76ff09ff71c682)
+++ b/src/crypto/CMakeLists.txt (date 1723279760256)
@@ -4,19 +4,19 @@

add_library(bitcoin_crypto STATIC EXCLUDE_FROM_ALL
aes.cpp
- chacha20.cpp
chacha20poly1305.cpp
+ chacha20.cpp
hex_base.cpp
hkdf_sha256_32.cpp
hmac_sha256.cpp
hmac_sha512.cpp
- poly1305.cpp
muhash.cpp
...

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712621771)
Nit:
```patch
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
--- a/src/CMakeLists.txt (revision 92f7fc21768b0d898655d68b5d76ff09ff71c682)
+++ b/src/CMakeLists.txt (date 1723286445028)
@@ -106,8 +106,8 @@
addresstype.cpp
base58.cpp
bech32.cpp
- chainparamsbase.cpp
chainparams.cpp
+ chainparamsbase.cpp
coins.cpp
common/args.cpp
common/bloom.cpp
@@ -130,18 +130,18 @@
key.cpp
key_io.cpp
merkleblock.cpp
+ net_permissions.cpp
net_types.cpp
...

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1712620747)
> Apparently, not every change invalidates the cache

My understanding is that we're already avoiding the recompilation when the source didn't change, but when `working_linker_werror_flag` changes, that's ignored.
I haven't opened a bug report to CMake since it seems to me we can fix it ourselved, see: https://github.com/hebasto/bitcoin/pull/319

🤔 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.