Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🤔 hebasto reviewed a pull request: "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain"
(https://github.com/bitcoin/bitcoin/pull/31849#pullrequestreview-2617167523)
Concept ACK on using as many CMake's abstractions as possible.

We already use them in depends:https://github.com/bitcoin/bitcoin/blob/2549fc6fd1cc958a0e2d59838907c8808fd129b3/depends/funcs.mk#L194-L198

> ... we'd end up with a duplicated `--target=$ARCH-apple-darwin` on the compiler line

This will become a persistent source of confusion and complains, which could, of course, just be ignored. That was the only reason why I initially refrained from using `CMAKE_<LANG>_COMPILER_TARGET` whi
...
💬 hebasto commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#discussion_r1955802997)
Add `set(CMAKE_OBJCXX_COMPILER_TARGET @host@)`?
💬 TheCharlatan commented on pull request "Cleanups to port mapping module post UPnP drop":
(https://github.com/bitcoin/bitcoin/pull/31157#discussion_r1955803986)
Ah, I was irritated by the reference to the expiry, but it is only a couple of lines above, so I guess that is fine as is.
💬 hebasto commented on pull request "cmake: Improve safety and robustness during building `crc32c` subtree":
(https://github.com/bitcoin/bitcoin/pull/31779#issuecomment-2658718992)
> Another thought, isn't this going to break git bisect, or, result in having to do unclean subtree pulls? As the commit where you update the subtree will no-longer compile unless it also simultaneously changes the hash hardcoded into our build system.

Fair enough.
hebasto closed a pull request: "cmake: Improve safety and robustness during building `crc32c` subtree"
(https://github.com/bitcoin/bitcoin/pull/31779)
💬 Sjors commented on pull request "Prune mining interface":
(https://github.com/bitcoin/bitcoin/pull/31196#issuecomment-2658745672)
@goodthebest this pull request is not about pruning blocks. A better place to ask is https://bitcoin.stackexchange.com

But yes, it doesn't matter if you prune your node for mining and any value is fine (the minimum is `-prune=550` MB).
👍 TheCharlatan approved a pull request: "Cleanups to port mapping module post UPnP drop"
(https://github.com/bitcoin/bitcoin/pull/31157#pullrequestreview-2617235585)
ACK 70398ae05bc36a2788e87f67ae06962f43fe35a6

Just a nit: Maybe cleanup the dangling includes?
🚀 fanquake merged a pull request: "test: deduplicates p2p_tx_download constants"
(https://github.com/bitcoin/bitcoin/pull/31758)
🚀 fanquake merged a pull request: "depends: avoid an unset `CMAKE_OBJDUMP`"
(https://github.com/bitcoin/bitcoin/pull/31857)
💬 maflcko commented on pull request "contrib: Add deterministic-fuzz-coverage":
(https://github.com/bitcoin/bitcoin/pull/31836#issuecomment-2658779105)
Force pushed to add support for executables linked with libFuzzer
💬 Sjors commented on pull request "net: reduce CAddress usage to CService or CNetAddr":
(https://github.com/bitcoin/bitcoin/pull/31854#issuecomment-2658830814)
ACK cd4bfaee10
💬 fanquake commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2658854038)
> to list the stuff I noticed that is using GCC coverage:

The only thing mentioned here is corecheck. Is there anything else (other than your use)?

> Figure out what the coverage workflow should look like. "Should there be gcc support at all?"

If we are currently unsure what our coverage support should even look like, I'm not sure we should ship something just because it exists, maybe just to delete it in the next release; that's why it's attached to a milestone. It seems reasonable to come t
...
🚀 fanquake merged a pull request: "Cleanups to port mapping module post UPnP drop"
(https://github.com/bitcoin/bitcoin/pull/31157)
💬 fanquake commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#discussion_r1955899910)
Added.
💬 fanquake commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#issuecomment-2658901801)
> This will become a persistent source of confusion and complains,

It's only visible during a verbose build, and only happens when cross-compiling for macOS, on Linux. Which is certainly a less common build for anyone to actually be doing (outside of a Guix build, where it also wouldn't be visible).

> Would it be worth filtering out --target=$ARCH-apple-darwin from CMAKE_<LANG>_COMPILER in the toolchain file?

It probably depends on how much code is needed, but I'm not sure filtering sho
...
🤔 eval-exec reviewed a pull request: "random: Initialize variables in hardware RNG functions"
(https://github.com/bitcoin/bitcoin/pull/31863#pullrequestreview-2617370922)
Hi @theuni, I'd appreciate it if you could review this PR.
💬 vasild commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2658930755)
I tried the coverage instructions from `doc/developer-notes.md` and gcc/gcov generated a reasonable report. It does not look broken. No reason to remove a working, documented and used feature, with urgency, in 29.
💬 Sjors commented on pull request "cmake: add a component for each binary":
(https://github.com/bitcoin/bitcoin/pull/31844#issuecomment-2658932409)
Tested 9b033bebb18dfd609c02736292f37cc6589fcc8d a bit on arm macOS 15.3.
🚀 fanquake merged a pull request: "cmake: Improve compatibility with Python version managers"
(https://github.com/bitcoin/bitcoin/pull/31421)
💬 maflcko commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2658944005)
> Can you elaborate on "all the GCC/lcov/gcov problems" you mentioned here: [#31588 (comment)](https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2648315209)? Seems like there is also some [agreement there](https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2651994364) to switch to LLVM/Clang.

I don't use macOS, nor have I tested this specific bash script on the different Linux distros mentioned, so I can't vouch for the problems, but it looks like some people ran into problems
...