build: Check QT library version #15706

lucayepa · 2019-03-30T14:19:52Z

Due to a bug, the version of the QT library was not checked at configure time for systems using pkg-config. Without any check at configure time, the build process stopped with unexplicative errors on systems with QT versions not supported.

This PR introduces the control of the version of the QT library, with a warning, or an error, at configure time, if the installed version is not supported.

Tested in all the following cases (all combinations):

on Debian Jessie (QT 5.3.2) and on Debian Stretch (QT 5.7.1)
with depends system (QT 5.9.7) enabled and disabled
with --with-gui and --without-gui on configure command line

Since the current 0.18 branch is affected, with compilation errors on some systems, like Debian Jessie (EOL on June 30, 2020), I suggest to include this low impact PR in 0.18.

laanwj · 2019-05-16T18:40:13Z

Concept ACK, can you take a look at the build system changes please @theuni

build-aux/m4/bitcoin_qt.m4

maflcko · 2019-07-18T19:36:27Z

@lucayepa There has been a comment from @laanwj. Mind to address it?

lucayepa · 2019-10-03T22:50:43Z

I tested it on an affected machine.

Are we in time to add back the 0.18.2 tag?

laanwj · 2019-10-05T10:17:28Z

@dongcarl @theuni can one of you take a look here please

build-aux/m4/bitcoin_qt.m4

hebasto · 2019-12-28T21:52:45Z

Concept ACK.

hebasto

Please quote macro arguments with [ and ] consistently.

build-aux/m4/bitcoin_qt.m4

DrahtBot · 2019-12-29T01:28:44Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

build: Make the help string for --with-gui configure option unambiguous #18361 (build: Make the help string for --with-gui configure option unambiguous by hebasto)
build: Fix Qt processing of configure script for depends with DEBUG=1 #18298 (build: Fix Qt processing of configure script for depends with DEBUG=1 by hebasto)
build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows #18297 (build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows by hebasto)

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.

build-aux/m4/bitcoin_qt.m4

jonasschnelli · 2020-01-05T20:58:18Z

Concept ACK
Assigned to @theuni / @dongcarl

Fixes issue bitcoin#15688 Due to a bug, in systems using pkg-config, the version of the Qt library is not checked at configure time. Without any check, when Qt version is not supported, the build process stops with unexpleined errors. This PR introduces the control of the version of the QT library, returning a warning or an error at configure time, when the installed version is not supported.

hebasto

ACK 35c46bc, tested on Linux Mint 19.3 with system Qt 5.9.5:

$ qmake -qt=qt5_system -v
QMake version 3.1
Using Qt version 5.9.5 in /usr/lib/x86_64-linux-gnu

Tests with pkg-config usage

$ ./autogen.sh 
$ ./configure
...
checking for QT5... yes
checking for QT_TEST... yes
checking for QT_DBUS... yes
checking for Qt >= 5.5.1... yes
checking for Qt >= 5.8.0... yes
checking for static Qt... no
checking whether -fPIE can be used with this Qt config... no
checking for moc-qt5... no
checking for moc5... no
checking for moc... /usr/lib/qt5/bin/moc
checking for uic-qt5... no
checking for uic5... no
checking for uic... /usr/lib/qt5/bin/uic
checking for rcc-qt5... no
checking for rcc5... no
checking for rcc... /usr/lib/qt5/bin/rcc
checking for lrelease-qt5... no
checking for lrelease5... no
checking for lrelease... /usr/lib/qt5/bin/lrelease
checking for lupdate-qt5... no
checking for lupdate5... no
checking for lupdate... /usr/lib/qt5/bin/lupdate
checking whether to build Bitcoin Core GUI... yes (Qt5)
...

To test alternative behavior the following patch was applied:

