This is the beginning of an autotools build-system refactor. My first step is to be able to create a common static lib for the binaries (console/qt/test) to link against, meaning that each of those will need its own entry-point object file.
This series gets rid of the QT_GUI define in favor of letting the build-system link in the proper startup file, and a global var to determine how the binary was started.
I've tried to match this project's formatting and general practices.
Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/37d4ab5e11da48e7627b446a2fae716555c58f18 for binaries and test log.
This could happen for one of several reasons:
If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/feb4b7ef8d3ffb49b9bca41928051f7a8f6b4f29 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
What's the benefit of this refactor? I like ifdefs. This change will mean we have GUI only code in the non-GUI binary (and vicaversa).
@jonasschnelli: Compilation speed and sanity. This allows compiling the core objects only once and storing them in an archive (library) to be used by the all of bitcoin-qt, bitcoind and the tests.
There are only very few differences left under QT_GUI defines (some help messages, and the default key which is going away anyway) so I honestly don't see the "GUI code in non-GUI" problem.
Any QT_GUI ifdef in the core code means that the core <-> GUI split wasn't done correctly in the first place. The core semantics shouldn't depend on whether there is a GUI on top of it or not...
0 | @@ -0,0 +1,23 @@ 1 | +// Copyright (c) 2009-2010 Satoshi Nakamoto 2 | +// Copyright (c) 2009-2012 The Bitcoin developers 3 | +// Distributed under the MIT/X11 software license, see the accompanying 4 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php. 5 | + 6 | +#include "init.h" 7 | + 8 | +extern void noui_connect();
Suggestion: move AppInit to this file as well. It is a bit empty, and AppInit is only used by the bitcoind (not by either bitcoin-qt or the tests), so it fits here.
I left AppInit where it was because there's no logical reason to split it out (meaning no risk of symbol collisions if left in the static archive). It could feasibly be used to start a non-graphical session via the qt binary using something like 'bitcoin-qt --no-gui' if such behavior was ever desired in the future. But if there's opposition, sure, I can do that.
Such a feature (start a daemon session from bitcoin-qt) will never be supported. There is already the debug console window and the --server option.
Functions that are only used by either the UI or daemon should be moved to their respective frontends (unless there is a very good reason not to, like keeping methods acting on an object together, but I disagree there is one in this case).
OK. Will move.
ACK on the concept; I haven't compiled/tested.
ACK compile and run (Bitcoin-Qt.app and bitcoind) smooth on osx 10.8.
@jonasschnelli you said: "I like ifdefs". If that's the resounding attitude here, I'll need to reevaluate my autotools work. Ifdefs are a portability nightmare if used to control runtime behavior. In this case, depending on the compiler/linker/settings used, the unreachable paths might be stripped away just as if ifdefs had been used.
Would you mind explaining your position, and if it's common for bitcoin development? I have no problem adapting (even if i disagree), but it may negate the reasoning for my work.
@theuni I don't think @jonasschnelli 's ifdef love is representative of most developers here. It makes sense for some optional build-time features (UPnP, IPv6), but not so much in this case.
@theuni: no. just go on (even when i – generally – like #ifdef's). I just like binaries that only containing code which will be runned through. But for this case ifdefs are not the right thing.
Roger, thanks. I have more to say on the subject, but I'll do it in code/PR form as I go rather than discussing vague concepts here.
#ifdefs are evaluated on a case-by-case basis, if they make sense. If the GUI supports a runtime switch that forces daemon mode, then "#ifdef GUI" construct is not applicable for that build. It depends on the build and platform. Clearly, #ifdef GUI is applicable to a bitcoind-only build.
Thus, just giving that one example, you can see where #ifdef is, and is not, applicable. Personal preference never enters into the equation. If #ifdef is applicable, it is used. Otherwise it is not.
ACK on the general concept
Ok, I've pushed changes to address comments here. I'll squash it down if everyone is ok with the above.
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/c5e9ebcfb206deb19db166db4063120f220a3f3b for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
Any reason why the init_noui code isn't just in noui?
Good idea.
yep, makes perfect sense. will do.
This will allow each to have its own main(), meaning that we can build a common
base client and simply link in the correct startup object to create the
appropriate binary.
renamed init_noui.cpp to bitcoind.cpp, rebased to current HEAD, and squashed logically.
Anything else needed?
Grr, I just reread the comment above and realized I didn't make the change you guys were after. doing now.
Actually, the noui stuff can't move into bitcoind.cpp, as that would defeat the purpose of this rework. That would mean that test_bitcoin would have to link in bitcoind.o for noui_connect(), while also pulling in a conflicting main().
The object containing main() needs to stay as thin as possible.
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/6887bb9ad7f6722a64e9fbdf0a780f362dc796a1 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
ACK
ACK
Great, best of all worlds :)
great, thanks