doc: added note about brew and Qt 6 on macOS #25948

da2ce7 commented at 12:06 AM on August 29, 2022: none

In the issues: #25947 and #21901 , I and others did not follow the instructions despite intending to do so.

So I created this simple documentation update fordoc/build-osx.md in a way that I expect it would have caught my attention.

DrahtBot added the label Docs on Aug 29, 2022

Sjors commented at 2:09 PM on August 31, 2022: member

ACK 94d1505dbe3d33e593b2a62ef12bb0478100122c

in doc/build-osx.md:100 in 94d1505dbe outdated

  95 | @@ -96,11 +96,13 @@ Skip if you don't intend to use the GUI.
  96 |  brew install qt@5
  97 |  ```
  98 |  
  99 | -Ensure that the `qt@5` package is installed, not the `qt` package.
 100 | -If 'qt' is installed, the build process will fail.
 101 | -if installed, remove the `qt` package with the following command:
 102 | +_Note:_ Ensure that _only_ `qt@5` is installed:
 103 | +> If the `qt` package, i.e. 'Qt 6', is installed, configuring Bitcoin will find 'Qt 5' as expected, however the build process will still fail. [#25947](https://github.com/bitcoin/bitcoin/issues/25947)

jarolrod commented at 3:51 PM on August 31, 2022:

I wouldn't move this into quotes, no need for that. I also wouldn't link to a specific issue here, and some cleanup suggestions.

If you've previously grabbed the `qt` package, which installs Qt 6, you will not be able to compile.

in doc/build-osx.md:102 in 94d1505dbe outdated

 100 | -If 'qt' is installed, the build process will fail.
 101 | -if installed, remove the `qt` package with the following command:
 102 | +_Note:_ Ensure that _only_ `qt@5` is installed:
 103 | +> If the `qt` package, i.e. 'Qt 6', is installed, configuring Bitcoin will find 'Qt 5' as expected, however the build process will still fail. [#25947](https://github.com/bitcoin/bitcoin/issues/25947)
 104 | +
 105 | +You may check for and uninstall packages with the following commands:

jarolrod commented at 4:21 PM on August 31, 2022:

You may check for and uninstall the `qt` package with the following commands:

jarolrod commented at 4:25 PM on August 31, 2022: member

concept ack

da2ce7 force-pushed on Aug 31, 2022

da2ce7 commented at 5:29 PM on August 31, 2022: none

@Sjors

Thank you for your review, I have partly adopted @jarolrod suggestions:

Include a brief reason why the compiling will fail.
Remove the link to the issue as suggested.
I chose not make the assumption that the user already has qt installed; maybe they install it after and find their builds stop working!

Zero-1729 commented at 5:33 AM on September 1, 2022: contributor

Concept ACK dc8e79bac208a72193e452cdcc0c9ada4dc7043b

in doc/build-osx.md:99 in dc8e79bac2 outdated

  95 | @@ -96,11 +96,13 @@ Skip if you don't intend to use the GUI.
  96 |  brew install qt@5
  97 |  ```
  98 |  
  99 | -Ensure that the `qt@5` package is installed, not the `qt` package.
 100 | -If 'qt' is installed, the build process will fail.
 101 | -if installed, remove the `qt` package with the following command:
 102 | +_Note:_ Ensure that _only_ `qt@5` is installed:

Zero-1729 commented at 5:34 AM on September 1, 2022:

nit: This seems to be the only place where "Note" is italicized in the doc; it might be worth reverting to match the other notes. Italicizing also seems redundant here for emphasis, as it's already a note.

Note: Ensure that _only_ `qt@5` is installed:

da2ce7 commented at 9:28 AM on September 1, 2022:

Done :)

da2ce7 force-pushed on Sep 1, 2022

Zero-1729 commented at 2:54 AM on September 3, 2022: contributor

crACK 0d600b999934a55c765cb2dcfd3ca64370dbcc97

Note: documentation/comments related commits are prefixed with doc: (in lowercase), not Docs:. So the commit message should be updated to doc: added note about brew and Qt 6 on macOS or something similar.

