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: qt 5.9.7 #14849

Merged
merged 3 commits into from Dec 13, 2018
Merged

depends: qt 5.9.7 #14849

merged 3 commits into from Dec 13, 2018

Conversation

@fanquake
Copy link
Member

@fanquake fanquake commented Nov 30, 2018

This PR upgrades expat and qt in depends. The intention is to upgrade Qt in master to the latest point release of the current Qt LTS. This change can then be back-ported to the 0.17 branch (wether it makes it into 0.17.1 or not).

Then, sometime before the 0.18.0 release, we could move to using Qt 5.12+ in depends (which is also LTS). That discussion, as well as minimum supported Qt versions is in #13478.

Qt 5.9.7

Release announcement
Changelog

Expat 2.2.6

  • Avoid doing arithmetic with NULL pointers in XML_GetBuffer
  • Fix 2.2.5 regression with suspend-resume while parsing a document like

Full changelog here

a46c847 disables a bunch of qt features we aren't currently using. This speeds up the qt depends build slightly (also decreases the size of the built qt-5.9.7 tar by about 2%). The disabling is somewhat unintuitive, hence [wip] until after a travis run and gitian build.

@hebasto
Copy link
Member

@hebasto hebasto commented Nov 30, 2018

I've look through ./configure --help and not sure if these options:
-no-egl
-no-libudev
-no-openvg
-optimized-qmake
are acceptable by Qt 5.9.7 configure.

$(package)_config_opts += -no-feature-printer
$(package)_config_opts += -no-feature-printdialog
$(package)_config_opts += -no-feature-concurrent
$(package)_config_opts += -no-feature-sql

This comment has been minimized.

@hebasto

hebasto Nov 30, 2018
Member

./configure --list-features output does not list sql feature.

This comment has been minimized.

@fanquake

fanquake Nov 30, 2018
Author Member

Passing -no-feature-sql disables the Qt sql module in its entirety, see the output in the configure summary:

Qt modules and options:
  Qt Network ............................. yes
  Qt Sql ................................. no
  Qt Testlib ............................. yes
@hebasto
Copy link
Member

@hebasto hebasto commented Nov 30, 2018

Some thoughts about disabled Qt features.

  1. Qt build does not include QtPrintSupport module; therefore, it is redundant to mention:
$(package)_config_opts += -no-feature-printer
$(package)_config_opts += -no-feature-printdialog
  1. Qt build does not include QtConcurrent module; therefore, it is redundant to mention:
$(package)_config_opts += -no-feature-concurrent
  1. Also these features can be disabled:
$(package)_config_opts += -no-feature-udpsocket
$(package)_config_opts += -no-feature-syntaxhighlighter

@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Nov 30, 2018

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #14066 (gitian-linux: Build binaries for 64-bit POWER by luke-jr)

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.

@fanquake fanquake force-pushed the fanquake:qt-5-9-7 branch Nov 30, 2018
@fanquake
Copy link
Member Author

@fanquake fanquake commented Nov 30, 2018

@hebasto Thanks for your thoughts:

$(package)_config_opts += -no-feature-printer
$(package)_config_opts += -no-feature-printdialog

These were specifically introduced to fix the depends build on macOS. I'd rather leave them in place for now. Similar thoughts for -no-egl -no-libudev -no-openvg -optimized-qmake. Maybe once we move to Qt 6 we can review the configure flags we are using. However for now I'd rather only remove extra unused features/reduce compile times etc, than inadvertently break any depends/gitian builds by removing a potential unused flag.

$(package)_config_opts += -no-feature-concurrent

We can leave this module disabled, and not do any extra compilation/work during the Qt compile. i.e

Qt modules and options:
  Qt Concurrent .......................... no

$(package)_config_opts += -no-feature-udpsocket
$(package)_config_opts += -no-feature-syntaxhighlighter

I've added these + -no-feature-statemachine to the PR.

@hebasto
Copy link
Member

@hebasto hebasto commented Dec 1, 2018

