-
Notifications
You must be signed in to change notification settings - Fork 0
Merge bitcoin/bitcoin#24051: Bugfix: configure: bitcoin-{cli,tx,util} don't need UPnP, NAT-PMP, or ZMQ #125
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
base: backport-0.25-batch-241
Are you sure you want to change the base?
Conversation
…eed UPnP, NAT-PMP, or ZMQ 9886863 Bugfix: configure: bitcoin-{cli,tx,util} don't need UPnP, NAT-PMP, or ZMQ (Luke Dashjr) Pull request description: As with bitcoin#23345, these other tools likewise don't use various deps. ACKs for top commit: achow101: ACK 9886863 Tree-SHA512: 4be056b8e0c9f69834229aa257187457de1bc34214d320b770834e21ecc1f0ca7aa7b9689fba525928947bfabbb461528795f709014fb9618b82f088fe64f271
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
The original Bitcoin change removed these tools from the conditional because they don't need UPnP, NAT-PMP, or ZMQ dependencies. The conditional should only disable these dependencies when none of the tools that might use them are being built. Original Bitcoin commit: 4766cd1
✅ Backport Verification - Issue FixedI found and automatically applied a fix to improve the faithful backporting. Original Bitcoin commit: Issue FoundThe original PR implementation was missing Fix AppliedUpdated the conditional from: if test "$build_bitcoind$bitcoin_enable_qt$use_bench$use_tests" = "nononono"; then To: if test "$build_bitcoin_cli$build_bitcoin_tx$build_bitcoind$bitcoin_enable_qt$use_bench$use_tests" = "nononononono"; then Validation Results
The backport is now complete and faithful to the original Bitcoin change. |
Bitcoin's intent was to exclude bitcoin-cli, bitcoin-tx, and bitcoin-util from the condition that disables UPnP/NAT-PMP/ZMQ dependencies, since these tools don't need them. The original implementation incorrectly added these tools to the condition instead of removing them. This fix correctly implements Bitcoin's simplified condition that only checks for dashd, Qt, benchmarks, and tests. Original Bitcoin commit: 4766cd1
✅ Backport Verification - Issues FixedI found 1 critical issue with the backport implementation and automatically applied a fix. Original Bitcoin commit: Issue Found and Fixed:Problem: The original implementation had Bitcoin's intent backwards. Bitcoin's change was to exclude What was wrong:
Fix applied:
To the correct Bitcoin implementation:
Verification Results:
The backport now correctly implements Bitcoin's optimization that disables UPnP, NAT-PMP, and ZMQ dependencies when only building dash-cli, dash-tx, or dash-wallet, since these utilities don't need networking functionality. This PR is now ready for merge. ✅ |
Backports bitcoin#24051
Original commit: 4766cd1
Backported from Bitcoin Core v0.25
This backport adds a build optimization that disables UPnP, NAT-PMP, and ZMQ dependencies when they're not needed. The optimization checks if only bitcoind, Qt, bench, or tests are being built, and if none of them are enabled, it disables these optional dependencies.
The change is minimal and focused, adding just the conditional check in configure.ac to improve build efficiency by avoiding unnecessary dependency compilation.