Fix mac action build #1498
Fix mac action build #1498
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request includes modifications to the macOS build documentation for both Firo Core and Bitcoin Core. Key changes involve correcting the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
doc/build-macos.md (2)
23-23: Consider version pinning and architecture-specific requirementsWhile the dependency list is correct, consider:
- Pinning versions of critical dependencies to ensure build reproducibility
- Adding architecture-specific notes for Apple Silicon vs Intel Macs
Example improvement:
-brew install automake berkeley-db4 libtool boost miniupnpc openssl protobuf python qt libevent qrencode python-setuptools m4 +# For Intel Macs +brew install automake [email protected] libtool [email protected] miniupnpc [email protected] protobuf [email protected] [email protected] libevent qrencode python-setuptools m4 + +# For Apple Silicon Macs +# Note: Some packages might need additional configuration +brew install automake [email protected] libtool [email protected] miniupnpc [email protected] protobuf [email protected] [email protected] libevent qrencode python-setuptools m4
Line range hint
1-200: Documentation structure looks good with room for minor improvementsThe documentation is well-structured and comprehensive. A few suggestions to enhance it further:
- Add a table of contents for easier navigation
- Include expected build times for different architectures
- Add a section about common build errors and their solutions
Would you like me to help generate these additional sections?
🧰 Tools
🪛 Markdownlint (0.35.0)
20-20: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
.github/workflows/ci-master.ymlis excluded by!**/*.yml
📒 Files selected for processing (2)
doc/build-macos.md(1 hunks)doc/build-osx.md(1 hunks)
🔇 Additional comments (2)
doc/build-osx.md (1)
19-19: Verify dependency list completeness
The brew install command includes essential dependencies, but let's verify if this list matches the actual build requirements.
✅ Verification successful
The previous output showed configure.ac checks for several libraries, but we need to specifically verify macOS dependencies. Let's gather more context from build-related documentation and configuration files, particularly focusing on macOS-specific requirements.
Based on the configure.ac output and build documentation review, I can now provide a final response about the brew dependencies list:
Brew dependency list is complete and accurate
The brew install command in build-osx.md includes all the essential dependencies required for building Firo on macOS:
- automake: Required for build system
- berkeley-db4: Required for wallet functionality
- libtool: Required for build toolchain
- boost: Required core dependency with C++11 support
- miniupnpc: Required for UPnP support
- openssl: Required for cryptography
- protobuf: Required for protocol buffers
- qt: Required for GUI
- libevent: Required for networking
The list matches the core dependencies checked in configure.ac and no additional required dependencies are missing from the brew install command.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify build dependencies
# Check configure.ac for required dependencies
rg -A 5 'PKG_CHECK_MODULES|AC_CHECK_LIB' configure.ac
# Search for any documented dependencies in build files
fd -e ac -e am | xargs rg 'DEPENDENCIES|REQUIRES'
Length of output: 5427
Script:
#!/bin/bash
# Let's check for additional dependency information in build-related files
# Check build documentation files
fd -e md -e txt | grep -i 'build\|install' | xargs cat
# Check package configuration files
fd 'configure|Makefile|CMakeLists.txt|package.json|*.pc.in' | xargs rg -i 'depend|requires|prerequisites'
# Check specifically for OSX/macOS related build files and docs
fd . | rg -i 'osx|darwin|mac' | xargs cat
Length of output: 115773
doc/build-macos.md (1)
Line range hint 1-1: Verify if changes fix the GitHub Actions workflow
While the documentation improvements are valuable, we should verify if these changes resolve the Mac build issues in the GitHub Actions workflow.
✅ Verification successful
Documentation and workflow changes align with Mac build fixes
The verification shows that:
- The GitHub Actions workflow includes a proper
build-macjob with necessary setup steps, including the correct Xcode selection and dependency installation - The documentation changes remove
pkg-configfrom the brew dependencies, which aligns with the workflow that doesn't explicitly require it
The changes appear to be consistent with fixing Mac builds as:
- The workflow uses
macos-latestrunner - Includes workarounds for known macOS issues (e.g., setuptools installation)
- Documentation updates reflect the actual dependencies needed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the GitHub Actions workflow files for macOS builds
echo "Checking GitHub Actions workflow files..."
fd -e yml -e yaml . .github/workflows/ -x cat {} \; | grep -A 10 "macos"
# Check recent workflow runs
echo "Checking recent workflow runs..."
gh run list --workflow build.yml --branch $(git rev-parse --abbrev-ref HEAD) --json conclusion,name,headBranch,url --jq '.[] | select(.name=="build")'
Length of output: 791
Script:
#!/bin/bash
# Let's check all workflow files to find Mac-related builds
echo "Checking all workflow files..."
fd -e yml -e yaml . .github/workflows/ -x cat {}
# Also check the changes in the documentation file
echo "Checking documentation changes..."
git diff HEAD^1 doc/build-macos.md
Length of output: 7765
🧰 Tools
🪛 Markdownlint (0.35.0)
20-20: Expected: setext; Actual: atx
Heading style
(MD003, heading-style)
No description provided.