@fanquake You are right. Qt docs confused me a little :)
utACK 311498de728bd584d51865aa62e06091235baab5

@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Dec 1, 2018

Gitian builds for commit 60b20c8 (master):

Gitian builds for commit 74fb02cb057ddb5016843662f3206e41049e4cc7 (master and this pull):

@jonasschnelli
Copy link
Member

@jonasschnelli jonasschnelli commented Dec 10, 2018

Successful gitian build:
https://bitcoin.jonasschnelli.ch/build/907

Tested on OSX:
bildschirmfoto 2018-12-09 um 13 55 17

@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Dec 12, 2018

Needs rebase
@fanquake fanquake force-pushed the fanquake:qt-5-9-7 branch to a46c847 Dec 12, 2018
@fanquake fanquake changed the title [wip] depends: qt 5.9.7 depends: qt 5.9.7 Dec 12, 2018
@fanquake
Copy link
Member Author

@fanquake fanquake commented Dec 12, 2018

Rebased on master and removed [wip].

@laanwj
Copy link
Member

@laanwj laanwj commented Dec 13, 2018

utACK a46c847

@laanwj laanwj merged commit a46c847 into bitcoin:master Dec 13, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Dec 13, 2018
a46c847 depends: disable unused qt features (fanquake)
73b46ee depends: qt 5.9.7 (fanquake)
095e765 depends: expat 2.2.6 (fanquake)

Pull request description:

  This PR upgrades `expat` and `qt` in depends. The intention is to upgrade Qt in master to the latest point release of the current Qt LTS. This change can then be back-ported to the 0.17 branch (wether it makes it into 0.17.1 or not).

  Then, sometime before the 0.18.0 release, we could move to using Qt 5.12+ in depends (which is also LTS). That discussion, as well as minimum supported Qt versions is in #13478.

  ### Qt 5.9.7
  [Release announcement](https://blog.qt.io/blog/2018/10/23/qt-5-9-7-released/)
  [Changelog](https://bugreports.qt.io/browse/QTBUG-70888?filter=20149)

  ### Expat 2.2.6
  * Avoid doing arithmetic with NULL pointers in XML_GetBuffer
  * Fix 2.2.5 regression with suspend-resume while parsing a document like <root/>

  Full changelog [here](https://github.com/libexpat/libexpat/blob/R_2_2_6/expat/Changes)

  a46c847 disables a bunch of qt features we aren't currently using. This speeds up the qt depends build slightly (also decreases the size of the built `qt-5.9.7` tar by about 2%). The disabling is somewhat unintuitive, hence `[wip]` until after a travis run and gitian build.

Tree-SHA512: f3d51d0c7dabe5b7043ef23f264abf2aba3e94e55ffc9d5c323b153b6852d9161368e1591db3ba28f3498f0613bac77d40b855bd0465296f52be03f9230656de
fanquake added a commit to fanquake/bitcoin that referenced this pull request Dec 13, 2018
Github-Pull: bitcoin#14849
Rebased-From: 095e765
fanquake added a commit to fanquake/bitcoin that referenced this pull request Dec 13, 2018
Github-Pull: bitcoin#14849
Rebased-From: 73b46ee
fanquake added a commit to fanquake/bitcoin that referenced this pull request Dec 13, 2018
@DrahtBot
Copy link
Contributor

@DrahtBot DrahtBot commented Dec 13, 2018

Gitian builds for commit 5f23460 (master):

Gitian builds for commit 39019f7d1a424e14fbb4ed2bf05412eb3f8b958f (master and this pull):

@fanquake fanquake deleted the fanquake:qt-5-9-7 branch Jan 28, 2019
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Mar 31, 2020
Summary:
```
This PR upgrades expat and qt in depends.

[...]

(commit id) disables a bunch of qt features we aren't currently using. This speeds up the qt depends build slightly (also decreases the size of the built qt-5.9.7 tar by about 2%).
```

Backport of core [[bitcoin/bitcoin#14849 | PR14849]].

Test Plan: Run the Gitian builds.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5606
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants