💬 fanquake commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2136977600)
Binary size comparison for `arm64-apple-darwin` of a Guix build of master (be100cf4c77a) vs this PR (6213ac97c6b5):
| Binary | master | PR |
| ------ | ------- | --- |
| bitcoin-cli | 481248 | 477056 |
| bitcoin-qt | 30559072 | 30511316 |
| bitcoin-tx | 2246456 | 2245808 |
| bitcoin-util | 324200 | 324168 |
| bitcoin-wallet | 5091904 | 5054440 |
| bitcoind | 11867476 | 11820592 |
| test_bitcoin | 20097440 | 20054668 |

💬 maflcko commented on pull request "releases: use LLVM 18 for macOS":
(https://github.com/bitcoin/bitcoin/pull/30022#discussion_r1618584825)
It should be possible now to nuke this and require the user (if they want to cross-compile to macOS) to type `apt install clang-18` instead?

I presume leaving this to the user will allow to run the cross-compile on more distros, as opposed to being possibly confined to `x86_64-linux-gnu-ubuntu` when using the llvm artefacts?

💬 fanquake commented on pull request "releases: use LLVM 18 for macOS":
(https://github.com/bitcoin/bitcoin/pull/30022#discussion_r1618590069)
Yea, the plan is to switch away from vendoring Clang entirely, which is more straightforward now that #21778 is merged. I'll be followingup.

💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1618645388)
This is the upstream fix: https://github.com/freebsd/freebsd-src/pull/1070.

The suggested workaround to define `typeof` is ok:

```diff
diff --git i/src/util/netif.cpp w/src/util/netif.cpp
index 845b8aed1d..1840974a83 100644
--- i/src/util/netif.cpp
+++ w/src/util/netif.cpp
@@ -9,18 +9,24 @@
#include <logging.h>
#include <netbase.h>
#include <util/check.h>
#include <util/sock.h>
#include <util/syserror.h>

+#ifdef __FreeBSD__
+#include <osreldate.h>
+#endif
+
// Linux
...

🤔 willcl-ark reviewed a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147#pullrequestreview-2085013260)
I think the version parser here is is pretty good shape generally, or at least improved from its current state.

I am not convinced yet that it is handling `rc` correctly yet though, see results of this additional test i added:

```log
₿ ./contrib/verify-binaries/test2.py
Checking 27.0
Found RC version: version_base=27.0, version_rc= os_filter=
Checking 27.0-x86_64
Found RC version: version_base=27.0, version_rc= os_filter=x86_64
Checking 27.0-linux
Found RC version: version_base=27.0
...

💬 willcl-ark commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1618650022)
Not sure if we want this to specify bitcoincore.org, `bitcoin.org` does host some binaries too (and recently even added a few which aren't totally out of date!), and we do try both hosts. On the other hand, this advice is actually more useful, accurate and likely to be correct, so 🤷🏼‍♂️

💬 laanwj commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1618653598)
Thanks!

> Silence a warning about signed vs unsigned integer comparison.

Oh no. Pretty sure i had to do the `response_len` thing to work around another error about signed-unsigned integer comparison in the i686 CI run.

💬 willcl-ark commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2137090343)
Here is the file I was using to check the new parsing

<details>
<summary>Details</summary>

```python
#!/usr/bin/env python3

import tempfile
from verify import parse_version_string, parse_sums_file

SHASUMS = """cb35e250ae9d0328aa90e7aad0b877ed692597420a1092e8ab1a5dd756209722 bitcoin-27.0-aarch64-linux-gnu.tar.gz
9d4c28e7620d03bf346ebea388f222e4d6d2b996d7eb32fab72707b8320d5249 bitcoin-27.0-arm-linux-gnueabihf.tar.gz
7f060f2cd07746ff9d09b000b4195fee88dfca8444ab7a73f0c76aff4225227
...

📝 fanquake opened a pull request: "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE"
(https://github.com/bitcoin/bitcoin/pull/30189)
Supposedly this is no-longer needed (and hasn't been ported to CMake), so remove it's usage here. Oringinally used to suppress warnings about deprecated/removed from the standard functioanlity, which was still supported by some compilers.

🤔 MaxJitphanu reviewed a pull request: "net: Replace libnatpmp with built-in PCP+NATPMP implementation"
(https://github.com/bitcoin/bitcoin/pull/30043#pullrequestreview-2085053341)
1

💬 maflcko commented on pull request "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137148183)
Did you test this with the minimum required boost version 1.71?

💬 fanquake commented on pull request "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137156339)
> Did you test this with the minimum required boost version 1.71?

Only with 1.81.0. (depends) which still has the problematic behaviour. Used a CMake CI (which has already dropped this) with Boost 1.73.0 as a proxy for anything lower (https://github.com/hebasto/bitcoin/actions/runs/9257726144/job/25466464660).

💬 brunoerg commented on pull request "rpc: net: follow-ups for #30062":
(https://github.com/bitcoin/bitcoin/pull/30183#discussion_r1618702221)
Much better, I will update it. I think it's worth updating `getpeerinfo` too.

💬 maflcko commented on pull request "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137176328)
The cmake run is using g++-11, but g++-12 is required according to https://github.com/boostorg/config/pull/430/files

💬 fanquake commented on pull request "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137183165)
@hebasto how did you test this macro is no-longer required in the CMake build (given it wasn't ported)?

💬 hebasto commented on pull request "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137188611)
> @hebasto how did you test this macro is no-longer required in the CMake build (given it wasn't ported)?

https://github.com/hebasto/bitcoin/actions/runs/9283914497

💬 hebasto commented on pull request "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137192595)
> Did you test this with the minimum required boost version 1.71?

It is 1.73 since https://github.com/bitcoin/bitcoin/pull/29066.

🤔 BrandonOdiwuor reviewed a pull request: "test: improve BDB parser (handle internal/overflow pages, support all page sizes)"
(https://github.com/bitcoin/bitcoin/pull/30125#pullrequestreview-2085148866)
Concept ACK

💬 hebasto commented on pull request "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137202507)
Concept ACK.

💬 maflcko commented on pull request "build: remove usage of DBOOST_NO_CXX98_FUNCTION_BASE":
(https://github.com/bitcoin/bitcoin/pull/30189#issuecomment-2137223724)
Looks like 880d4aaf81f3d5d7fbb915905c2e61b816a6a747 affected depends, not sure if it even reproduces outside of that?