Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

depends: Upgrade Qt to 5.9.6 #12971

Merged
merged 3 commits into from Jul 9, 2018

Conversation

Projects
None yet
9 participants
@TheCharlatan
Copy link
Contributor

TheCharlatan commented Apr 13, 2018

With the introduction of Ubuntu 18.04 (Bionic Beaver) modern versions of gcc and mingw that allow cross compilation of versions of Qt greater than 5.8 are now readily available. This pull requests upgrades the Qt depends recipe from Qt 5.7.1 to Qt 5.9.6. Qt 5.9.x is the current LTS version and should be supported by Qt until 2020.

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Apr 13, 2018

According to https://doc.qt.io/qt-5.10/supported-platforms-and-configurations.html#qt-5-9 gcc4.8 is supported. Also, I could build your receipt on ubuntu trusty, which comes with gcc4.8. I think you can get rid of the optional flag to compile qt5.9?

Edit: I didn't try windows or mac cross builds, so those might be causing issues ... ?

@fanquake

This comment has been minimized.

Copy link
Member

fanquake commented Apr 13, 2018

The plan is to just move depends to Qt 5.9+ quite soon, I have been delayed doing that work.
I don't think we want to want to have support for 5.7 and 5.9 in depends at the same time.
Would you be interested in turning this work into a Qt 5.7 -> 5.9 upgrade?

@fanquake fanquake requested a review from theuni Apr 13, 2018

@TheCharlatan

This comment has been minimized.

Copy link
Contributor Author

TheCharlatan commented Apr 13, 2018

@MarcoFalke The compiler version for linux native compilation is not a problem. It is though as soon as you are trying to cross compile to windows (which requires Mingw5.3).
I originally did not want to open the pr for this reason, since it would mean that the current travis tests would be broken because of the windows build requirement. I heard though that @theuni is working on getting the entire toolchain on depends, so I decided to publish the patch nevertheless to prevent duplicate work and enable other developers to already compile bitcoin static with a modern Qt version.

The macos build needs some further investigation. It fails upon creating qmake and the current qmake patch does not seem to work. Upon some investigation, I saw that multiple people had to do some manual property setting on xcode to make the build work. I am not sure if this is related though.

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Apr 13, 2018

cross compile to windows (which requires Mingw5.3)

Hmm. Imo we could drop support for cross-compile to windows on trusty and just require xenial or above.

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Apr 15, 2018

Could do a rebase since #12946 is merged?

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch to f870067 Apr 15, 2018

@TheCharlatan

This comment has been minimized.

Copy link
Contributor Author

TheCharlatan commented Apr 15, 2018

Rebased.

@fanquake

This comment has been minimized.

Copy link
Member

fanquake commented Apr 16, 2018

@TheCharlatan Thanks for the work so far, however did you see my comment above? I think this would be better as a straight 5.7 -> 5.9 upgrade, instead of adding an additional version of Qt to depends. We would like to use Qt 5.9+ to build the next release anyways.

Regardless of the that this needs a few fixes.

There are currently whitespace linting errors.

A linux Travis build is currently failing:
linux-raw-log.txt

configure: error: Package requirements (Qt5PlatformSupport) were not met:
No package 'Qt5PlatformSupport' found
Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.
Alternatively, you may set the environment variables QTPLATFORM_CFLAGS
and QTPLATFORM_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.
This file contains any messages produced by compilers while
running configure, to aid debugging if configure makes a mistake.
It was created by Bitcoin Core configure 0.16.99, which was
generated by GNU Autoconf 2.69.  Invocation command line was
  $ ../configure --cache-file=config.cache --disable-dependency-tracking --prefix=/home/travis/build/bitcoin/bitcoin/depends/x86_64-unknown-linux-gnu --bindir=/home/travis/build/bitcoin/bitcoin/out/12971/27461.7-x86_64-unknown-linux-gnu/bin --libdir=/home/travis/build/bitcoin/bitcoin/out/12971/27461.7-x86_64-unknown-linux-gnu/lib --enable-glibc-back-compat --enable-reduce-exports

The macOS build would also need to be fixed/updated:
macOS-raw-log.txt

