depends: disable unused Qt features #16386

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:slim_qt_597 changing 1 files +20 −3
  1. fanquake commented at 4:38 am on July 14, 2019: member

    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.

  2. fanquake added the label GUI on Jul 14, 2019
  3. fanquake added the label Build system on Jul 14, 2019
  4. fanquake added the label Needs gitian build on Jul 14, 2019
  5. MarcoFalke commented at 12:16 pm on July 14, 2019: member 
     0kernel/qsharedmemory_win.cpp: In constructor ‘QSharedMemoryPrivate::QSharedMemoryPrivate()’:
 1kernel/qsharedmemory_win.cpp:52:12: error: class ‘QSharedMemoryPrivate’ does not have any field named ‘systemSemaphore’
 2            systemSemaphore(QString()), lockedByMe(false), hand(0)
 3            ^~~~~~~~~~~~~~~
 4kernel/qsharedmemory_win.cpp:52:40: error: class ‘QSharedMemoryPrivate’ does not have any field named ‘lockedByMe’
 5            systemSemaphore(QString()), lockedByMe(false), hand(0)
 6                                        ^~~~~~~~~~
 7Makefile.Release:32576: recipe for target '.obj/release/qsharedmemory_win.o' failed
 8make[3]: *** [.obj/release/qsharedmemory_win.o] Error 1
 9make[3]: *** Waiting for unfinished jobs....
10make[3]: Leaving directory '/home/travis/build/bitcoin/bitcoin/depends/work/build/x86_64-w64-mingw32/qt/5.9.7-346a555d033/qtbase/src/corelib'
11Makefile:36: recipe for target 'release' failed
  6. promag commented at 9:43 pm on July 14, 2019: member
    Concept ACK, +1 for the documentation or features reference.
  7. molxyz commented at 10:33 pm on July 14, 2019: none

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

    0In file included from ../../include/QtCore/qdebug.h:1:0,
1                 from kernel/qsharedmemory_win.cpp:43:
2../../include/QtCore/../../src/corelib/io/qdebug.h: In member function ‘void QDebug::Stream::setVerbosity(int)’:
3../../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=]
4                 flags &= ~(VerbosityMask << VerbosityShift);
  8. DrahtBot commented at 1:08 am on July 15, 2019: member

    Gitian builds for commit 536590f358dc3d3e5821eba7f1009452ea93b205 (master):

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

  9. DrahtBot removed the label Needs gitian build on Jul 15, 2019
  10. fanquake force-pushed on Jul 15, 2019
  11. fanquake force-pushed on Jul 15, 2019
  12. in depends/packages/qt.mk:77 in ed04310699 outdated 
    70@@ -70,19 +71,35 @@ $(package)_config_opts += -system-zlib
71 $(package)_config_opts += -static
72 $(package)_config_opts += -silent
73 $(package)_config_opts += -v
74+$(package)_config_opts += -no-feature-bearermanagement
75+$(package)_config_opts += -no-feature-colordialog
76+$(package)_config_opts += -no-feature-commandlineparser
77+$(package)_config_opts += -no-feature-concurrent
    laanwj commented at 5:45 pm on July 15, 2019:
    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.
  13. laanwj commented at 5:47 pm on July 15, 2019: member
    Concept ACK
  14. hebasto commented at 6:50 am on July 19, 2019: member
    Concept ACK
  15. practicalswift commented at 11:14 am on July 22, 2019: contributor

    Concept ACK

    Do you have numbers for before vs after?

  16. fanquake force-pushed on Jul 23, 2019
  17. fanquake commented at 7:11 am on July 23, 2019: member
    Rebased for #16408.
  18. dongcarl commented at 6:38 pm on July 23, 2019: member
    For reviewers, here’s a gist with all the configure flags, features, and libraries: https://gist.github.com/dongcarl/f15946dff501dea85977b2c2fb9f00a0
  19. in depends/packages/qt.mk:76 in 0ce7ff7f21 outdated 
    70@@ -70,19 +71,35 @@ $(package)_config_opts += -system-zlib
71 $(package)_config_opts += -static
72 $(package)_config_opts += -silent
73 $(package)_config_opts += -v
74+$(package)_config_opts += -no-feature-bearermanagement
75+$(package)_config_opts += -no-feature-colordialog
76+$(package)_config_opts += -no-feature-commandlineparser
    theuni commented at 6:46 pm on July 23, 2019:
    Does this remove the ability to add qt-specific runtime args to bitcoin-qt ?
    fanquake commented at 2:35 am on July 24, 2019:

    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:

    Hello World

  20. in depends/packages/qt.mk:91 in 0ce7ff7f21 outdated 
    88-$(package)_config_opts += -no-feature-concurrent
89+$(package)_config_opts += -no-feature-printer
90+$(package)_config_opts += -no-feature-printpreviewdialog
91+$(package)_config_opts += -no-feature-printpreviewwidget
92+$(package)_config_opts += -no-feature-regularexpression
93+$(package)_config_opts += -no-feature-sessionmanager
    theuni commented at 6:47 pm on July 23, 2019:
    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.
    fanquake commented at 6:50 pm on July 23, 2019:
    Need to do some runtime checking of the affects of this flag, probably on MacOS or Windows.
    fanquake commented at 3:35 am on July 24, 2019:
    Discussion below.
    fanquake commented at 4:53 am on July 24, 2019:

    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.

  21. in depends/packages/qt.mk:99 in 0ce7ff7f21 outdated 
     96 $(package)_config_opts += -no-feature-syntaxhighlighter
 97 $(package)_config_opts += -no-feature-textbrowser
 98 $(package)_config_opts += -no-feature-textodfwriter
 99+$(package)_config_opts += -no-feature-topleveldomain
