build: fix some errors when building on macOS w/o depends #6950
Conversation
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThis pull request changes build configuration and a makefile entry. In Sequence Diagram(s)sequenceDiagram
participant Dev as Developer runs ./configure
participant Conf as configure.ac
participant Sys as System (headers/libs)
participant Make as Makefile
Dev->>Conf: invoke configure
Conf->>Conf: detect Darwin/Homebrew GMP prefix?
alt GMP prefix detected
Conf->>Conf: set GMP_CPPFLAGS (-isystem or -I) and GMP_LDFLAGS
Conf->>Conf: save CPPFLAGS/LDFLAGS (TEMP_*)
Conf->>Sys: run AC_CHECK_HEADER / AC_CHECK_LIB with GMP flags
Sys-->>Conf: header/lib detection results
Conf->>Conf: restore CPPFLAGS/LDFLAGS
Conf->>Conf: append GMP flags to CORE_CPPFLAGS/CORE_LDFLAGS
else no GMP prefix
Conf->>Sys: run standard GMP checks without special flags
Sys-->>Conf: header/lib detection results
end
Conf->>Make: write configured flags
Make->>Make: libbitcoin_common_a_CPPFLAGS now includes BOOST_CPPFLAGS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
configure.ac (1)
1692-1699: Temporary flag isolation pattern is correct.The save/restore pattern for
CPPFLAGSandLDFLAGScorrectly isolates GMP-specific flags to just the header and library checks. This prevents the GMP flags from leaking into subsequent configure checks while ensuring they're available when needed.The distinction between
CORE_CPPFLAGS(line 917, used at build time) andCPPFLAGS(line 1694, used during configure checks) is appropriate.Consider enhancing the error messages to provide guidance on how to resolve missing GMP:
-AC_CHECK_HEADER([gmp.h],, AC_MSG_ERROR(libgmp headers missing)) -AC_CHECK_LIB([gmp], [__gmpz_init],GMP_LIBS=-lgmp, AC_MSG_ERROR(libgmp missing)) +AC_CHECK_HEADER([gmp.h],, AC_MSG_ERROR([libgmp headers missing. Install libgmp development package (e.g., 'brew install gmp' on macOS or 'apt-get install libgmp-dev' on Debian/Ubuntu).])) +AC_CHECK_LIB([gmp], [__gmpz_init],GMP_LIBS=-lgmp, AC_MSG_ERROR([libgmp library missing. Install libgmp development package (e.g., 'brew install gmp' on macOS or 'apt-get install libgmp-dev' on Debian/Ubuntu).]))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
configure.ac(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: kwvg
Repo: dashpay/dash PR: 6543
File: src/wallet/receive.cpp:240-251
Timestamp: 2025-02-06T14:34:30.466Z
Learning: Pull request #6543 is focused on move-only changes and refactoring, specifically backporting from Bitcoin. Behavior changes should be proposed in separate PRs.
Learnt from: knst
Repo: dashpay/dash PR: 6916
File: src/univalue/include/univalue.h:81-88
Timestamp: 2025-10-25T07:08:51.918Z
Learning: For backport PRs from bitcoin/bitcoin, bitcoin-core/gui, etc., backported changes should match the original upstream PRs even if they appear strange, modify vendored code, or seem to violate coding guidelines. Still flag genuine issues like bugs, undefined behavior, crashes, compilation errors, or linter failures.
📚 Learning: 2025-01-06T09:51:03.167Z
Learnt from: kwvg
Repo: dashpay/dash PR: 6516
File: depends/patches/gmp/include_ldflags_in_configure.patch:557-621
Timestamp: 2025-01-06T09:51:03.167Z
Learning: The `GMP_GCC_ARM_UMODSI` macro checks only the compiler version, and `GMP_GCC_MIPS_O32` relies on the `-mabi=32` flag. Therefore, `$LDFLAGS` is irrelevant to these tests.
Applied to files:
configure.ac
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: linux64-build / Build source
- GitHub Check: linux64_sqlite-build / Build source
- GitHub Check: linux64_fuzz-build / Build source
- GitHub Check: linux64_ubsan-build / Build source
- GitHub Check: arm-linux-build / Build source
- GitHub Check: win64-build / Build source
- GitHub Check: mac-build / Build source
- GitHub Check: linux64_tsan-build / Build source
- GitHub Check: linux64_nowallet-build / Build source
- GitHub Check: Lint / Run linters
🔇 Additional comments (2)
configure.ac (2)
397-413: LGTM! Stacktrace detection correctly implements past review feedback.The nested
AC_CHECK_HEADERSapproach with explicit error and warning paths for bothexecinfo.handbacktrace.haligns with UdjinM6's previous suggestion. The logic properly handles the tri-state configuration (yes/no/auto) and provides clear error messages when required headers are missing.
909-919: GMP detection via Homebrew looks correct.The logic properly detects the GMP installation path from Homebrew and conditionally applies
-isystemor-Ibased on thesuppress_external_warningssetting. This is the correct approach for handling Homebrew's keg-only installations on macOS.
3 participants
Issue being fixed or feature implemented
Properly detect gmp, backtrace, etc
What was done?
See commit
How Has This Been Tested?
Builds on macOS
Breaking Changes
Please describe any breaking changes your code introduces
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.