Configuring qt59...
Creating qmake...
make[1]: Entering directory `/home/travis/build/bitcoin/bitcoin/depends/work/build/x86_64-apple-darwin11/qt59/5.9.4-01715d30425/qtbase/qmake'
make[1]: Leaving directory `/home/travis/build/bitcoin/bitcoin/depends/work/build/x86_64-apple-darwin11/qt59/5.9.4-01715d30425/qtbase/qmake'
Info: creating cache file /home/travis/build/bitcoin/bitcoin/depends/work/build/x86_64-apple-darwin11/qt59/5.9.4-01715d30425/qtbase/.qmake.cache
Command line: -bindir /home/travis/build/bitcoin/bitcoin/depends/x86_64-apple-darwin11/native/bin -c++std c++11 -confirm-license -dbus-runtime -hostprefix /home/travis/build/bitcoin/bitcoin/depends/x86_64-apple-darwin11/native -no-cups -no-egl -no-eglfs -no-freetype -no-gif -no-glib -no-icu -no-iconv -no-kms -no-linuxfb -no-libudev -no-mtdev -no-openvg -no-reduce-relocations -no-qml-debug -no-sql-db2 -no-sql-ibase -no-sql-oci -no-sql-tds -no-sql-mysql -no-sql-odbc -no-sql-psql -no-sql-sqlite -no-sql-sqlite2 -no-use-gold-linker -nomake examples -nomake tests -opensource -openssl-linked -optimized-qmake -pch -pkg-config -prefix /home/travis/build/bitcoin/bitcoin/depends/x86_64-apple-darwin11 -qt-libpng -qt-libjpeg -qt-pcre -system-zlib -static -silent -v -release -xplatform macx-clang-linux -device-option MAC_SDK_PATH=/home/travis/build/bitcoin/bitcoin/depends/SDKs/MacOSX10.11.sdk -device-option MAC_SDK_VERSION=10.11 -device-option CROSS_COMPILE=x86_64-apple-darwin11- -device-option MAC_MIN_VERSION=10.8 -device-option MAC_TARGET=x86_64-apple-darwin11 -device-option MAC_LD64_VERSION=253.9
Project ERROR: Could not resolve SDK --show-sdk-path for 'macosx'
make: *** [/home/travis/build/bitcoin/bitcoin/depends/work/build/x86_64-apple-darwin11/qt59/5.9.4-01715d30425/qtbase/.stamp_configured] Error 3
make: Leaving directory `/home/travis/build/bitcoin/bitcoin/depends'
qt_i686_linux_packages:=$(qt_x86_64_linux_packages)

qt_darwin_packages=qt

This comment has been minimized.

@fanquake

fanquake Apr 16, 2018

Member

This looks like it's building QT 5.9 in either case?

@TheCharlatan

This comment has been minimized.

Copy link
Contributor Author

TheCharlatan commented Apr 16, 2018

@fanquake thanks for the review.
If I were to change this to a straight upgrade, how would travis, or gitian windows builds be done?

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Apr 16, 2018

Our gitian descriptors will be switched to bionic soon: #12511. Similarly, we can switch travis to xenial. Those will happen in separate pull request, so don't worry about them.

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented May 14, 2018

Travis fails on linux 64 bit with:

checking for QTPLATFORM... no
configure: error: Package requirements (Qt5PlatformSupport) were not met:
No package 'Qt5PlatformSupport' found

And OSX:

Project ERROR: Could not resolve SDK --show-sdk-path for 'macosx'
@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented May 14, 2018

See also #13215

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch from f870067 May 16, 2018

@TheCharlatan

This comment has been minimized.

Copy link
Contributor Author

TheCharlatan commented May 16, 2018

Removed the optional flags, Qt5.9.4 is now the new default. The build still fails on MacOS.

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch May 16, 2018

@fanquake

This comment has been minimized.

Copy link
Member

fanquake commented May 16, 2018

@TheCharlatan Can you fixup any trailing whitespace so that Travis will run: https://travis-ci.org/bitcoin/bitcoin/jobs/379496754

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch May 16, 2018

depends/README.md Outdated
@@ -12,6 +12,10 @@ For example:

make HOST=x86_64-w64-mingw32 -j4

To build the more recent Qt5.9.4, you can build depends with

make HOST=host-platform-triple QT_59=1

This comment has been minimized.

@MarcoFalke

MarcoFalke May 16, 2018

Member

Can remove this hunk and all other references to qt 5.9?

depends/packages/qt.mk Outdated
@@ -75,12 +64,10 @@ $(package)_config_opts += -qt-libpng
$(package)_config_opts += -qt-libjpeg
$(package)_config_opts += -qt-pcre
$(package)_config_opts += -system-zlib
$(package)_config_opts += -reduce-exports
#$(package)_config_opts += -reduce-exports

This comment has been minimized.

@MarcoFalke

MarcoFalke May 17, 2018

Member

nit: Either remove or replace with proper comment/reasoning.

doc/build-windows.md Outdated
./autogen.sh # not required when building from tarball
CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site ./configure --prefix=/ --enable-qt59
make

This comment has been minimized.

@MarcoFalke

MarcoFalke May 17, 2018

Member

This whole section is added in #13246, so feel free to remove this to avoid merge conflicts.

@ken2812221

This comment has been minimized.

Copy link
Member

ken2812221 commented May 17, 2018

This should fix the problem. Patch from QTBUG-67286
[removed]

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch May 17, 2018

@ken2812221

This comment has been minimized.

Copy link
Member

ken2812221 commented May 18, 2018

This additional patch should fix all Mac and Windows build:
[removed]

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch May 18, 2018

@ken2812221

This comment has been minimized.

Copy link
Member

ken2812221 commented May 18, 2018

You committed the wrong patch, it's not fix-cocoahelpers-macos.patch but fix_no_printer.patch

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch 2 times, most recently May 18, 2018

@DrahtBot

This comment has been minimized.

Copy link
Contributor

DrahtBot commented Jun 24, 2018

Needs rebase

@laanwj laanwj added this to the 0.17.0 milestone Jul 5, 2018

TheCharlatan and others added some commits May 19, 2018

Ugrade Qt depends to Qt5.9.4
Depends can now be built with Qt5.9.4 , which is Qt's new long term
support version.
Fix depends Qt5.9.4 mac build
Apply patch from QTBUG-67286

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch 2 times, most recently to 5e9d4fe Jul 5, 2018

@TheCharlatan

This comment has been minimized.

Copy link
Contributor Author

TheCharlatan commented Jul 5, 2018

Rebased.

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jul 5, 2018

utACK 5e9d4fe

@theuni

This comment has been minimized.

Copy link
Member

theuni commented Jul 5, 2018

Thanks for the rebase, taking another look.

@ken2812221

This comment has been minimized.

Copy link
Member

ken2812221 commented Jul 5, 2018

utACK 5e9d4fe

@fanquake

This comment has been minimized.

Copy link
Member

fanquake commented Jul 5, 2018

Thanks @TheCharlatan:

Can this be updated to Qt 5.9.6, which contains further bugfixes from 5.9.5.

Once that's done the qfixed-coretext.patch can be dropped as it should have been fixed in QTBUG-67545.

Also, can you update the PR description now that this is a straight Qt upgrade, rather than adding an additional version.

@TheCharlatan TheCharlatan force-pushed the TheCharlatan:Qt59 branch from 5e9d4fe Jul 6, 2018

@TheCharlatan TheCharlatan changed the title depends: Upgrade Qt to 5.9.5 depends: Upgrade Qt to 5.9.6 Jul 6, 2018

@TheCharlatan

This comment has been minimized.

Copy link
Contributor Author

TheCharlatan commented Jul 6, 2018

Force-pushed to bump the version to Qt 5.9.6, as suggested by @fanquake .

@fanquake fanquake removed the Needs rebase label Jul 6, 2018

@Sjors

Sjors approved these changes Jul 6, 2018

Copy link
Member

Sjors left a comment

Tested 77fd4f8 on macOS 10.13.5 (did a make clean-all first). Also ran the tests.

@fanquake wrote:

this will likely bump the minimum required macOS version to 10.10. Which probably requires further discussion.

macOS 10.10 Yosemite indeed: https://doc-snapshots.qt.io/qt5-5.9/osx.html

See #13362 for discussion.

build-aux/m4/bitcoin_qt.m4 Outdated


dnl Internal. Check if the linked version of Qt was built as static libs.
dnl Requires: Qt5. This check cannot determine if Qt4 is static.

This comment has been minimized.

@Sjors

Sjors Jul 6, 2018

Member

Nit: no need to reference Qt4 anymore

depends/packages/qt.mk Outdated
$(package)_file_name=qtbase-$($(package)_suffix)
$(package)_sha256_hash=95f83e532d23b3ddbde7973f380ecae1bac13230340557276f75f2e37984e410
$(package)_sha256_hash=eed620cb268b199bd83b3fc6a471c51d51e1dc2dbb5374fc97a0cc75facbe36f

This comment has been minimized.

@Sjors

Sjors Jul 6, 2018

Member

Can confirm eed620cb268b199bd83b3fc6a471c51d51e1dc2dbb5374fc97a0cc75facbe36f matches https://download.qt.io/archive/qt/5.9/5.9.6/submodules/qtbase-opensource-src-5.9.6.tar.xz.mirrorlist

depends/packages/qt.mk Outdated
$(package)_download_path=https://download.qt.io/archive/qt/5.7/$($(package)_version)/submodules
$(package)_suffix=opensource-src-$($(package)_version).tar.gz
$(package)_version=5.9.6
$(package)_download_path=http://download.qt.io/official_releases/qt/5.9/$($(package)_version)/submodules

This comment has been minimized.

@Sjors

Sjors Jul 6, 2018

Member

Why the http downgrade?

This comment has been minimized.

@TheCharlatan

TheCharlatan Jul 6, 2018

Author Contributor

Thanks, missed this during rebase.

@@ -29,25 +27,18 @@ $(package)_config_opts += -c++std c++11
$(package)_config_opts += -confirm-license
$(package)_config_opts += -dbus-runtime
$(package)_config_opts += -hostprefix $(build_prefix)
$(package)_config_opts += -no-alsa

This comment has been minimized.

@Sjors

Sjors Jul 6, 2018

Member

Is the reason we can drop these lines that these options been removed from QT?

This comment has been minimized.

@TheCharlatan

TheCharlatan Jul 6, 2018

Author Contributor

The config_opts lines I removed are no longer supported by qt's configure.

$(package)_config_opts_linux += -fontconfig
$(package)_config_opts_linux += -no-opengl
$(package)_config_opts_arm_linux = -platform linux-g++ -xplatform $(host)
$(package)_config_opts_i686_linux = -xplatform linux-g++-32
$(package)_config_opts_x86_64_linux = -xplatform linux-g++-64

This comment has been minimized.

@Sjors

Sjors Jul 6, 2018

Member

I can try building these depends on an arm64 device soon.

This comment has been minimized.

@Sjors

Sjors Jul 12, 2018

Member

cd depends && make doesn't build QT when run from a aarch64 host, so nvm :-)

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Jul 9, 2018

utACK 800dea8

@laanwj laanwj merged commit 800dea8 into bitcoin:master Jul 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jul 9, 2018

Merge #12971: depends: Upgrade Qt to 5.9.6
800dea8 Upgrade Qt depends to 5.9.6 (Sebastian Kung)
70afa65 Fix depends Qt5.9.4 mac build (Ken Lee)
28482ef Ugrade Qt depends to Qt5.9.4 (Sebastian Kung)

Pull request description:

  With the introduction of Ubuntu 18.04 (Bionic Beaver) modern versions of gcc and mingw that allow cross compilation of versions of Qt greater than 5.8 are now readily available. This pull requests upgrades the Qt depends recipe from Qt 5.7.1 to Qt 5.9.6. Qt 5.9.x is the current LTS version and should be supported by Qt until 2020.

Tree-SHA512: 439295d594ff8954a5ba5e348a0452713721c805485be2edcb9f8603ee59e96db5a61e1c684bdff36bbfd643a79cd35c289817257af88f489d2890e7843460bf

MarcoFalke added a commit that referenced this pull request Oct 26, 2018

Merge #13515: travis: Enable qt for all jobs
3387bb0 travis: avoid timeout without saving caches, also enable all qt (Chun Kuan Lee)

Pull request description:

  - If depends build take more than 20 mins, skip Bitcoin Core build to store depends caches and mark it fail. Then restart the job for Bitcoin Core build.
  - Enable Qt build for Windows and 32-bit Linux
  - Enable wallet for depends x86-64 Linux
  - Disable gui tests for Windows since they are not supported

  This would be helpful for upgrading Qt (#12971) and protobuf (#13513)

Tree-SHA512: e943cbd848d90f9f70e29c94ed717f96ad2c2d27b433bafea762015756a2d2794fc28976c54aee087bf0f3726ac2c9140920272445a902038719b956e2160cf9

eabz added a commit to polispay/polis that referenced this pull request Jan 26, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.