Bitcoin-Qt: fix build of splashscreen.cpp #2538

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:fix-splash changing 1 files +2 −1

Diapolo commented at 1:04 PM on April 18, 2013: none
- include QApplictaion at the top-most position to make it compile again via Qt Creator on Windows with Qt5

Bitcoin-Qt: fix build of splashscreen.cpp

- include QApplictaion at the top-most position to make it compile again
  via Qt Creator on Windows with Qt5

4193a64735

BitcoinPullTester commented at 1:32 PM on April 18, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4193a647354634326957f5e00d53fc474a36e125 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.
in src/qt/splashscreen.cpp:None in 4193a64735
```
   0 | @@ -1,9 +1,10 @@
   1 | +#include <QApplication>
```
laanwj commented at 1:33 PM on April 18, 2013:

Don't do this. We always include internal .h files first, and only then the Qt ones. If this is needed it means that one of our own header files is missing #include <QApplication>

Diapolo commented at 1:40 PM on April 18, 2013:

If you take a look at some other Qt files, you'll see this is already the case in several files, which I also couldn't compile. If you are able to come up with a solution that works, that is fine :).

laanwj commented at 1:49 PM on April 18, 2013:

I don't have qt5 on windows. Can you paste the exact error message that you get?

Diapolo commented at 4:05 PM on April 20, 2013:

@laanwj I get this error without my patch.

laanwj commented at 4:30 PM on April 20, 2013:

That's curious. Include order shouldn't matter here. What is the offending code sequence in qcoreapplication.h:47?

Diapolo commented at 2:11 PM on April 25, 2013:

@laanwj I really see no relation, as line 47 in qcoreapplication.h is this #include <QtCore/qeventloop.h>. Any further idea?

laanwj commented at 12:04 PM on April 26, 2013:

My guess is that there is a macro defined somewhere in one of the headers (splashscreen, clientversion or util) that conflicts with the offending line in qeventloop.h:98.
laanwj commented at 4:45 PM on May 21, 2013: member

I've had to fix this at well in the Qt5 stream, it's indeed another Satoshi-ism in util.h that gets in the way:

https://github.com/laanwj/bitcoin/commit/9680be9396295c6103b354bfd37d2c2bdb73d4a8#diff-1

Do you need this for Qt4?
Diapolo commented at 1:30 PM on May 22, 2013: none

Wow, nice find... rather ugly though ^^. You mean if I need loop defined for Qt4?
laanwj commented at 2:07 PM on May 22, 2013: member

You need to undefine it.
Diapolo commented at 2:22 PM on May 22, 2013: none

Sorry, I meant your question Do you need this for Qt4?, because I'm not sure what you mean. Need the undef for Qt4? Need loop for Qt4?

Perhaps we should just replace loop in the core files with while(true)?
laanwj commented at 2:43 PM on May 22, 2013: member

Maybe. I don't like that loop macro at all either. But I don't want to change the core files for what is a qt compatibility fix.

My question meant "do you need this fix to build for qt4 on windows as well". Otherwise we can just close this issue here.
Diapolo commented at 7:41 PM on May 22, 2013: none

@laanwj I didn't try with Qt4 for some time now, let me see if my Qt4 build is still working... It IS working and I can confirm this patch or your is not needed for Qt4.

Do you intend to fix the includes I changed and replace with that #undef then in our Qt files?
laanwj commented at 12:05 PM on May 31, 2013: member

closing this, it will automatically be fixed when qt5 branch merged
laanwj closed this on May 31, 2013
DrahtBot locked this on Sep 8, 2021

Contributors

Diapolo

BitcoinPullTester

laanwj