Skip to content

[29.x] More backports #32810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 3, 2025
Merged

Conversation

Nix patches cmake to remove the root directory `/` from
`CMAKE_SYSTEM_PREFIX_PATH`:
https://github.com/NixOS/nixpkgs/blob/428b49b28ebc8938a6d9f6c540d32d7a06713972/pkgs/by-name/cm/cmake/001-search-path.diff#L10

Without this, and when using the toolchain for depends builds, cmake's
`find_path()` and `find_package()` do not know where to find
dependencies, causing issues like:
bitcoin#32428

Adding this path back via CMAKE_PREFIX_PATH is harmless on other
systems, and fixes the toolchain for Nix users.

We append the `/` dir a maximum of once, as the toolchain may be called
repeatedly during builds.

Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: josibake <josibake@protonmail.com>

Github-Pull: bitcoin#32798
Rebased-From: e27a945
@fanquake fanquake added this to the 29.1 milestone Jun 25, 2025
@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 25, 2025

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32810.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK pinheadmz, hebasto, willcl-ark
Stale ACK josibake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

  • "peers is" -> "peer is" [subject–verb agreement makes the sentence clear]

No other typos were found.

drahtbot_id_4_m

The catchup loop in the outbound eviction functional test currently has
a small flaw, as the contained waiting for a `getheaders` message just
waits for any such message instead of one with the intended block hash.
The reason is that the `prev_prev_hash` variable is set incorrectly,
since the `tip_header` instance is not updated and its field `.hash` is
None. Fix that by updating `tip_header` and use the correct field -- we
want the tip header's previous hash (`.hashPrevBlock`).

Github-Pull: bitcoin#32742
Rebased-From: dd8447f
@josibake
Copy link
Member

ACK e96d360

Verified that the correct commits are being pulled in and the release notes.

According to the CMake documentation, `HINTS` "should be paths computed
by system introspection, such as a hint provided by the location of
another item already found", which is precisely the case in the
`FindQRencode` module.

Entries in `HINTS` are searched before those in `PATHS`. On macOS,
Homebrew’s `libqrencode` will therefore be located at its real path
rather than via the symlink in the default prefix.

Github-Pull: bitcoin#32805
Rebased-From: ead4468
On macOS, this change ensures that the Boost package is located at its
real path rather than via the symlink in the default prefix.

Github-Pull: bitcoin#32814
Rebased-From: 8800b5a
@Sjors
Copy link
Member

Sjors commented Jun 26, 2025

Tested on M4 macOS 15.5 that with 5987c1b having qt5 and qt6 (qt@5 and qt via Homebrew) leads to the issues described in #31009, but with fe8034b it's happy.

Will test on Intel later.


The fix also works on Intel with macOS 13.7.6

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fe8034b. I've backported all mentioned PRs locally and got zero diff with this branch.

@DrahtBot DrahtBot requested a review from josibake June 26, 2025 15:41
darosior added 2 commits June 26, 2025 17:36
32-bit architecture is limited to 4GiB, so it doesn't make sense to set a too high value. 500 MB is
chosen as an arbitrary maximum value that seems reasonable.

Github-Pull: bitcoin#32530
Rebased-From: 2c43b6a
32-bit architecture is limited to 4GiB, so it doesn't make sense to set a too
high value. Since this setting is performance critical, pick an arbitrary value
higher than for -maxmempool but still reasonable.

Github-Pull: bitcoin#32530
Rebased-From: 9f8e7b0
@fanquake fanquake force-pushed the even_more_29_backports branch from c368c1a to ef2a013 Compare June 30, 2025 12:29
Copy link
Member

@pinheadmz pinheadmz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ef2a013

Build on macos/arm64 after experiencing issues locally building a checked-out v29.0 GUI with only qt 6 installed. Can confirm these backports solve that problem and GUI runs fine. Did not test the other backports.

Show Signature
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK ef2a013e31cf6fbded5735a998b4c992c176493d
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmhli/0ACgkQ5+KYS2KJ
yTruOxAAm8c/A9GC5xv2ZIHK+F8sCXyDLzXKm0OHHRGXVoalows6vtRbCx6hdS17
UhZFQA+McBLeLz/O3D5wtUROXafXSQ3qveZKjiwJWsi2gve/iK3DF2V9kX8B7YW4
63Oyje5vV+d8xa9UfyzMymWxYU6E9OORp5fjJM6hjudOhKABgaOIet0vF/mpqZL8
IATlFjQ27pFlyzZ0yZWrB8i6IZjrFuBaIM2fWX4Msr8te56h64s0Zp6TRJbOX9r6
ViXnw3N75Zp6zOwW9qa5i24M4Zo+mLYWup1jUhjE0IxibwnFT6/QVbdvl8Q1Vx/R
t8xkxmZr4W+gCBoTkrDJCDb6N5xvWK9N6j70g6VykY7p2pOTbCxmnOYtDovGH+xl
F/StVeqRwwzuH7ys5xw5LBWaWp9M8v0j2gDO4sXK+KZ6UJMEYJmTV7h3iUhGvL2U
QE6vCE8Gbq3DJN+mn5tHPqKYn5Ecr86Wt3CElquGvzS4jtHnhqq1KB8h7UmZQjvg
xNJhibtWa7nao+74/wWo7xiU6/MRgC9/URO/TaSu/2Z10i2U0jJE4w7WpbfOkWQT
r0B55rOV/P2nBQj0Su1s5ltLiuy5PZAfbswoI64Nz9ZgPQIgjff9lent61jXFgNo
TTIC7OOz8+8sL73nXRBqV9n6rCVS1h1Hr0YQcgAumqbU1Td5KfY=
=aEyl
-----END PGP SIGNATURE-----

pinheadmz's public key is on openpgp.org

@DrahtBot DrahtBot requested a review from hebasto July 2, 2025 19:47
@fanquake fanquake marked this pull request as ready for review July 3, 2025 09:22
@fanquake
Copy link
Member Author

fanquake commented Jul 3, 2025

@josibake / @hebasto want to re-review?

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-ACK ef2a013.

Missed credits to @darosior and @ryanofsky?

Copy link
Member

@willcl-ark willcl-ark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ef2a013

...modulo hebasto's comments.

Backports all look correct.

@fanquake
Copy link
Member Author

fanquake commented Jul 3, 2025

There's going to be another branch, so any credits can be fixed up there.

@fanquake fanquake merged commit d360a6e into bitcoin:29.x Jul 3, 2025
18 checks passed
@fanquake fanquake deleted the even_more_29_backports branch July 3, 2025 10:58
@hebasto hebasto mentioned this pull request Jul 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants