💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742288290)
`gmake` is an accepted alias for `make` when it refers to GNU Make, which is required to build depends.

Also please refer to https://packages.ubuntu.com/noble/amd64/make/filelist.

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742289121)
> I think this can be dropped from the commit message

Done, your right, the formulation wasn't the best

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742290292)
The iwyu changes were a bit excessive, but kept the ones that are related to this PR, thanks!

💬 ryanofsky commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#discussion_r1742290676)
Good point that "overridden" is probably more appropriate word, but I think I already understood that this meant overridden.

The thing I don't understand is what causes MSAN_FLAGS to be overridden when FLAGS_DEBUG variables are unset, but not overridden when they are empty? Appreciate the clarification. I'm not asking for any particular reason, just curious to understand this better.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742291673)
I don't recall installing Wine.

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742291832)
Pushed some changes, let me know if it's any better, or if you insist on placing them in the cpp file in a different place than the header (with some explanations)

💬 stickies-v commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742293570)
Isn't that just a very expensive way to do `assert(random_hex_string == ToLower(result->ToString())`? (and also, that's not `hex` testing, but `uint256` testing)

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742293594)
Fair, added const

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742294557)
Used the `@{` and `@}` formats

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742294935)
Use the version in my edited comment above

💬 achow101 commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2326857491)
Anyone have an asic want to reorg that?

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742295498)
nice! changed

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742295827)
Lack of fuzzing engines for MSVC?

💬 stickies-v commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742299214)
Again, here, I don't think the `const` is noise. This variable is passed to multiple functions, and verifying no mutation happens is an unnecessary burden on the developer. Why not just lean on the compiler?

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742300226)
isn't the point of these tests that we shouldn't have know the internals (i.e. black box)?
(though in this case that wouldn't even be true since `random_hex_string` can start with `0x` for `FromUserHex` which isn't added back via `ToString`)

⚠️ TheCharlatan opened an issue: "cmake: Installed static kernel library is unusable"
(https://github.com/bitcoin/bitcoin/issues/30801)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Since the move to cmake, the kernel static library that is installed after a `cmake --install build` is unusable. It basically lacks all symbols, besides those defined in the kernel library target. This was a problem before where some libtool and symbol visibility [issues](https://github.com/bitcoin/bitcoin/issues/25008#issuecomment-1440326006) led to the libsecp static library having to b
...

💬 ryanofsky commented on issue "cmake: incorrect assumption that `Debug` build type will not use optimisations":
(https://github.com/bitcoin/bitcoin/issues/30800#issuecomment-2326864201)
Not sure if this makes sense, but if the problem is that specifying _FORTIFY_SOURCE without optimizations triggers a compiler warning, maybe there should just be a check that drops _FORTIFY_SOURCE when it triggers the warning. That might avoid problems if there is a discrepancy between the way our cmake code detects optimizations and the way _FORTIFY_SOURCE implementation does.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1742301373)
https://github.com/bitcoin/bitcoin/blob/9cb9651d92ddb5d92724f6a52440601c7a0bbcf8/doc/design/libraries.md?plain=1#L7

💬 l0rinc commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742301624)
K, I don't feel strongly about it, pushed.

💬 stickies-v commented on pull request "test: increase FromUserHex FUZZ and unit testing coverage":
(https://github.com/bitcoin/bitcoin/pull/30618#discussion_r1742305814)
I don't think requiring `v` to have implemented `ToString()` is a good approach. Do you see any issues with `return v ? os << *v : os << "std::nullopt";`?