$ git diff
diff --git a/build-aux/m4/bitcoin_qt.m4 b/build-aux/m4/bitcoin_qt.m4
index 019a3ebfb..be20602f0 100644
--- a/build-aux/m4/bitcoin_qt.m4
+++ b/build-aux/m4/bitcoin_qt.m4
@@ -262,7 +262,7 @@ dnl Internal. Check included version of Qt against minimum specified in doc/depe
 dnl Requires: INCLUDES must be populated as necessary.
 dnl Output: bitcoin_cv_qt_minimumrequired=yes|no
 AC_DEFUN([_BITCOIN_QT_CHECK_MINIMUM_REQUIRED],[
-  AC_CACHE_CHECK(for Qt >= 5.5.1, bitcoin_cv_qt_minimumrequired,[
+  AC_CACHE_CHECK(for Qt >= 5.9.6, bitcoin_cv_qt_minimumrequired,[
   AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
       #include <QtCore/qconfig.h>
       #ifndef QT_VERSION
@@ -270,7 +270,7 @@ AC_DEFUN([_BITCOIN_QT_CHECK_MINIMUM_REQUIRED],[
       #endif
     ]],
     [[
-      #if QT_VERSION < 0x050501
+      #if QT_VERSION < 0x050906
       choke
       #endif
     ]])],
@@ -434,8 +434,8 @@ AC_DEFUN([_BITCOIN_QT_FIND_LIBS_WITH_PKGCONFIG],[
       fi
     ])
     BITCOIN_QT_CHECK([
-      AC_CACHE_CHECK(for Qt >= 5.5.1,bitcoin_cv_qt_minimumrequired,
-        PKG_CHECK_EXISTS(Qt5Core < 5.5.1,[bitcoin_cv_qt_minimumrequired=no],[bitcoin_cv_qt_minimumrequired=yes])
+      AC_CACHE_CHECK(for Qt >= 5.9.6,bitcoin_cv_qt_minimumrequired,
+        PKG_CHECK_EXISTS(Qt5Core < 5.9.6,[bitcoin_cv_qt_minimumrequired=no],[bitcoin_cv_qt_minimumrequired=yes])
       )
     ])
     BITCOIN_QT_CHECK([

$ ./autogen.sh 
$ ./configure
checking for QT5... yes
checking for QT_TEST... yes
checking for QT_DBUS... yes
checking for Qt >= 5.9.6... no
checking for Qt >= 5.8.0... yes
configure: WARNING: Qt version not supported; bitcoin-qt frontend will not be built
checking whether to build Bitcoin Core GUI... no
...

$ ./autogen.sh 
$ ./configure --with-gui=qt5
...
checking for QT5... yes
checking for QT_TEST... yes
checking for QT_DBUS... yes
checking for Qt >= 5.9.6... no
checking for Qt >= 5.8.0... yes
configure: error: Qt version not supported

Tests without pkg-config usage

For these tests the following patch was applied:

$ git diff configure.ac
diff --git a/configure.ac b/configure.ac
index 18f3104ac..d6c8745c7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -479,7 +479,7 @@ AC_ARG_WITH([daemon],
   [build_bitcoind=$withval],
   [build_bitcoind=yes])

-use_pkgconfig=yes
+use_pkgconfig=no
 case $host in
   *mingw*)

and --with-qt-incdir command-line option was passed to configure script.

$ ./autogen.sh
$ ./configure --with-qt-incdir=/usr/include/x86_64-linux-gnu/qt5
...
checking for Qt >= 5.5.1... yes
checking for >= Qt 5.8... yes
...
checking whether to build Bitcoin Core GUI... yes (Qt5)
...

$ ./autogen.sh
$ ./configure --with-qt-incdir=/usr/include/x86_64-linux-gnu/qt5
...
checking for Qt >= 5.9.6... no
checking for >= Qt 5.8... yes
...
configure: WARNING: Qt version not supported; bitcoin-qt frontend will not be built
checking whether to build Bitcoin Core GUI... no
...

hebasto · 2020-01-26T09:09:32Z

build-aux/m4/bitcoin_qt.m4

@@ -434,14 +433,25 @@ AC_DEFUN([_BITCOIN_QT_FIND_LIBS_WITH_PKGCONFIG],[
        PKG_CHECK_MODULES([QT_DBUS], [${QT_LIB_PREFIX}DBus], [QT_DBUS_INCLUDES="$QT_DBUS_CFLAGS"; have_qt_dbus=yes], [have_qt_dbus=no])
      fi
    ])
+    BITCOIN_QT_CHECK([
+      AC_CACHE_CHECK(for Qt >= 5.5.1,bitcoin_cv_qt_minimumrequired,
+        PKG_CHECK_EXISTS(Qt5Core < 5.5.1,[bitcoin_cv_qt_minimumrequired=no],[bitcoin_cv_qt_minimumrequired=yes])


non-blocking nit, for consistency with "for Qt >= 5.5.1":

Suggested change

PKG_CHECK_EXISTS(Qt5Core < 5.5.1,[bitcoin_cv_qt_minimumrequired=no],[bitcoin_cv_qt_minimumrequired=yes])

PKG_CHECK_EXISTS(Qt5Core >= 5.5.1,[bitcoin_cv_qt_minimumrequired=yes],[bitcoin_cv_qt_minimumrequired=no])

hebasto · 2020-01-26T09:12:33Z

build-aux/m4/bitcoin_qt.m4

+    ])
+    BITCOIN_QT_CHECK([
+      AC_CACHE_CHECK(for Qt >= 5.8.0,bitcoin_cv_qt58,
+        PKG_CHECK_EXISTS(Qt5Core < 5.8.0,[bitcoin_cv_qt58=no],[bitcoin_cv_qt58=yes])


non-blocking nit, for consistency with "for Qt >= 5.8.0":

Suggested change

PKG_CHECK_EXISTS(Qt5Core < 5.8.0,[bitcoin_cv_qt58=no],[bitcoin_cv_qt58=yes])

PKG_CHECK_EXISTS(Qt5Core >= 5.8.0,[bitcoin_cv_qt58=yes],[bitcoin_cv_qt58=no])

hebasto · 2020-01-26T09:16:41Z

build-aux/m4/bitcoin_qt.m4

-  AC_CACHE_CHECK(for Qt 5, bitcoin_cv_qt5,[
+dnl Output: bitcoin_cv_qt_minimumrequired=yes|no
+AC_DEFUN([_BITCOIN_QT_CHECK_MINIMUM_REQUIRED],[
+  AC_CACHE_CHECK(for Qt >= 5.5.1, bitcoin_cv_qt_minimumrequired,[


non-blocking nit: here and in other places mind quoting the first argument:

Suggested change

AC_CACHE_CHECK(for Qt >= 5.5.1, bitcoin_cv_qt_minimumrequired,[

AC_CACHE_CHECK([for Qt >= 5.5.1], bitcoin_cv_qt_minimumrequired,[

?

hebasto · 2020-01-26T09:17:37Z

build-aux/m4/bitcoin_qt.m4

 dnl Requires: INCLUDES must be populated as necessary.
 dnl Output: bitcoin_cv_qt58=yes|no
 AC_DEFUN([_BITCOIN_QT_CHECK_QT58],[
-  AC_CACHE_CHECK(for > Qt 5.7, bitcoin_cv_qt58,[
+  AC_CACHE_CHECK(for >= Qt 5.8, bitcoin_cv_qt58,[


Suggested change

AC_CACHE_CHECK(for >= Qt 5.8, bitcoin_cv_qt58,[

AC_CACHE_CHECK([for >= Qt 5.8], bitcoin_cv_qt58,[

hebasto · 2020-01-26T09:18:45Z

build-aux/m4/bitcoin_qt.m4

@@ -434,14 +433,25 @@ AC_DEFUN([_BITCOIN_QT_FIND_LIBS_WITH_PKGCONFIG],[
        PKG_CHECK_MODULES([QT_DBUS], [${QT_LIB_PREFIX}DBus], [QT_DBUS_INCLUDES="$QT_DBUS_CFLAGS"; have_qt_dbus=yes], [have_qt_dbus=no])
      fi
    ])
+    BITCOIN_QT_CHECK([
+      AC_CACHE_CHECK(for Qt >= 5.5.1,bitcoin_cv_qt_minimumrequired,


Suggested change

AC_CACHE_CHECK(for Qt >= 5.5.1,bitcoin_cv_qt_minimumrequired,

AC_CACHE_CHECK([for Qt >= 5.5.1],bitcoin_cv_qt_minimumrequired,

hebasto · 2020-01-26T09:20:30Z

build-aux/m4/bitcoin_qt.m4

+      )
+    ])
+    BITCOIN_QT_CHECK([
+      AC_CACHE_CHECK(for Qt >= 5.8.0,bitcoin_cv_qt58,


Suggested change

AC_CACHE_CHECK(for Qt >= 5.8.0,bitcoin_cv_qt58,

AC_CACHE_CHECK([for Qt >= 5.8.0],bitcoin_cv_qt58,

hebasto · 2020-03-17T10:30:27Z

It seems we should consider an alternative solution based on #18297.

fanquake · 2020-05-15T07:26:16Z

I agree that something like #18297 is the way forward here, so I'm going to close. This also would not be getting backported to 0.18 or 0.19 at this point. Thanks for the contribution & sorry it didn't make it into the repo.

4af4672 build, qt: Add Qt version checking (Hennadii Stepanov) 30e336f build: Drop unused bitcoin_cv_qt58 (Hennadii Stepanov) Pull request description: Now `configure` script checks that Qt version is not less then minimum required (currently [5.5.1](#15393)). This PR is an alternative to #15706 (see #15706 (comment)). Closes #15688. The first commit removes dead code (see #18297 (comment)). ACKs for top commit: fanquake: ACK 4af4672 - this looks ok. I've tested this with Qt 5.15.0 and Qt 5.7.1 system libs, as well as 5.9.8 from depends. Tree-SHA512: 8e3b82fa3a98926814923331038185633fabad962c271f31bd158e1ab293dcde52ab1dbf997745540a9ed27e16835cf5b5f3701d405876d877fa561eb03cc619

4af4672 build, qt: Add Qt version checking (Hennadii Stepanov) 30e336f build: Drop unused bitcoin_cv_qt58 (Hennadii Stepanov) Pull request description: Now `configure` script checks that Qt version is not less then minimum required (currently [5.5.1](bitcoin#15393)). This PR is an alternative to bitcoin#15706 (see bitcoin#15706 (comment)). Closes bitcoin#15688. The first commit removes dead code (see bitcoin#18297 (comment)). ACKs for top commit: fanquake: ACK 4af4672 - this looks ok. I've tested this with Qt 5.15.0 and Qt 5.7.1 system libs, as well as 5.9.8 from depends. Tree-SHA512: 8e3b82fa3a98926814923331038185633fabad962c271f31bd158e1ab293dcde52ab1dbf997745540a9ed27e16835cf5b5f3701d405876d877fa561eb03cc619

fanquake added the Build system label Mar 30, 2019

maflcko changed the title ~~Check QT library version~~ build: Check QT library version Mar 30, 2019

maflcko added the Needs gitian build label Mar 30, 2019

maflcko added this to the 0.18.1 milestone Mar 30, 2019

maflcko added the Needs backport label Mar 30, 2019

DrahtBot removed the Needs gitian build label Mar 31, 2019

fanquake requested a review from theuni April 16, 2019 05:07

laanwj reviewed Jun 6, 2019

View reviewed changes

build-aux/m4/bitcoin_qt.m4 Show resolved Hide resolved

bitcoin deleted a comment from DrahtBot Jun 16, 2019

maflcko added the Needs gitian build label Jun 16, 2019

laanwj added the Waiting for author label Jun 17, 2019

DrahtBot removed the Needs gitian build label Jun 18, 2019

bitcoin deleted a comment from DrahtBot Jun 18, 2019

maflcko removed this from the 0.18.1 milestone Jul 18, 2019

laanwj added this to the 0.18.2 milestone Jul 18, 2019

bitcoin deleted a comment from DrahtBot Jul 18, 2019

fanquake removed the Waiting for author label Sep 19, 2019

fanquake requested a review from dongcarl September 19, 2019 01:37

maflcko added Waiting for author and removed Needs backport labels Sep 30, 2019

maflcko removed this from the 0.18.2 milestone Sep 30, 2019

lucayepa force-pushed the check-qt-version branch from 9d74d35 to 25e43fc Compare October 3, 2019 21:54

maflcko added Needs gitian build and removed Waiting for author labels Oct 3, 2019

laanwj added Needs backport (0.19) labels Oct 4, 2019

laanwj added this to the 0.18.2 milestone Oct 4, 2019

lucayepa mentioned this pull request Oct 4, 2019

[0.19] Backport of #15706 #17055

Closed

luke-jr suggested changes Nov 4, 2019

View reviewed changes

build-aux/m4/bitcoin_qt.m4 Show resolved Hide resolved

hebasto requested changes Dec 28, 2019

View reviewed changes

build-aux/m4/bitcoin_qt.m4 Outdated Show resolved Hide resolved

build-aux/m4/bitcoin_qt.m4 Outdated Show resolved Hide resolved

build-aux/m4/bitcoin_qt.m4 Outdated Show resolved Hide resolved

hebasto reviewed Dec 29, 2019

View reviewed changes

build-aux/m4/bitcoin_qt.m4 Outdated Show resolved Hide resolved

build-aux/m4/bitcoin_qt.m4 Show resolved Hide resolved

jonasschnelli assigned theuni and dongcarl Jan 5, 2020

lucayepa force-pushed the check-qt-version branch from 25e43fc to 35c46bc Compare January 24, 2020 23:55

hebasto approved these changes Jan 26, 2020

View reviewed changes

laanwj removed this from the 0.18.2 milestone Jan 27, 2020

DrahtBot mentioned this pull request Feb 11, 2020

build: bitcoin_qt.m4 fixes and improvements #17820

Closed

This was referenced Mar 9, 2020

build: Use pkg-config in BITCOIN_QT_CONFIGURE for all hosts including Windows #18297

Merged

build: Fix Qt processing of configure script for depends with DEBUG=1 #18298

Merged

build: Require pkg-config for all of the hosts #18307

Merged

DrahtBot mentioned this pull request Mar 16, 2020

build: Make the help string for --with-gui configure option unambiguous #18361

Closed

fanquake closed this May 15, 2020

fanquake removed Needs backport (0.18) labels May 15, 2020

hebasto mentioned this pull request Aug 9, 2020

build: Add Qt version checking #19689

Merged

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Check QT library version #15706

build: Check QT library version #15706

lucayepa commented Mar 30, 2019

laanwj commented May 16, 2019

maflcko commented Jul 18, 2019

lucayepa commented Oct 3, 2019

laanwj commented Oct 5, 2019

hebasto commented Dec 28, 2019

hebasto left a comment

DrahtBot commented Dec 29, 2019 •

edited

Loading

jonasschnelli commented Jan 5, 2020

hebasto left a comment

hebasto Jan 26, 2020

hebasto Jan 26, 2020

hebasto Jan 26, 2020

hebasto Jan 26, 2020

hebasto Jan 26, 2020

hebasto Jan 26, 2020

hebasto commented Mar 17, 2020

fanquake commented May 15, 2020

	PKG_CHECK_EXISTS(Qt5Core < 5.5.1,[bitcoin_cv_qt_minimumrequired=no],[bitcoin_cv_qt_minimumrequired=yes])
	PKG_CHECK_EXISTS(Qt5Core >= 5.5.1,[bitcoin_cv_qt_minimumrequired=yes],[bitcoin_cv_qt_minimumrequired=no])

	PKG_CHECK_EXISTS(Qt5Core < 5.8.0,[bitcoin_cv_qt58=no],[bitcoin_cv_qt58=yes])
	PKG_CHECK_EXISTS(Qt5Core >= 5.8.0,[bitcoin_cv_qt58=yes],[bitcoin_cv_qt58=no])

	AC_CACHE_CHECK(for Qt >= 5.5.1, bitcoin_cv_qt_minimumrequired,[
	AC_CACHE_CHECK([for Qt >= 5.5.1], bitcoin_cv_qt_minimumrequired,[

	AC_CACHE_CHECK(for >= Qt 5.8, bitcoin_cv_qt58,[
	AC_CACHE_CHECK([for >= Qt 5.8], bitcoin_cv_qt58,[

	AC_CACHE_CHECK(for Qt >= 5.5.1,bitcoin_cv_qt_minimumrequired,
	AC_CACHE_CHECK([for Qt >= 5.5.1],bitcoin_cv_qt_minimumrequired,

	AC_CACHE_CHECK(for Qt >= 5.8.0,bitcoin_cv_qt58,
	AC_CACHE_CHECK([for Qt >= 5.8.0],bitcoin_cv_qt58,

build: Check QT library version #15706

build: Check QT library version #15706

Conversation

lucayepa commented Mar 30, 2019

laanwj commented May 16, 2019

maflcko commented Jul 18, 2019

lucayepa commented Oct 3, 2019

laanwj commented Oct 5, 2019

hebasto commented Dec 28, 2019

hebasto left a comment

Choose a reason for hiding this comment

DrahtBot commented Dec 29, 2019 • edited Loading

Conflicts

jonasschnelli commented Jan 5, 2020

hebasto left a comment

Choose a reason for hiding this comment

hebasto Jan 26, 2020

Choose a reason for hiding this comment

hebasto Jan 26, 2020

Choose a reason for hiding this comment

hebasto Jan 26, 2020

Choose a reason for hiding this comment

hebasto Jan 26, 2020

Choose a reason for hiding this comment

hebasto Jan 26, 2020

Choose a reason for hiding this comment

hebasto Jan 26, 2020

Choose a reason for hiding this comment

hebasto commented Mar 17, 2020

fanquake commented May 15, 2020

DrahtBot commented Dec 29, 2019 •

edited

Loading