gui: Drop qt4 support #13458

pull laanwj wants to merge 4 commits into bitcoin:master from laanwj:2018_06_remove_qt4_support changing 30 files +110 −344
  1. laanwj commented at 3:10 pm on June 13, 2018: member

    Implements #8263.

    Qt4.x has been EOL since 2015, and at least Gentoo has, or is going to drop support for it. I wouldn’t be surprised if other Linux distributions follow.

    This removes Qt4 detection from the build system, as well as removes all Qt4 fallbacks from the code. Turns out there’s more than I expected: this is going to make maintenance of the GUI code, as well as adding new features significantly easier.

    (I know there’s still some references left to qt4 in RPM and Debian build script, but I don’t have the knowledge how to fix them)

  2. laanwj added the label GUI on Jun 13, 2018
  3. laanwj added this to the milestone 0.17.0 on Jun 13, 2018
  4. laanwj requested review from theuni on Jun 13, 2018
  5. laanwj renamed this:
    gui: Drop qt5 support
    gui: Drop qt4 support
    on Jun 13, 2018
  6. fanquake commented at 3:18 pm on June 13, 2018: member
    What would this make our minimum supported version of Qt? 5.4.x IIRC?
  7. laanwj commented at 3:22 pm on June 13, 2018: member

    What would this make our minimum supported version of Qt? 5.4.x IIRC?

    That’s a good question. I’m not sure that’s something to be decided here, I don’t change anything with regard to Qt5 versions supported. Though there are a few QT_VERSION checks for Qt 5.2 that would not be relevant in that case..

  8. in build-aux/m4/bitcoin_qt.m4:112 in f00d4d1fea outdated 
    112@@ -113,63 +113,48 @@ AC_DEFUN([BITCOIN_QT_CONFIGURE],[
113   TEMP_CXXFLAGS=$CXXFLAGS
    fanquake commented at 3:43 pm on June 13, 2018:
    Qt4 mentions in the comment here ^, and another starting at line 288.
  9. fanquake commented at 3:45 pm on June 13, 2018: member

    That’s a good question. I’m not sure that’s something to be decided here, I don’t change anything with regard to Qt5 versions supported. Though there are a few QT_VERSION checks for Qt 5.2 that would not be relevant in that case..

    Fair enough.

    Some qt4 related code here: https://github.com/bitcoin/bitcoin/blob/f532d52d39658d0b58d9117a567336cab479dfed/src/qt/coincontroldialog.cpp#L125

    https://github.com/bitcoin/bitcoin/blob/f532d52d39658d0b58d9117a567336cab479dfed/src/qt/test/wallettests.cpp#L90

    There are some qt4 related docs that could be dropped. i.e: https://github.com/bitcoin/bitcoin/blame/master/doc/build-osx.md#L27 https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#dependencies-for-the-gui

  10. build: Build system changes to support only Qt5 bad068ad9f
  11. laanwj force-pushed on Jun 13, 2018
  12. laanwj commented at 4:17 pm on June 13, 2018: member

    Thanks @fanquake, updated for those.

    Edit: I left the // change coin control first column label due Qt4 bug. though, because I’m not sure what to do there otherwise. Looks like the statement can be moved, but I don’t know where.

  13. promag commented at 4:19 pm on June 13, 2018: member
    Concept ACK, will review.
  14. MarcoFalke added the label Build system on Jun 13, 2018
  15. MarcoFalke commented at 4:36 pm on June 13, 2018: member
    ACK. A follow up pull request could clarify which versions of qt5 are supported.
  16. MarcoFalke commented at 6:30 pm on June 13, 2018: member

    Concept ACK ecda9463649d7a5b3d35e31f0c6753f7f4f721c5.

    Nits:

  17. theuni commented at 6:33 pm on June 13, 2018: member
    Concept ACK.
  18. in .travis.yml:39 in ecda946364 outdated 
    34@@ -35,8 +35,8 @@ env:
35     - HOST=i686-pc-linux-gnu PACKAGES="g++-multilib python3-zmq" DEP_OPTS="NO_QT=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports LDFLAGS=-static-libstdc++" CONFIG_SHELL="/bin/dash"
36 # x86_64 Linux (uses qt5 dev package instead of depends Qt to speed up build and avoid timeout)
37     - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools protobuf-compiler libdbus-1-dev libharfbuzz-dev libprotobuf-dev" DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports CPPFLAGS=-DDEBUG_LOCKORDER"
38-# Qt4 & system libs
39-    - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qt4-dev-tools libssl1.0-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-program-options-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev xvfb libqt4-dev" NO_DEPENDS=1 NEED_XVFB=1 RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt4 CPPFLAGS=-DDEBUG_LOCKORDER" DISPLAY=:99.0
40+# x86_64 Linux (Qt5 & system libs)
41+    - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libssl1.0-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-program-options-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev xvfb libqt4-dev" NO_DEPENDS=1 NEED_XVFB=1 RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER" DISPLAY=:99.0
    theuni commented at 6:35 pm on June 13, 2018:

    Missed libqt4-dev.

    Also, I believe this means that we can get rid of xvfb, since qt5’s ‘minimal’ plugin should work.

  19. meshcollider commented at 9:02 pm on June 13, 2018: contributor
    Concept ACK
  20. in src/qt/guiutil.h:236 in ecda946364 outdated 
    232@@ -233,7 +233,7 @@ namespace GUIUtil
233         void mouseReleaseEvent(QMouseEvent *event);
234     };
235 
236-#if defined(Q_OS_MAC) && QT_VERSION >= 0x050000
237+#if defined(Q_OS_MAC)
    fanquake commented at 1:34 am on June 14, 2018:
    Note: Still need to test, but we might be able to remove this Qt workaround entirely.
  21. jonasschnelli commented at 7:10 am on June 14, 2018: contributor
    Concept ACK Gitian Build: https://bitcoin.jonasschnelli.ch/build/656
  22. promag commented at 8:53 am on June 14, 2018: member
    Remove Qt 4 is also supported from bitcoin/src/qt/README.md.
  23. promag commented at 9:00 am on June 14, 2018: member
    All QT_VERSION usages LGTM.
  24. laanwj commented at 11:10 am on June 14, 2018: member
    Ok, I think all comments by @promag @MarcoFalke @theuni have been addressed. @fanquake That’d be good. But maybe in a separate PR, I think this update should be dumbly removing Qt4 support but otherwise keep the logic for Qt5 the same.
  25. in .travis.yml:39 in 2b0ac8ae13 outdated 
    34@@ -35,8 +35,8 @@ env:
35     - HOST=i686-pc-linux-gnu PACKAGES="g++-multilib python3-zmq" DEP_OPTS="NO_QT=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports LDFLAGS=-static-libstdc++" CONFIG_SHELL="/bin/dash"
36 # x86_64 Linux (uses qt5 dev package instead of depends Qt to speed up build and avoid timeout)
37     - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools protobuf-compiler libdbus-1-dev libharfbuzz-dev libprotobuf-dev" DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports CPPFLAGS=-DDEBUG_LOCKORDER"
38-# Qt4 & system libs
39-    - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qt4-dev-tools libssl1.0-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-program-options-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev xvfb libqt4-dev" NO_DEPENDS=1 NEED_XVFB=1 RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt4 CPPFLAGS=-DDEBUG_LOCKORDER" DISPLAY=:99.0
40+# x86_64 Linux (Qt5 & system libs)
41+    - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libssl1.0-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-program-options-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev" NO_DEPENDS=1 NEED_XVFB=1 RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER" DISPLAY=:99.0
    MarcoFalke commented at 4:40 pm on June 14, 2018:
    I think cory mentioned you could get rid of xvfb?
    laanwj commented at 5:17 pm on June 14, 2018:
    Didn’t I? oh, there’s an environment variable too…
    theuni commented at 5:50 pm on June 14, 2018:
    $DISPLAY was also related to xvfb and can be removed.
  26. in .travis.yml:39 in 96076a2a07 outdated 
    34@@ -35,8 +35,8 @@ env:
35     - HOST=i686-pc-linux-gnu PACKAGES="g++-multilib python3-zmq" DEP_OPTS="NO_QT=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports LDFLAGS=-static-libstdc++" CONFIG_SHELL="/bin/dash"
36 # x86_64 Linux (uses qt5 dev package instead of depends Qt to speed up build and avoid timeout)
37     - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools protobuf-compiler libdbus-1-dev libharfbuzz-dev libprotobuf-dev" DEP_OPTS="NO_QT=1 NO_UPNP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1" RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-glibc-back-compat --enable-reduce-exports CPPFLAGS=-DDEBUG_LOCKORDER"
38-# Qt4 & system libs
39-    - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qt4-dev-tools libssl1.0-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-program-options-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev xvfb libqt4-dev" NO_DEPENDS=1 NEED_XVFB=1 RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt4 CPPFLAGS=-DDEBUG_LOCKORDER" DISPLAY=:99.0
40+# x86_64 Linux (Qt5 & system libs)
41+    - HOST=x86_64-unknown-linux-gnu PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libssl1.0-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-program-options-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libprotobuf-dev protobuf-compiler libqrencode-dev" NO_DEPENDS=1 RUN_TESTS=true GOAL="install" BITCOIN_CONFIG="--enable-zmq --with-incompatible-bdb --enable-glibc-back-compat --enable-reduce-exports --with-gui=qt5 CPPFLAGS=-DDEBUG_LOCKORDER" DISPLAY=:99.0
    MarcoFalke commented at 7:43 pm on June 14, 2018:
    Can probably also remove two instances of DISPLAY?
  27. DrahtBot commented at 8:12 pm on June 14, 2018: member
    • #13482 (Remove boost::program_options dependency by ken2812221)
    • #13278 ([qt] Fixed tx-view min amount typing period on locales using comma by GreatSock)
    • #13249 (Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations. by practicalswift)
    • #13235 (Break circular dependency: init -> * -> init by extracting shutdown.h by Empact)
    • #12971 (depends: Upgrade Qt to 5.9.5 by TheCharlatan)
    • #12873 ([ci] Run functional tests using bitcoin-qt in one Travis job by jamesob)
    • #12686 (Add -ftrapv to CFLAGS and CXXFLAGS when –enable-debug is used. Enable -ftrapv in Travis. by practicalswift)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  28. practicalswift commented at 8:36 pm on June 14, 2018: contributor
    Concept ACK
  29. MarcoFalke commented at 2:27 pm on June 15, 2018: member

    @laanwj You wouldn’t need to export DISPLAY to the docker as well? Could remove that as well?

    https://github.com/laanwj/bitcoin/blob/e2dc0e7f02b63ab5990b6f3c806ceaedf40cf3ea/.travis.yml#L48

  30. fanquake commented at 3:43 pm on June 15, 2018: member

    Should be able to drop this check from paymentrequestplus.cpp:

    0#if QT_VERSION >= 0x050000
1        if (qCert.isBlacklisted()) {
2            ...snip...
3            return false;
4        }
5#endif
  31. laanwj commented at 7:55 pm on June 15, 2018: member
    @MarcoFalke @fanquake done this is becoming a palimpsest of commits, if there’s no more things I missed I’m going to squash them into more sensible subdivision
  32. TheBlueMatt commented at 8:09 pm on June 15, 2018: member

    I presume, barring some major rewrite, this implies the PPA should simply not ship with a GUI except on 18.04? (And replace bitcoin-qt with a dummy package instead of letting it get stale?)

    On June 15, 2018 7:56:01 PM UTC, “Wladimir J. van der Laan” notifications@github.com wrote:

    @MarcoFalke @fanquake done this is becoming a palimpsest of commits, if there’s no more things I missed I’m going to squash them into more sensible subdivision

    – You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/bitcoin/bitcoin/pull/13458#issuecomment-397726195

  33. MarcoFalke commented at 8:17 pm on June 15, 2018: member

    @TheBlueMatt I doubt anyone running ubuntu 14.04 is running the gui from the ppa. If there is one, they might as well download the release build.

    Edit: Ah, according to #8043, using the release builds wouldn’t help and xenial is affected as well. Though, since the only problem is the tray icon, we might as well just ship a package with a slight ui glitch instead of no package at all.

  34. fanquake commented at 10:19 am on June 16, 2018: member
  35. laanwj commented at 11:11 am on June 16, 2018: member

    @TheBlueMatt I doubt anyone running ubuntu 14.04 is running the gui from the ppa. If there is one, they might as well download the release build.

    Or a three-line patch to disable tray icon support in the PPA, for those versions. It seems ridiculous to me to remove the entire GUI package just because of a small glitch. (or even just ship with the glitch, as @MarcoFalke says - note it as known issue, people can upgrade to Ubuntu 18.04 if it bothers them)

  36. wumpus commented at 3:13 pm on June 16, 2018: none
    Wrong @wumpus.
  37. gui: Remove QT_VERSION fallbacks for Qt < 5 
    There were surprisingly many `#ifdef` fallbacks for Qt 4.

Remiving them simplifies maintenance, as well as adding new GUI
functionality.
    907f73bbc5
  38. test: Update travis to not test Qt4 anymore 
    Change Qt4 & system libs build to Qt5 & system libs build.
    462c71f71b
  39. doc: Remove mention of Qt4 from build docs af6ac3b677
  40. laanwj force-pushed on Jun 18, 2018
  41. laanwj commented at 10:33 am on June 18, 2018: member
    Squashed into sensible commits: 56c25b36b17b73b4ad36d608c428aaa052130fab → af6ac3b677454644364fd24d0df0c02ac9b8c8db No other changes.
  42. MarcoFalke commented at 11:17 am on June 18, 2018: member
    Concept re-ACK af6ac3b677. (No changes since feedback addressed, but I didn’t review the build changes)
  43. laanwj merged this on Jun 24, 2018
  44. laanwj closed this on Jun 24, 2018
  45. laanwj referenced this in commit dc53f7f251 on Jun 24, 2018
  46. 10xcryptodev referenced this in commit 08c57e4126 on May 14, 2020
  47. 10xcryptodev referenced this in commit 5abc479e04 on May 15, 2020
  48. 10xcryptodev referenced this in commit b4605a4f19 on May 20, 2020
  49. 10xcryptodev referenced this in commit f3799930f9 on Jun 12, 2020
  50. gades referenced this in commit a290a58aa3 on Jun 26, 2021
  51. MarcoFalke locked this on Sep 8, 2021


laanwj fanquake promag MarcoFalke theuni meshcollider jonasschnelli DrahtBot practicalswift TheBlueMatt wumpus

Labels
GUI Build system

Milestone
0.17.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me