depends: disable unused Qt features #16386

fanquake · 2019-07-14T04:38:35Z

Related to #16354. Kept separate from #16370, because:

QT is a monster 😂 - dongcarl in #bitcoin-builds

I've done some basic testing on macOS 10.14 and Debian 9.9 so far. Would be good to have someone test on Windows.

I was thinking about adding some inline documentation, i.e info about where to find the lists of Qt features & libraries, as well as breaking the flags up so that it's clearer which libraries we are supplying, which we rely on Qt for etc. Could go towards addressing some of2 in #16354.

maflcko · 2019-07-14T12:16:43Z

kernel/qsharedmemory_win.cpp: In constructor â€˜QSharedMemoryPrivate::QSharedMemoryPrivate()â€™:
kernel/qsharedmemory_win.cpp:52:12: error: class â€˜QSharedMemoryPrivateâ€™ does not have any field named â€˜systemSemaphoreâ€™
            systemSemaphore(QString()), lockedByMe(false), hand(0)
            ^~~~~~~~~~~~~~~
kernel/qsharedmemory_win.cpp:52:40: error: class â€˜QSharedMemoryPrivateâ€™ does not have any field named â€˜lockedByMeâ€™
            systemSemaphore(QString()), lockedByMe(false), hand(0)
                                        ^~~~~~~~~~
