Skip to content

Bitcoin-Qt: massive header and cpp cleanup#2210

Merged
laanwj merged 1 commit intobitcoin:masterfrom
Diapolo:Qt_header_cpp_cleanup
Mar 18, 2013
Merged

Bitcoin-Qt: massive header and cpp cleanup#2210
laanwj merged 1 commit intobitcoin:masterfrom
Diapolo:Qt_header_cpp_cleanup

Conversation

@Diapolo
Copy link
Copy Markdown

@Diapolo Diapolo commented Jan 23, 2013

This pull is currently here to see how pull-tester is handling it. To really be sure I didn't break something, this pull should be sent through a Gitian build.

I tried to harmonize the layout / style in all changed files. The QApplication moved to top in bitcoin.cpp and bitcoingui.cpp is related to Qt5 compatibility, I need to see if this is working correctly before I start doing my Qt5 compatibility pull.

@laanwj I know this has the potential to make you angry (:-P)or at least may steal some of your time, but in the end it should be well worth it. I even think removed includes could speed up compilation time a little :).

  • try to enforce the same style to all Qt related files
  • remove unneeded includes from the files
  • add missing Q_OBJECT, QT_BEGIN_NAMESPACE / QT_END_NAMESPACE
  • prepares for a pull-req to include Qt5 compatibility

@Diapolo
Copy link
Copy Markdown
Author

Diapolo commented Jan 23, 2013

  • fixed needed #include <QTextDocument> in sendcoinsdialog.cpp

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/c8c9c3bad62bb39266535e99fe6206b2de3f6dfe for binaries and test log.

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/167761c60514b524eb019039c14d243b345b888b for binaries and test log.

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/b00139fbfadbcb15af0e285aacf1f7d6728ff166 for binaries and test log.

This could happen for one of several reasons:

  1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts
  2. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
  3. The test suite fails on either Linux i386 or Win32
  4. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

@Diapolo
Copy link
Copy Markdown
Author

Diapolo commented Feb 7, 2013

Seems to be a @BitcoinPullTester error...

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f162a728e535ce6557cbdfd2fed32cbb3445e801 for binaries and test log.

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: WARNING, see http://jenkins.bluematt.me/pull-tester/726e30abf847e8e2606e14706641846aa8746751 for binaries and test log.

This pull decreases total test coverage, please add unit tests to test all new code and help us add test cases for existing code.
Coverage report can be found at http://jenkins.bluematt.me/pull-tester/726e30abf847e8e2606e14706641846aa8746751/bitcoin/src/total.coverage/

@Nothing4You
Copy link
Copy Markdown
Contributor

I just installed qt5 on my system (which is used as default qt now) and building bitcoin-qt no longer works. Merging this branch also isn't possible anymore.

[...]
cd /home/rschwab/build/bitcoin-git/src/bitcoin-build; /bin/sh share/genbuild.sh /home/rschwab/build/bitcoin-git/src/bitcoin-build/build/build.h
g++ -c -m64 -pipe -fstack-protector-all -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2 -D_REENTRANT -fdiagnostics-show-option -Wall -Wextra -Wformat -Wformat-security -Wno-unused-parameter -Wstack-protector -fPIE -DQT_GUI -DBOOST_THREAD_USE_LIB -DBOOST_SPIRIT_THREADSAFE -DUSE_UPNP=1 -DSTATICLIB -DUSE_IPV6=1 -DHAVE_BUILD_INFO -DLINUX -D_FILE_OFFSET_BITS=64 -DQT_NO_DEBUG -DQT_NETWORK_LIB -DQT_GUI_LIB -DQT_CORE_LIB -I/usr/lib/qt/mkspecs/linux-g++-64 -Isrc -Isrc/json -Isrc/qt -Isrc/leveldb/include -Isrc/leveldb/helpers -I/usr/include/qt5 -I/usr/include/qt5/QtNetwork -I/usr/include/qt5/QtGui -I/usr/include/qt5/QtCore -Ibuild -o build/bitcoin.o src/qt/bitcoin.cpp
In file included from src/qt/bitcoin.cpp:4:0:
src/qt/bitcoingui.h:4:23: fatal error: QMainWindow: No such file or directory
compilation terminated.
make: *** [build/bitcoin.o] Error 1

@Diapolo
Copy link
Copy Markdown
Author

Diapolo commented Mar 17, 2013

There is currently no Qt5 compatibility. I have the needed changes in my local build, but as long as this one is not merged, I don't want to open the Qt5 compatibility pull.

@Nothing4You
Copy link
Copy Markdown
Contributor

I'm using arch linux, can you suggest me a better way than removing qt5 while building bitcoin-qt, then reinstalling it?

@Diapolo
Copy link
Copy Markdown
Author

Diapolo commented Mar 17, 2013

I'm no Linux guy, sorry... is there any way to point your compiler to the old Qt4 instead of Qt5?
As long as @laanwj doesn't merge this, because of other refactoring pulls you will be unlucky, sorry.

@Nothing4You
Copy link
Copy Markdown
Contributor

I'm sure there is a way, I just have no idea how I would do that, which is why I asked.
Using qmake-qt4 should be the solution, didn't test it yet though as I don't have the time to compile now.

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/ba32615717fe58bbd0b58f4696392cdd7116e3fb for binaries and test log.

This could happen for one of several reasons:

  1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts
  2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
  3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
  4. The test suite fails on either Linux i386 or Win32
  5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

- try to enforce the same style to all Qt related files
- remove unneeded includes from the files
- add missing Q_OBJECT, QT_BEGIN_NAMESPACE / QT_END_NAMESPACE
- prepares for a pull-req to include Qt5 compatibility
@laanwj
Copy link
Copy Markdown
Member

laanwj commented Mar 17, 2013

@Diapolo I'm fine with merging this now.

Seems there is a merge conflict though:

src/qt/paymentserver.cpp:122: error: invalid use of incomplete type 'struct QUrl'
/usr/include/qt4/QtCore/qmetatype.h:260: error: forward declaration of 'struct QUrl'

@Diapolo
Copy link
Copy Markdown
Author

Diapolo commented Mar 17, 2013

@laanwj Indeed, I removed QUrl from paymentserver.cpp by mistake. I hope @BitcoinPullTester is happy now after the last update.

@BitcoinPullTester
Copy link
Copy Markdown

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/32af5266cfc604a32ed90b6099f8b2fb308d6c15 for binaries and test log.

laanwj added a commit that referenced this pull request Mar 18, 2013
Bitcoin-Qt: massive header and cpp cleanup
@laanwj laanwj merged commit 74e4d80 into bitcoin:master Mar 18, 2013
laudney pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request Mar 19, 2014
Bitcoin-Qt: massive header and cpp cleanup
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants