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

💬 hodlinator commented on pull request "test: Remove stale gettime test":
(https://github.com/bitcoin/bitcoin/pull/31846#issuecomment-2658947230)
Yeah, was experimenting with something like this as an extra sanity check.
```diff
diff --git a/src/util/time.cpp b/src/util/time.cpp
index cafc27e0d0..419ddb3702 100644
--- a/src/util/time.cpp
+++ b/src/util/time.cpp
@@ -34,6 +34,7 @@ NodeClock::time_point NodeClock::now() noexcept
mocktime :
std::chrono::system_clock::now().time_since_epoch()};
assert(ret > 0s);
+ assert(ret < std::chrono::years{200});
return time_point{ret};
};
```
But on
...

💬 fanquake commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2658948674)
Looking at Corecheck, does it use our `Coverage.cmake`? I can't find it being used anywhere there, seems like only calls to gcovr directly.

💬 vasild commented on pull request "[POC] cmake: Introduce LLVM's Source-based Code Coverage reports":
(https://github.com/bitcoin/bitcoin/pull/31394#issuecomment-2658958631)
> > Would we keep supporting generating coverage with gcc?

> I don't think we should keep both approaches.

Why not? GCC is the native compiler on Linux, a widely used OS for developing Bitcoin Core.

💬 janb84 commented on pull request "test: check `scanning` field from `getwalletinfo`":
(https://github.com/bitcoin/bitcoin/pull/31768#issuecomment-2658997619)
Post-merge ACK [bb0879d](https://github.com/bitcoin/bitcoin/commit/bb0879ddabc8b3a7253bc774d23b842937d18015)

Small improvement suggestion; also testing that the progress is between 0 and 1.
``` python
progress = wallet_info["scanning"]["progress"]
assert_approx(progress,0,1)
```

💬 maflcko commented on pull request "random: Initialize variables in hardware RNG functions":
(https://github.com/bitcoin/bitcoin/pull/31863#discussion_r1955969666)
Setting the sanity check value to a default of `0` is fine, because if it was never set, it will run in an infinite loop, just as-if the rng failed.

However, setting the random value that is returned to a default value is problematic and controversial, because:

* It silences sanitizers such as msan and valgrind that detect uninitialized reads
* If we wanted to zero-init everything, we could just do https://github.com/bitcoin/bitcoin/issues/18892 (or a project-wide source change)
* The in
...

👍 laanwj approved a pull request: "net: reduce CAddress usage to CService or CNetAddr"
(https://github.com/bitcoin/bitcoin/pull/31854#pullrequestreview-2617470398)
Code review ACK cd4bfaee103591f212b88afd17969c16c1056eb6

💬 pinheadmz commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1955976383)
Since `key_type` here is `std::shared_ptr<const Sock>` shouldn't this map also use the optimized hash/equal functions defined in:

https://github.com/bitcoin/bitcoin/blob/86528937e5c4da2e12c46085fc41e87ed759258e/src/util/sock.h#L208

💬 Sjors commented on pull request "cmake: Improve compatibility with Python version managers":
(https://github.com/bitcoin/bitcoin/pull/31421#issuecomment-2659034075)
Tested dead9086543671b07e6ce041821e4d2a6627075b on arm macOS 15.3 that before this PR cmake would find the Python version installed by Homebrew (3.13.1) rather than the one I set with PyEnv (3.10.14).

The system version at `/usr/bin/python3` is 3.9.6.

With this change it still does that and ignores PyEnv version:

```
-- Found Python3: /opt/homebrew/bin/python3 (found suitable version "3.13.1", minimum required is "3.10") found components: Interpreter
```

```
which -a python3
/op
...

💬 Sjors commented on pull request "cmake: Improve compatibility with Python version managers":
(https://github.com/bitcoin/bitcoin/pull/31421#issuecomment-2659048632)
Same when testing on master @ 7bbd761e816b713ac88c87f8b0045ea2f958ecbf after this was merged. I can't uninstall python3 from Homebrew because it's a dependency for a bunch of things.

Full `cmake -B build` log: https://gist.github.com/Sjors/becdc73b4dadcfa26121f37a11665e59

💬 eval-exec commented on pull request "random: Initialize variables in hardware RNG functions":
(https://github.com/bitcoin/bitcoin/pull/31863#discussion_r1955991364)
Agreed, seting `r1 = 0` is problematic.

💬 hebasto commented on pull request "depends: set `CMAKE_*_COMPILER_TARGET` in toolchain":
(https://github.com/bitcoin/bitcoin/pull/31849#issuecomment-2659057610)
> > Would it be worth filtering out --target=$ARCH-apple-darwin from CMAKE__COMPILER in the toolchain file?
>
> It probably depends on how much code is needed, but I'm not sure filtering should be done unless its generic.

For example:
```diff
--- a/depends/toolchain.cmake.in
+++ b/depends/toolchain.cmake.in
@@ -29,8 +29,15 @@ if(NOT DEFINED CMAKE_C_FLAGS_DEBUG_INIT)
set(CMAKE_C_FLAGS_DEBUG_INIT "@CFLAGS_DEBUG@")
endif()

+macro(fliter_flags compiler_invocation_line)
+ if(CMA
...

💬 Sjors commented on pull request "cmake: Improve compatibility with Python version managers":
(https://github.com/bitcoin/bitcoin/pull/31421#issuecomment-2659113645)
`brew unlink python3` "fixes" it for me. We could mention it as a build hint if someone runs into a problem. Not sure how likely that is to be problem though.