-
Notifications
You must be signed in to change notification settings - Fork 36.7k
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
[doc] Update OS X build notes for new qt5 configure #7040
Conversation
make | ||
|
||
3. It is also a good idea to build and run the unit tests: | ||
By default 'make' will also run the unit tests. |
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.
Nit: Are you sure?
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.
You need to specifically call make check
to run the unit-tests.
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.
Not sure why I thought this was the case. 'make' is still only compiling the tests...
I think we should either give instructions to build "bitcoin-core" (including the GUI) at point 2) or add another point in how to build the GUI (--with-gui). It's only mentioned in the Qt-Creator instructions (which is somehow pretty hard to setup and not really necessary). |
It is already implicitly mentioned in this step. (via "brew install [...] qt5 libevent"). So it should be sufficient to adjust the headings (or text to mention gui) and state that the qt-creator step is not needed. |
Agree with @MarcoFalke: just changing "2. bitcoind" to "2. bitcoin-core" would be enough. But still, I think we should mention how to build bitcoin-core without GUI then. |
Reverted the 'make check' changes, and updated 2. to bitcoin-core |
@@ -43,10 +43,10 @@ NOTE: Building with Qt4 is still supported, however, could result in a broken UI | |||
git clone https://github.com/bitcoin/bitcoin.git | |||
cd bitcoin | |||
|
|||
2. Build bitcoind: | |||
2. Build bitcoin-core: | |||
|
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.
Nit: Change " ### Building bitcoind
" to " ### Building bitcoin"
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.
Nit: Add a line with sth. like: "This will configure and build the headless bitcoin binaries and the bitcoin gui (if qt is found). You can disable the gui build with --disable-gui
"?
ACK You can add something like this: If you want to build Bitcoin Core without graphical user interface, add an option |
Updated to address nits. |
@@ -60,7 +62,7 @@ NOTE: Building with Qt4 is still supported, however, could result in a broken UI | |||
Use Qt Creator as IDE | |||
------------------------ | |||
You can use Qt Creator as IDE, for debugging and for manipulating forms, etc. | |||
Download Qt Creator from http://www.qt.io/download/. Download the "community edition" and only install Qt Creator (uncheck the rest during the installation process). | |||
Download Qt Creator from https://www.qt.io/download/. Download the "community edition" and only install Qt Creator (uncheck the rest during the installation process). | |||
|
|||
1. Make sure you installed everything through Homebrew mentioned above | |||
2. Do a proper ./configure --with-gui=qt5 --enable-debug |
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.
Sorry, another nit: remove -with-gui=qt5
?
@jonasschnelli nit fixed. |
ACK |
The reason to keep passing |
@laanwj yes, but as someone already mentioned, this describes the build on OS X and after |
2. Build bitcoind: | ||
2. Build bitcoin-core: | ||
This will configure and build the headless bitcoin binaries as well as the gui (if Qt is found). | ||
You can disable the gui build by passing `--disable-gui` to configure. |
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.
@paveljanik is right. This is incorrect. Should be --without-gui
.
You no longer need to explicitly pass qt5 to configure, as it will now choose qt5 over qt4 if both are installed. [skip-ci]
Nit fixed. |
ACK |
@@ -5,7 +5,7 @@ This guide will show you how to build bitcoind (headless client) for OS X. | |||
Notes | |||
----- | |||
|
|||
* Tested on OS X 10.7 through 10.10 on 64-bit Intel processors only. | |||
* Tested on OS X 10.7 through 10.11 on 64-bit Intel processors only. |
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.
OS X 10.11: #7110
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.
@jonasschnelli Does this need any further changes? |
Re-ACK. Only problem could be that someone ends up with a GUI-less bitcoin-core (missing bitcoin-qt) because he forget to install qt5 (would happen without error only a tiny warning/info in the |
b171c69 [doc] Update OS X build notes for new qt5 configure (Michael Ford)
You no longer need to explicitly pass qt5 to configure, as it will now choose qt5 over qt4 if both are installed.
[skip-ci]