-
-
Notifications
You must be signed in to change notification settings - Fork 353
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
Update macOS Build Instructions for Dependency and Configuration IssuesMac os build fix #1463
Conversation
WalkthroughThe updates enhance the clarity and completeness of the macOS build instructions for Firo Core. Key changes include a reformatted preparation section, expanded dependency installation commands, and a newly added troubleshooting guide for Changes
Sequence Diagram(s)No Sequence Diagrams needed as the changes are primarily documentation updates. Poem
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- doc/build-macos.md (4 hunks)
Additional context used
Markdownlint
doc/build-macos.md
39-39: Expected: h3; Actual: h4 (MD001, heading-increment)
Heading levels should only increment by one level at a time
6-6: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
19-19: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
39-39: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
46-46: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
29-29: Expected: 0 or 2; Actual: 6 (MD009, no-trailing-spaces)
Trailing spaces
67-67: Expected: 0 or 2; Actual: 8 (MD009, no-trailing-spaces)
Trailing spaces
13-13: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
25-25: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
72-72: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1-1: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
6-6: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
19-19: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
30-30: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
39-39: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
46-46: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
97-97: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
90-90: Punctuation: ':' (MD026, no-trailing-punctuation)
Trailing punctuation in heading
8-8: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
10-10: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
15-15: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
21-21: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
41-41: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
48-48: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
55-55: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
7-7: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
47-47: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
104-104: null (MD038, no-space-in-code)
Spaces inside code span elements
28-28: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
70-70: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
80-80: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
88-88: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
93-93: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
119-119: null (MD047, single-trailing-newline)
Files should end with a single newline character
Additional comments not posted (3)
doc/build-macos.md (3)
19-23
: Ensure all dependencies are correctly listed.The command to install dependencies using Homebrew includes all necessary libraries and tools as mentioned in the PR objectives. However, ensure there are no trailing spaces after
python-setuptools m4
.Tools
Markdownlint
19-19: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
19-19: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
21-21: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
46-66
: Validate build instructions for Firo Core.The steps to prepare the build environment, configure, and build Firo Core are well-detailed. Ensure that the paths and commands are tested on the specified macOS version and hardware.
Tools
Markdownlint
46-46: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
46-46: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
48-48: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
55-55: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
47-47: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
Line range hint
68-93
: Review and secure the RPC configuration setup.The instructions for setting up the RPC configuration are critical for security. Ensure that the
rpcpassword
is generated securely as shown and that file permissions are set correctly. Recommend verifying the security practices with the latest macOS guidelines.Tools
Markdownlint
39-39: Expected: h3; Actual: h4 (MD001, heading-increment)
Heading levels should only increment by one level at a time
39-39: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
46-46: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
67-67: Expected: 0 or 2; Actual: 8 (MD009, no-trailing-spaces)
Trailing spaces
72-72: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
39-39: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
46-46: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
41-41: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
48-48: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
55-55: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
47-47: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
70-70: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- doc/build-macos.md (4 hunks)
Additional context used
LanguageTool
doc/build-macos.md
[uncategorized] ~109-~109: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ...isting Project 4. Enter "bitcoin-qt" as project name, entersrc/qt
as location 5. Lea...
[uncategorized] ~109-~109: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ...-qt" as project name, entersrc/qt
as location 5. Leave the file selection as it is 6....
Markdownlint
doc/build-macos.md
39-39: Expected: h3; Actual: h4 (MD001, heading-increment)
Heading levels should only increment by one level at a time
6-6: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
19-19: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
39-39: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
46-46: Expected: setext; Actual: atx (MD003, heading-style)
Heading style
29-29: Expected: 0 or 2; Actual: 6 (MD009, no-trailing-spaces)
Trailing spaces
67-67: Expected: 0 or 2; Actual: 8 (MD009, no-trailing-spaces)
Trailing spaces
13-13: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
25-25: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
72-72: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines
1-1: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
6-6: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
19-19: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
30-30: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
39-39: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
46-46: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
97-97: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
90-90: Punctuation: ':' (MD026, no-trailing-punctuation)
Trailing punctuation in heading
8-8: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
10-10: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
15-15: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
21-21: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
41-41: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
48-48: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
55-55: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
105-105: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
107-107: null (MD031, blanks-around-fences)
Fenced code blocks should be surrounded by blank lines
7-7: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
47-47: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
28-28: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
70-70: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
80-80: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
88-88: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
93-93: Expected: fenced; Actual: indented (MD046, code-block-style)
Code block style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fails on an M1 MacBook Air. The build reports no suitable m4
installation, and prompts to have the build tools installed again. This will keep occurring.
From discussions elsewhere, it sounds like this may be related to whether or not you have Xcode installed (not just the xcode-select
-installed tools), but I have not verified this.
doc/build-macos.md
Outdated
|
||
In case you want to build the disk image with `make deploy` (.dmg / optional), you need RSVG | ||
|
||
brew install librsvg | ||
brew install librsvg | ||
|
||
Berkley DB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This renders as a second-level heading that also contains the Firo Core build instructions, which is probably not what was intended. It might be more clear to have it under the dependency header, and then have the build instructions as a higher-level heading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That’s an interesting point. My tests were on Macs that all had Xcode installed, which likely explains why I didn’t encounter the same problem. Considering the size of the full Xcode, its installation is mostly unnecessary for users who are just trying to manually build the project. I’ll continue to look into this issue to find a workaround. If I can't find a better solution, I'll update our documentation to suggest installing Xcode. Also, I’ve taken your advice and reorganized the build instructions for clearer navigation. Thanks for your insightful comments!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plus it's apparently necessary to obtain an Apple ID in order to even obtain Xcode, which not all users may wish to do, and which imposes another effective build requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- doc/build-macos.md (3 hunks)
Additional context used
Markdownlint
doc/build-macos.md
31-31: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
42-42: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
6-6: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
20-20: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
31-31: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
40-40: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
42-42: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
49-49: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
69-69: Expected: 0 or 2; Actual: 8
Trailing spaces(MD009, no-trailing-spaces)
13-13: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
19-19: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
75-75: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
6-6: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
20-20: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
31-31: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
42-42: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
49-49: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
8-8: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
10-10: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
15-15: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
22-22: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
27-27: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
33-33: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
35-35: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
44-44: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
51-51: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
58-58: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
65-65: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
71-71: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
7-7: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
50-50: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
108-108: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
110-110: null
Fenced code blocks should be surrounded by blank lines(MD031, blanks-around-fences)
LanguageTool
doc/build-macos.md
[uncategorized] ~112-~112: You might be missing the article “the” here.
Context: ...isting Project 4. Enter "bitcoin-qt" as project name, entersrc/qt
as location 5. Lea...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~112-~112: You might be missing the article “the” here.
Context: ...-qt" as project name, entersrc/qt
as location 5. Leave the file selection as it is 6....(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Additional comments not posted (6)
doc/build-macos.md (6)
6-17
: Improve markdown consistency and code block formatting.The markdown formatting for headings and code blocks could be improved for consistency and readability. Consider using a consistent heading style across the document and ensure that code blocks are surrounded by blank lines for better separation and clarity.
-## Preparation +Preparation ------------ -1. Install macOS Command Line Tools (if not already installed): +1. Install macOS Command Line Tools (if not already installed): ```bash xcode-select --install-2. Install Homebrew (if not already installed):
+2. Install Homebrew (if not already installed):/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"
[EXISTING_COMMENT] <details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 6-6: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 13-13: Expected: 1; Actual: 2 Multiple consecutive blank lines (MD012, no-multiple-blanks) --- 6-6: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 8-8: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 10-10: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 15-15: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 7-7: null Lists should be surrounded by blank lines (MD032, blanks-around-lists) </blockquote></details> </details> --- `20-24`: **Clarify installation steps for dependencies and improve markdown consistency.** The command for installing dependencies is crucial and should be clearly explained. Ensure that all necessary dependencies are listed and consider breaking down the command into multiple lines for better readability. Also, improve markdown consistency. ```diff -## Dependencies +Dependencies ------------ -Install the required dependencies using Homebrew: +Install the required dependencies using Homebrew. Ensure all necessary tools are included: ```bash brew install automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf python qt libevent qrencode python-setuptools m4
[EXISTING_COMMENT] <details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 20-20: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 20-20: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 22-22: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) </blockquote></details> </details> --- `31-35`: **Ensure consistency in heading styles and improve code block formatting.** The markdown formatting for headings and code blocks could be improved for consistency and readability. Ensure that headings follow a consistent style and that code blocks are surrounded by blank lines. ```diff -#### Berkeley DB +Berkeley DB ------------ -It is recommended to use Berkeley DB 4.8. If you have to build it yourself, you can use [the installation script included in contrib/](https://github.com/bitcoin/bitcoin/blob/master/contrib/install_db4.sh) like so: +It is recommended to use Berkeley DB 4.8. If you have to build it yourself, you can use [the installation script included in contrib/](https://github.com/bitcoin/bitcoin/blob/master/contrib/install_db4.sh) as follows: ```bash ./contrib/install_db4.sh .
[EXISTING_COMMENT] <details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 31-31: Expected: h3; Actual: h4 Heading levels should only increment by one level at a time (MD001, heading-increment) --- 31-31: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 31-31: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 33-33: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 35-35: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) </blockquote></details> </details> --- `40-73`: **Streamline the build instructions and improve markdown consistency.** The steps to build Firo Core are crucial and should be presented in a clear and structured manner. Consider reorganizing these steps to enhance clarity and ensure that each step is accurately described. Also, ensure markdown consistency. ```diff -## Build Instructions +Build Instructions ------------------- -#### Download the Source +Download the Source -------------------- -Before building, download the Firo source code: +Before building, download the Firo source code: ```bash git clone https://github.com/firoorg/firo cd firo
-#### Build Firo Core
+Build Firo Core-1. Prepare the build environment:
+1. Prepare the build environment:cd depends make cd ..-2. Configure and build Firo-core:
+2. Configure and build Firo-core:./autogen.sh ./configure --prefix=`pwd`/depends/`depends/config.guess` make-3. (optional) It is recommended to build and run the unit tests:
+3. (optional) It is recommended to build and run the unit tests:./configure --prefix=`pwd`/depends/`depends/config.guess` --enable-tests make check-4. (optional) You can also create a .dmg that contains the .app bundle:
+4. (optional) You can also create a .dmg that contains the .app bundle:make deploy[EXISTING_COMMENT] <details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 42-42: Expected: h3; Actual: h4 Heading levels should only increment by one level at a time (MD001, heading-increment) --- 40-40: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 42-42: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 49-49: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 69-69: Expected: 0 or 2; Actual: 8 Trailing spaces (MD009, no-trailing-spaces) --- 42-42: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 49-49: Expected: 1; Actual: 0; Below Headings should be surrounded by blank lines (MD022, blanks-around-headings) --- 44-44: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 51-51: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 58-58: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 65-65: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 71-71: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 50-50: null Lists should be surrounded by blank lines (MD032, blanks-around-lists) </blockquote></details> </details> --- `107-110`: **Clarify the Qt Creator setup instructions and improve markdown consistency.** The instructions for setting up Qt Creator are somewhat unclear, especially regarding the configuration commands. Ensure that the steps are clear and correct any formatting issues. Also, ensure markdown consistency. ```diff -2. Properly configure the build environment: +2. Properly configure the build environment: ```bash ./configure --prefix=`pwd`/depends/`depends/config.guess` --enable-debug
[EXISTING_COMMENT] <details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 108-108: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) --- 110-110: null Fenced code blocks should be surrounded by blank lines (MD031, blanks-around-fences) </blockquote></details> </details> --- `123-124`: **Ensure markdown consistency in the "Notes" section.** The markdown formatting for the "Notes" section could be improved for consistency with the rest of the document. ```diff -Notes ------ -* Tested on macOS 10.11 through 10.14 on 64-bit Intel processors, and on macOS 14.5 on an M2 chip. -* Building with downloaded Qt binaries is not officially supported. See the notes in [#7714](https://github.com/bitcoin/bitcoin/issues/7714) +Notes ------ * Tested on macOS 10.11 through 10.14 on 64-bit Intel processors, and on macOS 14.5 on an M2 chip. * Building with downloaded Qt binaries is not officially supported. See the notes in [#7714](https://github.com/bitcoin/bitcoin/issues/7714)
… m4 detection issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (6)
doc/build-macos.md (6)
6-17
: Improve markdown consistency in the "Preparation" section.The markdown formatting for headings and code blocks can be improved for consistency and readability. Consider using a consistent heading style and ensuring that code blocks are surrounded by blank lines for better separation and clarity.
-## Preparation +Preparation ------------ -1. Install macOS Command Line Tools (if not already installed): +1. Install macOS Command Line Tools (if not already installed): ```bash xcode-select --install<details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 6-6: Expected: setext; Actual: atx Heading style (MD003, heading-style) </blockquote></details> </details> --- `20-24`: **Clarify installation steps for dependencies and improve markdown consistency.** Ensure that all necessary dependencies are listed and consider breaking down the command into multiple lines for better readability. Improve markdown formatting for consistency. ```diff -Install the required dependencies using Homebrew: +Install the required dependencies using Homebrew. Ensure all necessary tools are included: ```bash brew install automake berkeley-db4 libtool boost miniupnpc openssl pkg-config protobuf python qt libevent qrencode python-setuptools m4
<details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 20-20: Expected: setext; Actual: atx Heading style (MD003, heading-style) </blockquote></details> </details> --- `30-40`: **Improve markdown consistency in the "Ensure `m4` is Found" section.** Ensure that code blocks are surrounded by blank lines for better separation and clarity. ```diff ### Ensure `m4` is Found After installing `m4`, ensure that it is correctly linked and available in your PATH: ```bash brew link m4
You can verify that
m4
is properly installed by running:which m4This should output the path to the
m4
binary, typically/opt/homebrew/bin/m4
on Apple Silicon Macs.<details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 30-30: Expected: setext; Actual: atx Heading style (MD003, heading-style) </blockquote></details> </details> --- `42-46`: **Improve markdown consistency in the "Troubleshooting `m4` Issues" section.** Ensure that code blocks are surrounded by blank lines for better separation and clarity. ```diff ### Troubleshooting `m4` Issues If `m4` is not found even after installation, and running `brew link m4` does not resolve the issue, you may need to install Xcode to ensure that `m4` is recognized: 1. Install Xcode from the Mac App Store. 2. Once installed, open Xcode at least once to complete the setup.
Tools
Markdownlint
42-42: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
57-90
: Streamline the build instructions and improve markdown consistency.The steps to build Firo Core are crucial and should be presented in a clear and structured manner. Consider reorganizing these steps to enhance clarity and ensure that each step is accurately described. Improve markdown formatting for consistency.
## Build Instructions #### Download the Source Before building, download the Firo source code: ```bash git clone https://github.com/firoorg/firo cd firoBuild Firo Core
Prepare the build environment:
cd depends make cd ..Configure and build Firo-core:
./autogen.sh ./configure --prefix=`pwd`/depends/`depends/config.guess` make(optional) It is recommended to build and run the unit tests:
./configure --prefix=`pwd`/depends/`depends/config.guess` --enable-tests make check(optional) You can also create a .dmg that contains the .app bundle:
make deploy<details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 59-59: Expected: h3; Actual: h4 Heading levels should only increment by one level at a time (MD001, heading-increment) --- 57-57: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 59-59: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 66-66: Expected: setext; Actual: atx Heading style (MD003, heading-style) </blockquote></details> </details> --- `124-127`: **Clarify the Qt Creator setup instructions.** The instructions for setting up Qt Creator are somewhat unclear, especially regarding the configuration commands. Ensure that the steps are clear and correct any formatting issues. ```diff 2. Properly configure the build environment: ```bash ./configure --prefix=`pwd`/depends/`depends/config.guess` --enable-debug
</blockquote></details> </blockquote></details> <details> <summary>Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>Commits</summary> Files that changed from the base of the PR and between 4f77176011985af9d61ad457a7114de72d8d3f01 and d9b865e50ab660183a525cb8113ce20646af280a. </details> <details> <summary>Files selected for processing (1)</summary> * doc/build-macos.md (3 hunks) </details> <details> <summary>Additional context used</summary> <details> <summary>Markdownlint</summary><blockquote> <details> <summary>doc/build-macos.md</summary><blockquote> 59-59: Expected: h3; Actual: h4 Heading levels should only increment by one level at a time (MD001, heading-increment) --- 6-6: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 20-20: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 30-30: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 42-42: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 48-48: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 57-57: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 59-59: Expected: setext; Actual: atx Heading style (MD003, heading-style) --- 66-66: Expected: setext; Actual: atx Heading style (MD003, heading-style) </blockquote></details> </blockquote></details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested builds with and without GUI, as well as the deploy
target.
doc/build-macos.md
Outdated
After installing `m4`, ensure that it is correctly linked and available in your PATH: | ||
```bash | ||
brew link m4 | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This returned a warning to pass --force
to the command to properly set symlinks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (4)
doc/build-macos.md (4)
20-29
: LGTM: Comprehensive dependency list with clear installation instructions.The dependencies section is well-detailed and includes all necessary packages. The addition of
python-setuptools
andm4
is appropriate for the build process.Consider adding a brief explanation for why
python-setuptools
andm4
are necessary, to help users understand their importance in the build process.Tools
Markdownlint
20-20: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
31-48
: LGTM: Comprehensive m4 setup and troubleshooting instructions.The sections on ensuring m4 is found and troubleshooting m4 issues are well-detailed and provide valuable information for users. The explanation of the keg-only nature of m4 in Homebrew is particularly helpful.
Consider adding a note explaining why m4 is crucial for the build process, to help users understand its importance.
Tools
Markdownlint
31-31: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
43-43: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
Line range hint
58-128
: LGTM: Comprehensive and well-structured build instructions.The build instructions section is thorough and well-organized, covering source download, build process, optional unit testing, and .dmg creation. The updated Qt Creator instructions are helpful for developers using an IDE.
For consistency, consider using the same heading level for all main sections (e.g., "Download the Source" and "Build Firo Core" could be at the same level as "Build Instructions").
Tools
Markdownlint
60-60: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
6-6: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
20-20: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
31-31: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
43-43: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
49-49: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
58-58: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
60-60: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
67-67: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
Line range hint
6-141
: Consider addressing markdown formatting inconsistencies.The document structure could be improved by addressing the following markdown formatting issues:
- Ensure consistent heading styles throughout the document (either all ATX or all Setext style).
- Maintain a logical heading level hierarchy, incrementing only one level at a time.
These changes would enhance the document's structure and improve its adherence to markdown best practices.
Tools
Markdownlint
60-60: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
6-6: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
20-20: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
31-31: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
43-43: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
49-49: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
58-58: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
60-60: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
67-67: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- doc/build-macos.md (3 hunks)
Additional context used
Markdownlint
doc/build-macos.md
60-60: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
6-6: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
20-20: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
31-31: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
43-43: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
49-49: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
58-58: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
60-60: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
67-67: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
Additional comments not posted (3)
doc/build-macos.md (3)
6-17
: LGTM: Clear and concise preparation instructions.The preparation section is well-structured, providing clear instructions for installing macOS Command Line Tools and Homebrew. The formatting is consistent and follows markdown best practices.
Tools
Markdownlint
6-6: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
49-57
: LGTM: Clear instructions for Berkeley DB setup.The Berkeley DB section provides clear recommendations and instructions for installation. The reference to the installation script in the contrib folder is helpful.
Tools
Markdownlint
49-49: Expected: setext; Actual: atx
Heading style(MD003, heading-style)
141-141
: LGTM: Updated compatibility information.The notes section has been appropriately updated to include testing information for macOS 14.5 on an M2 chip, which is valuable for users with newer hardware.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, except I haven't tested the Berkeley DB installation.
Description
This pull request revises the macOS build instructions for the Firo wallet, addressing dependency recognition and configuration issues on macOS Sonoma 14.5 with an M chip Mac.
Key Changes Include:
Boost Library Configuration:
Dependency Installation Adjustments:
m4
andpython-setuptools
in the Homebrew installation commands, addressing issues where these tools were not recognized post-installation. This ensures a more streamlined setup and reduces configuration errors during the build process.Environment Setup:
Purpose:
The updates aim to streamline the setup process for developers on macOS, ensuring all key dependencies like Boost,
m4
, andpython-setuptools
are installed and recognized correctly, enhancing the clarity and accuracy of the build instructions.Testing:
These revised instructions have been tested on macOS Sonoma 14.5 with an M chip Mac.