100 $(package)_config_opts += -no-feature-udpsocket
101+$(package)_config_opts += -no-feature-undocommand
    theuni commented at 6:48 pm on July 23, 2019:
    Do these disable ctrl+z for, for example, input fields?
    fanquake commented at 2:21 am on July 24, 2019:
    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.
  22. in depends/packages/qt.mk:38 in 0ce7ff7f21 outdated 
    34@@ -35,6 +35,7 @@ $(package)_config_opts += -no-freetype
35 $(package)_config_opts += -no-gif
36 $(package)_config_opts += -no-glib
37 $(package)_config_opts += -no-icu
38+$(package)_config_opts += -no-ico
    theuni commented at 6:54 pm on July 23, 2019:
    For reviewers: We have .ico files in-tree, but they’re processed at build-time by windres.
  23. theuni commented at 6:55 pm on July 23, 2019: member
    Concept ACK
  24. depends: disable unused Qt features 248e22bbc0
  25. fanquake force-pushed on Jul 24, 2019
  26. fanquake commented at 4:56 am on July 24, 2019: member

    Updated to also disable the VNC QPA backend using:

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

    which we have been building on Linux:

    0QPA backends:
1  DirectFB ............................... no
2  EGLFS .................................. no
3  LinuxFB ................................ no
4  VNC .................................... yes
5  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.

  27. sipsorcery commented at 7:12 pm on July 24, 2019: member

    tACK 248e22bbc0d7bc40ae3584d53a18507c46b0e553 (Windows 10 test only)

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

  28. laanwj commented at 8:05 pm on July 25, 2019: member

    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 248e22bbc0d7bc40ae3584d53a18507c46b0e553

  29. laanwj merged this on Jul 25, 2019
  30. laanwj referenced this in commit a54a12046e on Jul 25, 2019
  31. DrahtBot added the label Needs rebase on Jul 25, 2019
  32. fanquake removed the label Needs rebase on Jul 25, 2019
  33. fanquake deleted a comment on Jul 25, 2019
  34. DrahtBot commented at 0:12 am on July 26, 2019: member
  35. DrahtBot added the label Needs rebase on Jul 26, 2019
  36. fanquake removed the label Needs rebase on Jul 26, 2019
  37. DrahtBot commented at 2:16 am on July 26, 2019: member
  38. DrahtBot added the label Needs rebase on Jul 26, 2019
  39. molxyz commented at 2:30 am on July 26, 2019: none

    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?

    image

  40. MarcoFalke closed this on Jul 26, 2019
  41. MarcoFalke removed the label Needs rebase on Jul 26, 2019
  42. sipa commented at 3:27 am on July 26, 2019: member
    @molxyz There shouldn’t be any observable differences from this PR. It’s disabling things we aren’t using anyway.
  43. molxyz commented at 4:49 am on July 26, 2019: none
    @sipa Ah.. Ok, thanks for letting me know. So far I haven’t seen any issue but will keep running the master.
  44. konez2k referenced this in commit 5e9dd64953 on Jul 27, 2019
  45. fanquake deleted the branch on Jan 22, 2020
  46. deadalnix referenced this in commit 99dc69303f on Apr 2, 2020
  47. in depends/packages/qt.mk:79 in 248e22bbc0 
    70@@ -70,19 +71,36 @@ $(package)_config_opts += -system-zlib
71 $(package)_config_opts += -static
72 $(package)_config_opts += -silent
73 $(package)_config_opts += -v
74+$(package)_config_opts += -no-feature-bearermanagement
75+$(package)_config_opts += -no-feature-colordialog
76+$(package)_config_opts += -no-feature-commandlineparser
77+$(package)_config_opts += -no-feature-concurrent
78 $(package)_config_opts += -no-feature-dial
79+$(package)_config_opts += -no-feature-filesystemwatcher
    hebasto commented at 5:42 pm on July 16, 2020:
    This change causes https://github.com/bitcoin-core/gui/issues/32, and it is fixed in #19536.
  48. fanquake referenced this in commit c04485850e on Jul 17, 2020
  49. PastaPastaPasta referenced this in commit 0af66e6574 on Sep 11, 2021
  50. PastaPastaPasta referenced this in commit 1b5d29bb9c on Sep 11, 2021
  51. Munkybooty referenced this in commit 97632ca890 on Nov 9, 2021
  52. Munkybooty referenced this in commit ccd976d600 on Nov 16, 2021
  53. Munkybooty referenced this in commit bf59b2ec92 on Nov 18, 2021
  54. Munkybooty referenced this in commit dcca9d8eb1 on Nov 24, 2021
  55. Munkybooty referenced this in commit 3e40a99573 on Nov 30, 2021
  56. Munkybooty referenced this in commit a9727f8c24 on Dec 15, 2021
  57. Munkybooty referenced this in commit f4ce3738c2 on Dec 17, 2021
  58. Munkybooty referenced this in commit 5e8474fbe5 on Dec 24, 2021
  59. Munkybooty referenced this in commit 94dfa72738 on Jan 19, 2022
  60. Munkybooty referenced this in commit 0ec883f34c on Jan 24, 2022
  61. DrahtBot locked this on Feb 15, 2022


fanquake MarcoFalke promag molxyz DrahtBot laanwj hebasto practicalswift dongcarl theuni sipsorcery sipa

Labels
GUI Build system

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-07-06 01:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me