Skip to content

doc: revamp macOS build doc #21343

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 18, 2021
Merged

doc: revamp macOS build doc #21343

merged 1 commit into from
Mar 18, 2021

Conversation

jarolrod
Copy link
Member

@jarolrod jarolrod commented Mar 2, 2021 •
edited
Loading

This PR makes the macOS build-docs more informative and adds in the following information:

  • Proper descriptions and delineation of required/optional dependencies
  • walk-through of optional dependencies
  • configuration walk-through
  • various other tidbits of information

This is a part of the efforts done in #20601 and #20610 to update the docs and introduce some consistency between them.

This update does not add instructions for arm-based M1 Macbooks as I do not have one to test with. It would be nice to have someone follow up with an update containing instructions for arm-based Macs.

Before/Master: render
After/PR: render

@hebasto
Copy link
Member

hebasto commented Mar 2, 2021

Why dropped "Usually, macOS installation already has a suitable SQLite installation." ?

@jarolrod
Copy link
Member Author

jarolrod commented Mar 2, 2021 •
edited
Loading

Why dropped "Usually, macOS installation already has a suitable SQLite installation." ?

Since it was introduced in 10.14, which is our minimum supported macOS, I guess you're right in that it can stay

hebasto
hebasto reviewed Mar 2, 2021
View reviewed changes
hebasto
hebasto reviewed Mar 2, 2021
View reviewed changes
@DrahtBot DrahtBot added the Docs label Mar 2, 2021
@jarolrod
Copy link
Member Author

jarolrod commented Mar 3, 2021

updated from 6d08ec2 -> 4780ea7, addressed @hebasto suggestions

changes:

  • use new 'safe' name for berekeley-db
  • git is not exactly a build dependency, so it has been removed
  • Various little text-fix ups found upon another read through

fanquake
fanquake reviewed Mar 3, 2021
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

It seems sub optimal that we are basically just copy pasting blocks of dependencies throughout all the build docs. Isn't the point of dependencies.md to list the dependencies, what they are, and when they are required?

I think the same can be said for all of the "configuration" blocks. These seem to get repeated (across lots of docs) in varying combinations and levels of verbosity, and I'd think a lot of the time they might just be downright confusing to a first time builder.

This also includes the parts where configure output is basically pasted verbatim into documentation. i.e upnp or libnatpmp options. Can't we just tell users to look at configure --help? This means they'll always be getting accurate information; rather than something pasted into a build doc that now may have drifted from what actually happens.

doc/build-osx.md
## Dependencies
```shell
brew install automake libtool boost miniupnpc libnatpmp pkg-config python qt libevent qrencode
Homebrew is a package manager for macOS that allows one to install packages from the command line easily.
Copy link
Member

Choose a reason for hiding this comment

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

I preferred the one liner with a link to https://docs.brew.sh/Installation. I don't think we need a paragraph explaining brew, or listing the actual install invocation.

Copy link
Member Author

Choose a reason for hiding this comment

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

removed the install invocation in c180c91

I want this paragraph to just be a quick explanation that the commands use homebrew but they are free to use whatever package manager they use and adapt the commands. I added the following line to make this clearer:

Otherwise, you can adapt the commands to your package manager of choice.

@jarolrod
Copy link
Member Author

jarolrod commented Mar 3, 2021 •
edited
Loading

It seems sub optimal that we are basically just copy pasting blocks of dependencies throughout all the build docs. Isn't the point of dependencies.md to list the dependencies, what they are, and when they are required?

Not exactly copy-pasting. Naming and links are different. I don't think it would be 'better' to point to dependencies.md and have the user figure out how that dependency correlates to their system.

I think the same can be said for all of the "configuration" blocks. These seem to get repeated (across lots of docs) in varying combinations and levels of verbosity, and I'd think a lot of the time they might just be downright confusing to a first time builder.

Just trying to improve that situation across the build docs so that it's informative in the right ways to get someone up and running with a couple of examples that are relevant.

This also includes the parts where configure output is basically pasted verbatim into documentation. i.e upnp or libnatpmp options. Can't we just tell users to look at configure --help? This means they'll always be getting accurate information; rather than something pasted into a build doc that now may have drifted from what actually happens.

Well I was thinking that the build docs can be a one-stop place on how to get up and running. I think it would be way more confusing for a first time builder to have to hop through 3 different files and try to piece together the right information.

The information doesn't need to drift away from what's happening if it's maintained.

@DrahtBot DrahtBot mentioned this pull request Mar 3, 2021
Merged
@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 3, 2021 •
edited
Loading

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

Conflicts

No conflicts as of last run.

@jarolrod
doc: revamp macOS build doc
c180c91 
This pr makes the macOS build docs more informative and adds in the following information:
- Proper descriptions and delineation of required/optional dependencies
- walk-through of optional dependencies
- configuration walk-through
- various other tid-bits of information
@jarolrod
Copy link
Member Author

jarolrod commented Mar 3, 2021

Updated from 4780ea7 -> c180c91, rebased to resolve conflict with #21346 and addressed some of @fanquake suggestions.

changes:

  • updated references of qt to qt@5
  • Removed confusing Xcode Tools dependency
  • Removed specific configuration to port mapping and zmq in favor of pointing the user to check out ./configure -help

Justification:
I guess, following from fanquake's comment , this may need some more justification. I simply think that this adds value and the format can help a first-time builder get up and running. I certainly don't want to waste reviewer time or contribute something that would be an annoyance to maintain. If this PR falls into that, then it's ok if its closed.

@DrahtBot
Copy link
Contributor

🕵️ @harding has been requested to review this pull request as specified in the REVIEWERS file.

fanquake
fanquake approved these changes Mar 18, 2021
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK c180c91 - I still think these are getting too verbose and we're duplicating information all over the place; dependencies, configure options, combinations of options etc. However if people are happy to maintain them, I guess it's fine for now, and this revamping has already happened for some of the other build READMEs.

@fanquake fanquake merged commit d6e3ac8 into bitcoin:master Mar 18, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 18, 2021
@fanquake
Merge bitcoin#21343: doc: revamp macOS build doc
8318ad1
@jarolrod jarolrod deleted the macos-build-docs branch March 25, 2021 20:35
@jarolrod jarolrod mentioned this pull request May 16, 2021
Closed
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@hebasto hebasto hebasto left review comments

@fanquake fanquake fanquake approved these changes

Assignees
No one assigned
Labels
Docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jarolrod @hebasto @DrahtBot @fanquake