da2ce7 force-pushed on Sep 3, 2022

da2ce7 renamed this:
~~Docs: macOS, make note about brew and Qt 6 more explicit.~~
doc: added note about brew and Qt 6 on macOS
on Sep 3, 2022

da2ce7 commented at 4:37 AM on September 3, 2022: none

@Zero-1729 Thank you, I have updated the message of the comment according to your suggestion.

Zero-1729 commented at 1:31 PM on September 3, 2022: contributor

ACK a786182632c55a523b1c3851ece7eaeaeb37f919

in doc/build-osx.md:100 in a786182632 outdated

  95 | @@ -96,11 +96,13 @@ Skip if you don't intend to use the GUI.
  96 |  brew install qt@5
  97 |  ```
  98 |  
  99 | -Ensure that the `qt@5` package is installed, not the `qt` package.
 100 | -If 'qt' is installed, the build process will fail.
 101 | -if installed, remove the `qt` package with the following command:
 102 | +Note: Ensure that _only_ `qt@5` is installed:
 103 | +> If the `qt` package, which provides Qt 6, is installed alongside `qt@5`, configuring will find Qt 5, but compiling will fail because some headers are incorrectly imported from Qt 6 locations.

jarolrod commented at 3:18 AM on September 6, 2022:

Please unquote this

If the `qt` package, which provides Qt 6, is installed alongside `qt@5`, configuring will find Qt 5, but compiling will fail because some headers are incorrectly imported from Qt 6 locations.

jarolrod commented at 3:18 AM on September 6, 2022:

It's sufficient to say that you can't build if qt6 is installed, we don't need to go into any technical detail. The only information needed is that qt@5 is the proper package and not qt6. I'd still prefer something like this:

If you've previously grabbed the `qt` package, which installs Qt 6, you will not be able to compile.

da2ce7 commented at 11:32 AM on September 8, 2022:

@jarolrod I have removed the quote formatting as suggested.

Can you please explain why there is no need to go into rationale that you cannot have both Qt 5 and Qt 6 installed simultaneously? After-all Qt 6 is becoming quite common now, and it quite reasonable that some developers will think: "Hey, why not? I need Qt 6 for this other project I'm working on.".

doc: added note about brew and Qt 6 on macOS 7d67976d6d

da2ce7 force-pushed on Sep 8, 2022

luke-jr commented at 1:23 AM on September 9, 2022: member

Seems like we should just fix the bug. Unrelated packages shouldn't cause problems.

kouloumos commented at 9:07 AM on September 9, 2022: contributor

ACK 7d67976d6d4e454f958d923189095a0484383712,

I also dealt with this problem once. This version of the docs would probably save me a round-trip to my favourite search-engine.

fanquake commented at 10:27 AM on September 9, 2022: member

~0

Having both qt5 and qt6 installed, on an M1 macOS system doesn't cause any issues, so I don't think the problem has been properly/fully identified, and this addition would be incorrect for modern / macOS systems going forward.

hebasto commented at 6:48 PM on September 9, 2022: member

Seems like we should just fix the bug. Unrelated packages shouldn't cause problems.

Having both qt5 and qt6 installed, on an M1 macOS system doesn't cause any issues, so I don't think the problem has been properly/fully identified, and this addition would be incorrect for modern / macOS systems going forward.

Also #25947 (comment)

Agree with @luke-jr and @fanquake -- let's fully identify the problem first.

Zero-1729 commented at 12:38 PM on September 10, 2022: contributor

Reading through the comments and issue, agree as well, makes sense to narrow down the issue.

hebasto commented at 1:15 PM on September 10, 2022: member

Reading through the comments and issue, agree as well, makes sense to narrow down the issue.

#26056 could fix it.

fanquake commented at 11:05 AM on September 13, 2022: member

Going to close this in favour of fixing the code. Would be great if anyone who's run into this issue could test #26056 instead.

fanquake closed this on Sep 13, 2022

bitcoin locked this on Sep 13, 2023