Makefile.Release:32576: recipe for target '.obj/release/qsharedmemory_win.o' failed
make[3]: *** [.obj/release/qsharedmemory_win.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[3]: Leaving directory '/home/travis/build/bitcoin/bitcoin/depends/work/build/x86_64-w64-mingw32/qt/5.9.7-346a555d033/qtbase/src/corelib'
Makefile:36: recipe for target 'release' failed

promag · 2019-07-14T21:43:44Z

Concept ACK, +1 for the documentation or features reference.

molxyz · 2019-07-14T22:33:43Z

Compiling on Windows-10 WSL I also got the same errors and a lot of these warnings:

In file included from ../../include/QtCore/qdebug.h:1:0,
                 from kernel/qsharedmemory_win.cpp:43:
../../include/QtCore/../../src/corelib/io/qdebug.h: In member function ‘void QDebug::Stream::setVerbosity(int)’:
../../include/QtCore/../../src/corelib/io/qdebug.h:99:42: warning: result of ‘(7 << 29)’ requires 33 bits to represent, but ‘int’ only has 32 bits [-Wshift-overflow=]
                 flags &= ~(VerbosityMask << VerbosityShift);

DrahtBot · 2019-07-15T01:08:43Z

Gitian builds for commit 536590f (master):

7dcf7fcc532d77238c3c5087d39f0282... bitcoin-0.18.99-aarch64-linux-gnu-debug.tar.gz
cefd9f4187080619242e2412e323f542... bitcoin-0.18.99-aarch64-linux-gnu.tar.gz
24bd9d984e4f919468b2b442f7fbfff6... bitcoin-0.18.99-arm-linux-gnueabihf-debug.tar.gz
5a36c9ba05b9ab2cc8053348652fa666... bitcoin-0.18.99-arm-linux-gnueabihf.tar.gz
796c6d40c287a8a00c8e5451847679eb... bitcoin-0.18.99-i686-pc-linux-gnu-debug.tar.gz
02622e118c3a5878f8d9d01d34bcba9d... bitcoin-0.18.99-i686-pc-linux-gnu.tar.gz
6ddca99a3ef3f839584ada171b18da4b... bitcoin-0.18.99-osx-unsigned.dmg
8c33846b7abcd4af177810f3a9e4d391... bitcoin-0.18.99-osx64.tar.gz
c779723a9ffb0fe3d21a787db276671f... bitcoin-0.18.99-riscv64-linux-gnu-debug.tar.gz
67aaa6ef5495d2dedbead5e9f921dc0b... bitcoin-0.18.99-riscv64-linux-gnu.tar.gz
62cb8db02120dd4e2d641e7ed26dd568... bitcoin-0.18.99-win64-debug.zip
85256e1545fff71b8ec45bbb22968b14... bitcoin-0.18.99-win64-setup-unsigned.exe
6ef44e7b9a91df768c611908f63b081b... bitcoin-0.18.99-win64.zip
1d5d21d9ff75e5277e0dd6aad4e83bb6... bitcoin-0.18.99-x86_64-linux-gnu-debug.tar.gz
0d88d9c9ae2d3852f0360f118912ce35... bitcoin-0.18.99-x86_64-linux-gnu.tar.gz
10acde2a51c3543dd15b32e23b327d68... bitcoin-0.18.99.tar.gz
aa4ebc4807bca5959f4ab61499b253a9... bitcoin-core-linux-0.19-res.yml
9dc9cb4fe3541b6e4ced959a35e9be37... bitcoin-core-osx-0.19-res.yml
f030f7e1b4a3213c252181ab1005bf4b... bitcoin-core-win-0.19-res.yml
954ac1b5e6f159bd013361e11d911ed2... linux-build.log
ca7a587232728f3374528d59b14f78e9... osx-build.log
7112e03faf4bb4f40c115adb18a78bd0... win-build.log

Gitian builds for commit e1332da (master and this pull):

e93d8efe38c42d7d2af93ddbf5f720d2... bitcoin-0.18.99-aarch64-linux-gnu-debug.tar.gz
3bd32492d33d2532d7b8bfc4baa47cb2... bitcoin-0.18.99-aarch64-linux-gnu.tar.gz
52bcb399017ff39bf11dda7a84fe532d... bitcoin-0.18.99-arm-linux-gnueabihf-debug.tar.gz
530e576c30ca9ce7cd79361967ff5e90... bitcoin-0.18.99-arm-linux-gnueabihf.tar.gz
2dafb688aa2bde51aa3f06ef94de77cf... bitcoin-0.18.99-i686-pc-linux-gnu-debug.tar.gz
3d12239db764a3966f589eb54e92f9ec... bitcoin-0.18.99-i686-pc-linux-gnu.tar.gz
88125a043bf2824d5aec0e377ce5de1d... bitcoin-0.18.99-osx-unsigned.dmg
b0309b6f1ccdb2d94215be8ad92b45b9... bitcoin-0.18.99-osx64.tar.gz
b1155e8dc5cf24718cb0ebd8d1d50a16... bitcoin-0.18.99-riscv64-linux-gnu-debug.tar.gz
a34a3744b6c8f12c2968441866ba554c... bitcoin-0.18.99-riscv64-linux-gnu.tar.gz
063afa49c119bc4006e84d38d12143c4... bitcoin-0.18.99-x86_64-linux-gnu-debug.tar.gz
fb0c0259ac6181750fef41ea7ee85ef1... bitcoin-0.18.99-x86_64-linux-gnu.tar.gz
aaca2be3653cf592c8729959e7610562... bitcoin-0.18.99.tar.gz
e92b0620652facfb6546df81ed51d3d5... bitcoin-core-linux-0.19-res.yml
960852c63cc3c04c2bd865d0d112b9a3... bitcoin-core-linux-0.19-res.yml.diff
9893c087b2833047ac71af565455554e... bitcoin-core-osx-0.19-res.yml
7048237dc18bcbccc25751220ca0bbf1... bitcoin-core-osx-0.19-res.yml.diff
ced3f52bb327f83a37f418a2578bd70f... linux-build.log
02c196ef9df8b0b00513dcd17d6508b4... linux-build.log.diff
a7f0052c1573613bf995300820b2ebd4... osx-build.log
00e78fc6800a08a89a12ffbb96eb08e1... osx-build.log.diff
6a22905151e661ece0efc3619cf43bd8... win-build.log
d386178714dc23cf52c149e80b2151b7... win-build.log.diff

laanwj · 2019-07-15T17:45:59Z

depends/packages/qt.mk

+$(package)_config_opts += -no-feature-bearermanagement
+$(package)_config_opts += -no-feature-colordialog
+$(package)_config_opts += -no-feature-commandlineparser
+$(package)_config_opts += -no-feature-concurrent


I was scared here for a bit, but turns out concurrent is for high-level thread handling, an API that is not used in our project, and it's not necessary for low-level multithreading that we use.

laanwj · 2019-07-15T17:47:01Z

Concept ACK

hebasto · 2019-07-19T06:50:34Z

Concept ACK

practicalswift · 2019-07-22T11:14:46Z

Concept ACK

Do you have numbers for before vs after?

fanquake · 2019-07-23T07:11:56Z

Rebased for #16408.

dongcarl · 2019-07-23T18:38:16Z

For reviewers, here's a gist with all the configure flags, features, and libraries: https://gist.github.com/dongcarl/f15946dff501dea85977b2c2fb9f00a0

depends/packages/qt.mk

theuni

Concept ACK

theuni · 2019-07-23T18:46:20Z

depends/packages/qt.mk

@@ -70,19 +71,35 @@ $(package)_config_opts += -system-zlib
 $(package)_config_opts += -static
 $(package)_config_opts += -silent
 $(package)_config_opts += -v
+$(package)_config_opts += -no-feature-bearermanagement
+$(package)_config_opts += -no-feature-colordialog
+$(package)_config_opts += -no-feature-commandlineparser


Does this remove the ability to add qt-specific runtime args to bitcoin-qt ?

It shouldn't do. We don't use any QCommandLineParser functionality, and it is actually another layer on top of any command line argument handling done by QCoreApplication.

I have tested passing through some qt specific options, such as -qwindowtitle and they still seem to work:

theuni · 2019-07-23T18:47:40Z

depends/packages/qt.mk

+$(package)_config_opts += -no-feature-printpreviewdialog
+$(package)_config_opts += -no-feature-printpreviewwidget
+$(package)_config_opts += -no-feature-regularexpression
+$(package)_config_opts += -no-feature-sessionmanager


As mentioned on IRC, I remember originally making this platform-specific for good reason. Maybe it's no longer needed, but let's make sure to understand what's changed.

I've done some basic testing on macOS 10.14 and Windows 10 (built using WSL) and couldn't find obvious runtime issues. Although it might be good to get a Windows user to verify this: @sipsorcery, @NicolasDorier ?

I can't see us using or including QSessionManager anywhere, nor making any calls to QGuiApplication::commitDataRequest or QGuiApplication::saveStateRequest which are Qts two session management signals. My other thought was that it might be being used by QSettings, however config settings in the GUI (i.e coin control) seem to be saved and loaded correctly on the next start.

Looking at our qt package, -no-sm was originally added as a Linux only config option at the introduction of depends. It then became -no-feature-sessionmanager during the Qt 5.9.4 upgrade.

theuni · 2019-07-23T18:48:20Z

depends/packages/qt.mk

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


Do these disable ctrl+z for, for example, input fields?

No, using something like ctrl-z (⌘-z on macOS) still works as expected. My understanding is that QUndoCommands are used when you build a QUndoStack, for example when editing a document. We don't use QUndoStacks or Commands at all.

theuni · 2019-07-23T18:54:58Z

depends/packages/qt.mk

@@ -35,6 +35,7 @@ $(package)_config_opts += -no-freetype
 $(package)_config_opts += -no-gif
 $(package)_config_opts += -no-glib
 $(package)_config_opts += -no-icu
+$(package)_config_opts += -no-ico


For reviewers: We have .ico files in-tree, but they're processed at build-time by windres.

fanquake · 2019-07-24T04:56:34Z

Updated to also disable the VNC QPA backend using:

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

which we have been building on Linux:

QPA backends:
  DirectFB ............................... no
  EGLFS .................................. no
  LinuxFB ................................ no
  VNC .................................... yes
  Mir client ............................. no

-no-feature-vnc doesn't appear if you pass -list-features to Qts ./configure, but you can check that it's being disabled by checking the config summary during a depends build.

sipsorcery · 2019-07-24T19:12:55Z

tACK 248e22b (Windows 10 test only)

Didn't notice any discrepancies when running bitcoin-qt.exe built with this PR.

laanwj · 2019-07-25T20:05:13Z

Qt has a VNC backend ?!? that's wicked cool, but yes, not useful for this project, the last thing you'd want is a wallet listening on a VNC port

ACK 248e22b

248e22b depends: disable unused Qt features (fanquake) Pull request description: Related to #16354. Kept separate from #16370, because: > QT is a monster 😂 - dongcarl in #bitcoin-builds I've done some basic testing on `macOS 10.14` and `Debian 9.9` so far. Would be good to have someone test on Windows. I was thinking about adding some inline documentation, i.e info about where to find the lists of Qt features & libraries, as well as breaking the flags up so that it's clearer which libraries we are supplying, which we rely on Qt for etc. Could go towards addressing some of`2` in #16354. ACKs for top commit: sipsorcery: tACK 248e22b (Windows 10 test only) laanwj: ACK 248e22b Tree-SHA512: 2cdcea8d268de21d355a7625c4d352f65728df0b8d8cc0f396aca676f42099a819f95652dfbfc665c991ba12c52735c1e9b693df4b12e3ee178fd39356fba8e0

DrahtBot · 2019-07-26T00:12:34Z

Needs rebase

DrahtBot · 2019-07-26T02:16:08Z

Needs rebase

molxyz · 2019-07-26T02:30:40Z

I've got the Qt been running on Windows 10 but I'm not sure what we're disabling and what I should check in testing?

Btw while we're at this, could we get rid of the Main Window on the GUI that has no purpose for Windows Qt which was supposed to be changed since last version?

sipa · 2019-07-26T03:27:25Z

@molxyz There shouldn't be any observable differences from this PR. It's disabling things we aren't using anyway.

molxyz · 2019-07-26T04:49:23Z

@sipa Ah.. Ok, thanks for letting me know. So far I haven't seen any issue but will keep running the master.

248e22b depends: disable unused Qt features (fanquake) Pull request description: Related to bitcoin#16354. Kept separate from bitcoin#16370, because: > QT is a monster 😂 - dongcarl in #bitcoin-builds I've done some basic testing on `macOS 10.14` and `Debian 9.9` so far. Would be good to have someone test on Windows. I was thinking about adding some inline documentation, i.e info about where to find the lists of Qt features & libraries, as well as breaking the flags up so that it's clearer which libraries we are supplying, which we rely on Qt for etc. Could go towards addressing some of`2` in bitcoin#16354. ACKs for top commit: sipsorcery: tACK 248e22b (Windows 10 test only) laanwj: ACK 248e22b Tree-SHA512: 2cdcea8d268de21d355a7625c4d352f65728df0b8d8cc0f396aca676f42099a819f95652dfbfc665c991ba12c52735c1e9b693df4b12e3ee178fd39356fba8e0

Summary: Backport of core [[bitcoin/bitcoin#16386 | PR16386]]. Depends on D5636. Test Plan: Run the Gitian builds. Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D5638

hebasto · 2020-07-16T17:42:00Z

depends/packages/qt.mk

 $(package)_config_opts += -no-feature-dial
+$(package)_config_opts += -no-feature-filesystemwatcher


This change causes bitcoin-core/gui#32, and it is fixed in #19536.

6457361 qt: Fix QFileDialog for static builds (Hennadii Stepanov) Pull request description: This change partially reverts 248e22b (#16386) and makes `QFileDialog`s work again for static builds. Fixes bitcoin-core/gui#32. ACKs for top commit: fanquake: ACK 6457361. Although it would be good to know exactly _why_ this fixes the issue. At this stage I also don't think this should be a blocker for 0.20.1. theuni: ACK 6457361 Tree-SHA512: 8ad27e0bcae6debd02f73b7c374743e37d4edd806922b103a2fe494cf2d9930fe9ef3107b5a6c61f3c466cf7462de2641171880398954e7f2c4f417f5bb820d7

248e22b depends: disable unused Qt features (fanquake) Pull request description: Related to bitcoin#16354. Kept separate from bitcoin#16370, because: > QT is a monster 😂 - dongcarl in #bitcoin-builds I've done some basic testing on `macOS 10.14` and `Debian 9.9` so far. Would be good to have someone test on Windows. I was thinking about adding some inline documentation, i.e info about where to find the lists of Qt features & libraries, as well as breaking the flags up so that it's clearer which libraries we are supplying, which we rely on Qt for etc. Could go towards addressing some of`2` in bitcoin#16354. ACKs for top commit: sipsorcery: tACK 248e22b (Windows 10 test only) laanwj: ACK 248e22b Tree-SHA512: 2cdcea8d268de21d355a7625c4d352f65728df0b8d8cc0f396aca676f42099a819f95652dfbfc665c991ba12c52735c1e9b693df4b12e3ee178fd39356fba8e0

fanquake added GUI Build system Needs gitian build labels Jul 14, 2019

DrahtBot removed the Needs gitian build label Jul 15, 2019

fanquake force-pushed the slim_qt_597 branch 2 times, most recently from d235a61 to ed04310 Compare July 15, 2019 05:57

laanwj reviewed Jul 15, 2019

View reviewed changes

fanquake force-pushed the slim_qt_597 branch from ed04310 to 0ce7ff7 Compare July 23, 2019 07:11

fanquake commented Jul 23, 2019

View reviewed changes

depends/packages/qt.mk Show resolved Hide resolved

theuni reviewed Jul 23, 2019

View reviewed changes

depends: disable unused Qt features

248e22b

fanquake force-pushed the slim_qt_597 branch from 0ce7ff7 to 248e22b Compare July 24, 2019 04:55

laanwj merged commit 248e22b into bitcoin:master Jul 25, 2019

DrahtBot added the Needs rebase label Jul 25, 2019

fanquake removed the Needs rebase label Jul 25, 2019

bitcoin deleted a comment from DrahtBot Jul 25, 2019

fanquake removed the Needs rebase label Jul 26, 2019

DrahtBot added the Needs rebase label Jul 26, 2019

maflcko removed the Needs rebase label Jul 26, 2019

fanquake mentioned this pull request Aug 10, 2019

Do not pass in command line arguments to QApplication #16578

Merged

hebasto mentioned this pull request Aug 12, 2019

qt: Add privacy to the Overview page #16432

Merged

molxyz mentioned this pull request Aug 30, 2019

gui: Remove unused menu items for Windows and Linux #16735

Merged

fanquake deleted the slim_qt_597 branch January 22, 2020 11:29

hebasto mentioned this pull request Jul 16, 2020

qt, build: Fix QFileDialog for static builds #19536

Merged

hebasto reviewed Jul 16, 2020

View reviewed changes

hebasto mentioned this pull request Feb 23, 2021

[RFC] test: Clean test_bitcoin-qt output bitcoin-core/gui#222

Open

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

depends: disable unused Qt features #16386

depends: disable unused Qt features #16386

fanquake commented Jul 14, 2019

maflcko commented Jul 14, 2019

promag commented Jul 14, 2019

molxyz commented Jul 14, 2019

DrahtBot commented Jul 15, 2019

laanwj Jul 15, 2019 •

edited

Loading

laanwj commented Jul 15, 2019

hebasto commented Jul 19, 2019

practicalswift commented Jul 22, 2019

fanquake commented Jul 23, 2019

dongcarl commented Jul 23, 2019

theuni left a comment

theuni Jul 23, 2019

fanquake Jul 24, 2019

theuni Jul 23, 2019

fanquake Jul 24, 2019

theuni Jul 23, 2019

fanquake Jul 24, 2019

theuni Jul 23, 2019

fanquake commented Jul 24, 2019

sipsorcery commented Jul 24, 2019

laanwj commented Jul 25, 2019

DrahtBot commented Jul 26, 2019

DrahtBot commented Jul 26, 2019

molxyz commented Jul 26, 2019

sipa commented Jul 26, 2019

molxyz commented Jul 26, 2019

hebasto Jul 16, 2020 •

edited

Loading

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

		$(package)_config_opts += -no-feature-dial
		$(package)_config_opts += -no-feature-filesystemwatcher

depends: disable unused Qt features #16386

depends: disable unused Qt features #16386

Conversation

fanquake commented Jul 14, 2019

maflcko commented Jul 14, 2019

promag commented Jul 14, 2019

molxyz commented Jul 14, 2019

DrahtBot commented Jul 15, 2019

laanwj Jul 15, 2019 • edited Loading

Choose a reason for hiding this comment

laanwj commented Jul 15, 2019

hebasto commented Jul 19, 2019

practicalswift commented Jul 22, 2019

fanquake commented Jul 23, 2019

dongcarl commented Jul 23, 2019

theuni left a comment

Choose a reason for hiding this comment

theuni Jul 23, 2019

Choose a reason for hiding this comment

fanquake Jul 24, 2019

Choose a reason for hiding this comment

theuni Jul 23, 2019

Choose a reason for hiding this comment

fanquake Jul 24, 2019

Choose a reason for hiding this comment

theuni Jul 23, 2019

Choose a reason for hiding this comment

fanquake Jul 24, 2019

Choose a reason for hiding this comment

theuni Jul 23, 2019

Choose a reason for hiding this comment

fanquake commented Jul 24, 2019

sipsorcery commented Jul 24, 2019

laanwj commented Jul 25, 2019

DrahtBot commented Jul 26, 2019

DrahtBot commented Jul 26, 2019

molxyz commented Jul 26, 2019

sipa commented Jul 26, 2019

molxyz commented Jul 26, 2019

hebasto Jul 16, 2020 • edited Loading

Choose a reason for hiding this comment

laanwj Jul 15, 2019 •

edited

Loading

hebasto Jul 16, 2020 •

edited

Loading