Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 Sjors opened a pull request: "doc: macOS 15 ships llvm 16"
(https://github.com/bitcoin/bitcoin/pull/30949)
#30263 bumped the minimum clang to 16

macOS versions before 15 ship older versions, see https://en.wikipedia.org/wiki/Xcode#Xcode_15.0_-_(since_visionOS_support)

Updating macOS build instructions to reflect this.
💬 maflcko commented on pull request "doc: macOS 15 ships llvm 16":
(https://github.com/bitcoin/bitcoin/pull/30949#issuecomment-2367502905)
Are you sure?

CI checks for this and passes, so you'll have to explain why CI didn't fail. Ideally, with exact steps to reproduce. Otherwise, it will be hard to follow.

Also, keep in mind that the "Clang version string" inside XCode is completely made up by Apple. It does not correspond to the llvm versioning.
💬 maflcko commented on pull request "test: Add missing sync_mempools() to fill_mempool()":
(https://github.com/bitcoin/bitcoin/pull/30948#issuecomment-2367507024)
Can be tested via the following diff, which should fail on master and pass on this pull request:

```diff
diff --git a/test/functional/p2p_1p1c_network.py b/test/functional/p2p_1p1c_network.py
index f9e782f524..5ea6add9ab 100755
--- a/test/functional/p2p_1p1c_network.py
+++ b/test/functional/p2p_1p1c_network.py
@@ -39,7 +39,7 @@ class PackageRelayTest(BitcoinTestFramework):
self.setup_clean_chain = True
self.num_nodes = 4
# hugely speeds up the test, as it in
...
💬 maflcko commented on issue "Intermittent failure in p2p_1p1c_network.py", line 58, in raise_network_minfee assert_greater_than(node.getmempoolinfo()['mempoolminfee'], FEERATE_1SAT_VB) ; AssertionError: 0.00001000 <= 0.00001000":
(https://github.com/bitcoin/bitcoin/issues/30922#issuecomment-2367521476)
Thanks @mzumsande for the detailed analysis and suggested fix. I've submitted it as https://github.com/bitcoin/bitcoin/pull/30948 to address the intermittent failure sooner rather than later. I hope it is fine.
💬 fanquake commented on pull request "doc: macOS 15 ships llvm 16":
(https://github.com/bitcoin/bitcoin/pull/30949#issuecomment-2367536842)
Yea, this isn't correct. The Apple Clang that is installable with macOS 14.x work perfectly fine. No need to install a different LLVM toolchain.
💬 maflcko commented on issue "Warn about / refuse unsupported clang version":
(https://github.com/bitcoin/bitcoin/issues/30947#issuecomment-2367541261)
There is no need to use brew to install clang/llvm. You can just pick the newer XCode: `sudo xcode-select --switch /Applications/Xcode_15.0.app`

You may have to upgrade to macOS 13.5 (or later), according to the release notes.
💬 fanquake commented on issue "Warn about / refuse unsupported clang version":
(https://github.com/bitcoin/bitcoin/issues/30947#issuecomment-2367541499)
> It would be good if cmake -B build checks the minimum version

We don't check compiler versions, we check for C++ compatibility (i.e `CMAKE_CXX_STANDARD`, `CMAKE_CXX_STANDARD_REQUIRED`, `CMAKE_CXX_EXTENSIONS`). If CMakes `CMAKE_CXX_STANDARD` check is incorrectly passing, when a compiler doesn't actually support C++20, we should file a bug upstream, so they can expand the checks they perform.
💬 hebasto commented on pull request "doc: macOS 15 ships llvm 16":
(https://github.com/bitcoin/bitcoin/pull/30949#issuecomment-2367594534)
https://en.wikipedia.org/wiki/Xcode#Toolchain_versions shows that Apple clang version 15.0.0 (clang-1500.0.40.1) used in the CI is based on LLVM 16.0.0.
💬 hebasto commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2367608976)
> Not sure if that platform is supposed to work at all, cc @ryanofsky, but if not, the Guix and/or depends build process should imo complain about that earlier.

MIght be related: https://github.com/chaincodelabs/libmultiprocess/issues/53.
maflcko closed an issue: "Improve documentation of estimatesmartfee"
(https://github.com/bitcoin/bitcoin/issues/18217)
💬 maflcko commented on issue "Improve documentation of estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/18217#issuecomment-2367613179)
Closing for now. However, if there are still questions about the documentation, or if there are ideas to improve it, please leave a comment below, or create a pull request, or a new issue.
💬 hebasto commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2367622858)
For the `x86_64-linux-gnu` host, builds are non-deterministic:
- mine, aarch64:
```
dde3c647c0121f5f0623c8edffe4bc3fccf79bf1195752b3c51362fc35904e4f guix-build-878cb237b717/output/x86_64-linux-gnu/bitcoin-878cb237b717-x86_64-linux-gnu.tar.gz
```
- @Sjors's, x86_64:
```
f7d4f900d53db09d4958363382e75fe169a97ef4996743b5a96f6c8b9d18b800 guix-build-878cb237b717/output/x86_64-linux-gnu/bitcoin-878cb237b717-x86_64-linux-gnu.tar.gz
```
💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2367639325)
> Seems like you missed some review comments? https://github.com/bitcoin/bitcoin/pull/30861/files#r1752126150 [#30861 (comment)](https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752127638)
>
> Seems like [#30861 (comment)](https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752135299) is now also more relevant given #30905, so it'd be good to know how this is going to be handed going forward.

Converting to a draft for now.
📝 hebasto converted_to_draft a pull request: "build: Improve `ccache` performance for different build directories"
(https://github.com/bitcoin/bitcoin/pull/30861)
This PR makes it possible to reuse the `ccache` cache populated during a build in another build directory.

For example, on Ubuntu 24.04:
```
$ cmake -B build_1 -DWITH_CCACHE=ON
$ cmake --build build_1 -t bitcoind
$ cmake -B build_2 -DWITH_CCACHE=ON
$ ccache --zero-stats
$ cmake --build build_2 -t bitcoind
$ ccache --show-stats
Cacheable calls: 298 / 298 (100.0%)
Hits: 221 / 298 (74.16%)
Direct: 59 / 221 (26.70%)
Preprocessed: 162 / 221 (73.30%)

...
💬 hebasto commented on pull request "build: scripted-diff: drop config/ subdir for bitcoin-config.h":
(https://github.com/bitcoin/bitcoin/pull/30937#issuecomment-2367649337)
Concept ACK, given the early stage of the current release cycle and the presence of only a single conflicting PR.
💬 fanquake commented on pull request "depends: Fix build with `MULTIPROCESS=1` in Guix environment":
(https://github.com/bitcoin/bitcoin/pull/30940#issuecomment-2367651708)
Not sure I understand this fix yet, and it seems like there are some other depends changes here, for what I would have expected to be a fixup in libmultiprocess (either in how it's installed, or how we are searching for it in CMake)?

In 38bc0463aaf1f3e20e2f3447af40c29c6d2d6859:
> which in turn facilitates the use of depends
in scenarios where building depends is a separate step, such as on
NixOS.

Building depends is always it's own/a separate step. Not really clear what this means? As f
...
💬 hebasto commented on issue "`test_bitcoin` from pre-built 28.0rc2 tarball is failing for JSON parsing":
(https://github.com/bitcoin/bitcoin/issues/30938#issuecomment-2367667574)
> I think the bug is that commit introduced a (silent) runtime dependency that isn't actually checked/enforced in anyway.

I [agree](https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2331243915).

I'd suggest reverting 199bb09d88e28d951c5068eb65643390dbedd066, as workarounds for https://github.com/bitcoin/bitcoin/issues/30601 and https://github.com/bitcoin/bitcoin/issues/30608 are available.

Additionally, I suspect there is a bug in `src/util/subprocess.h` that needs to be invest
...
💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2367668253)
re-utACK 6ea0eb954bc810da71cd057c116f5c2d359a1f37

Tested against my OPNsense router running miniupnpd 2.3.6 with pf backend, on Intel macOS 13.7 and 15.0, Ubuntu 24.04, FreeBSD 14 and Windows 11.
💬 fanquake commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#discussion_r1771053156)
> OK. What can you suggest?

My suggestion is to remove the changes that have been added to facilitate bypassing tests (i.e separating the `LDFLAGS` into `LDFLAGS` and `HARDENED_LDFLAGS`, so that some flags are skipped when tests are performed). If the build environment has changed such that the current set of tests don't pass with the new changes, then the tests should be reworked, not skipped.
💬 Sjors commented on pull request "doc: macOS 15 ships llvm 16":
(https://github.com/bitcoin/bitcoin/pull/30949#issuecomment-2367688021)
The Github CI native macOS 14 build uses Xcode 15:

https://github.com/bitcoin/bitcoin/blob/33adc7521cc8bb24b941d959022b084002ba7c60/.github/workflows/ci.yml#L81-L100

macOS 14 supports Xcode 15: https://developer.apple.com/support/xcode/

So it seems only macOS Ventura 13 needs the manual llvm install, but let me double-check that.