[POC] build: Hello Qt 6 #24798

pull hebasto wants to merge 7 commits into bitcoin:master from hebasto:220406-qt6 changing 29 files +599 −425
  1. hebasto commented at 2:11 pm on April 7, 2022: member

    This is a draft of upgrading Qt in depends up to Qt 6.2 LTS.

    The motivation is:

    • Qt 6 seems (almost) inevitable in the future
    • this change drops all of the backported patches (but introduces a new one)

    Tasks

    • backport the QTBUG-86080 fix
    • integrate Qt 6 into build-aux/m4/bitcoin_qt.m4
    • fix builds which are configured with --disable-wallet
    • cross-compiling
    • Guix builds

    Closes bitcoin/bitcoin#20627.

    Steps to build on Linux x86_64:

    0$ make -C depends
1$ ./autogen.sh
2$ ./configure --with-gui=qt6 CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site
3$ make clean 
4$ make
5$ make check
  2. DrahtBot added the label Build system on Apr 7, 2022
  3. laanwj commented at 4:25 pm on April 7, 2022: member
    So I understand this doesn’t need any actual code changes? Only build system and depends?
  4. laanwj added the label GUI on Apr 7, 2022
  5. hebasto commented at 4:51 pm on April 7, 2022: member

    So I understand this doesn’t need any actual code changes? Only build system and depends?

    Cannot say for sure at this point.

  6. DrahtBot commented at 7:40 am on April 8, 2022: contributor

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

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #25797 (build: Add CMake-based build system by hebasto)
    • #25391 (guix: Use LTO to build releases by fanquake)
    • #16545 (refactor: Implement missing error checking for ArgsManager flags by ryanofsky)

    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.

  7. laanwj commented at 12:45 pm on April 8, 2022: member

    Cannot say for sure at this point.

    OK I assumed so, as it wouldn’t make much sense to change depends if the code cannot handle it. We could try compiling against system Qt6.

  8. hebasto commented at 11:52 pm on April 8, 2022: member

    Cannot say for sure at this point.

    OK I assumed so, as it wouldn’t make much sense to change depends if the code cannot handle it. We could try compiling against system Qt6.

    Some changes are required: bitcoin-core/gui#577, bitcoin-core/gui#578, bitcoin-core/gui#579, bitcoin-core/gui#580, #24813.

  9. hebasto commented at 11:27 am on April 9, 2022: member

    Compiled and linked ~(not pushed yet)~:

    02022-04-09T11:25:38Z Bitcoin Core version v23.99.0-388ee2edb127 (release build)
12022-04-09T11:25:38Z Qt 6.2.4 (static), plugin=xcb (static)
22022-04-09T11:25:38Z Static plugins:
32022-04-09T11:25:38Z  QXcbIntegrationPlugin, version 393728
42022-04-09T11:25:38Z Style: fusion / QFusionStyle
52022-04-09T11:25:38Z System: Ubuntu Jammy Jellyfish (development branch), x86_64-little_endian-lp64
6...

    Screenshot from 2022-04-09 13-24-24

  10. hebasto force-pushed on Apr 9, 2022
  11. hebasto force-pushed on Apr 9, 2022
  12. hebasto commented at 4:01 pm on April 9, 2022: member

    Steps to build this PR in its current state on Linux x86_64:

    0$ make -C depends
1$ ./autogen.sh
2$ CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site ./configure
3$ make clean 
4$ make
  13. hebasto commented at 5:06 pm on April 9, 2022: member

    I ran into a problem while working on this PR.

    The problem’s root is in two facts:

    1. our build system rely on pkg-config
    2. Qt 6 build system, after switching from qmake to cmake, does not produce *.pc files

    The fix suggested in QTBUG-86080 works only for shared libraries.

    Here are some ways to work out this problem:

    1. modify Qt’s patch to support static builds. Nevertheless, the generated *.pc files lack *.private sections which contains libraries required for static builds. This issue in turn can be solved by adjusting resulted *_LIB variables ad-hoc. Btw, this approach is currently implemented in this PR
    2. develop our own patch which make Qt generate correct *.pc files for static builds (the content of internal QMAKE_PRL_LIBS variables could be used for that)
    3. keep working *.pc files in our repo, and substitute them for static builds
    4. something else
    5. …
  14. jarolrod commented at 8:14 pm on April 10, 2022: member

    While it’s good to think about this kind of integration, I don’t know how feasible this will ultimately be.

    In d0015fd5f3b0465d2b3f36fda3714e248f1b4c51, you uncomment lines where the patch author intended to return when performing a static build. Here’s a link to a discussion on the QTBUG-86080 page where it talks about the current qt internal issues with *.pc files and static builds.

    On the surface, it appears we most likely can get this working. But, I worry about the implications of the hacky path we’d take to get there.

  15. laanwj commented at 10:42 am on April 11, 2022: member

    IMO for static linking in the release, it’s acceptable to carry some hacky patches in depends until the issue gets resolved upstream. I don’t have any specific opinion which one, as long as it’s works and is maintainable.

    I do think we should be able to build against distro Qt6 -dev packages for local builds for development (so without any Qt-side patches). But dynamic-only is fine there. Not sure what the recommended approach to find the libraries is if they don’t ship *.pc, though. Probing hardcoded paths is really crappy. Any other autoconf-using Qt6 applications we can borrow scripting from?

  16. luke-jr commented at 9:12 pm on April 12, 2022: member
    Would prefer to see this PR split between Qt6 compatibility (goal ASAP), and using Qt6 for releases (goal TBD later).
  17. hebasto commented at 9:15 pm on April 12, 2022: member

    Would prefer to see this PR split between Qt6 compatibility (goal ASAP), and using Qt6 for releases (goal TBD later).

    #24798 (comment) – is it what you mean?

  18. hebasto referenced this in commit f60a63cc5f on Apr 12, 2022
  19. hebasto force-pushed on Apr 12, 2022
  20. hebasto commented at 11:35 pm on April 12, 2022: member
    Rebased on top of the bitcoin-core/gui#577.
  21. hebasto force-pushed on Apr 15, 2022
  22. hebasto commented at 10:08 am on April 15, 2022: member
    Rebased on top of the bitcoin-core/gui#579.
  23. hebasto referenced this in commit 37e49cc1b5 on Apr 19, 2022
  24. hebasto force-pushed on Apr 19, 2022
  25. hebasto commented at 5:46 pm on April 19, 2022: member
    Rebased on top of the bitcoin-core/gui#580 and bitcoin-core/gui#584.
  26. sidhujag referenced this in commit 1fcb269d44 on Apr 19, 2022
  27. in build-aux/m4/bitcoin_qt.m4:437 in 2be94f6924 outdated 
    435   BITCOIN_QT_CHECK([
436-    PKG_CHECK_MODULES([QT_WIDGETS], [${qt_lib_prefix}Widgets${qt_lib_suffix} $qt_version], [QT_INCLUDES="$QT_WIDGETS_CFLAGS $QT_INCLUDES" QT_LIBS="$QT_WIDGETS_LIBS $QT_LIBS"],
437+    PKG_CHECK_MODULES([QT_WIDGETS], [${qt_lib_prefix}Widgets${qt_lib_suffix} $qt_version], [],
438                       [BITCOIN_QT_FAIL([${qt_lib_prefix}Widgets${qt_lib_suffix} $qt_version not found])])
439+    if test "$bitcoin_qt_want_version" = "qt6"; then
440+      QT_WIDGETS_LIBS="$QT_WIDGETS_LIBS ${qt_lib_path}/objects-Release/Widgets_resources_1/.rcc/qrc_qstyle.cpp.o ${qt_lib_path}/objects-Release/Widgets_resources_2/.rcc/qrc_qstyle1.cpp.o ${qt_lib_path}/objects-Release/Widgets_resources_3/.rcc/qrc_qmessagebox.cpp.o"
    fanquake commented at 10:57 am on April 20, 2022:
    In 9f61d6a7605eee4b116b8791276bd71baf75bf10: Can you explain what’s going on here? Trying to link against .o files in configure checks not only seems very fragile, but outright hacky.
    hebasto commented at 11:01 am on April 20, 2022:
    This is reproducing of lacking pkg-config --static functionality. List of files to link against has been taken from *.prl files.
  28. hebasto force-pushed on Apr 22, 2022
  29. hebasto force-pushed on Apr 22, 2022
  30. hebasto force-pushed on Apr 27, 2022
  31. hebasto commented at 1:02 pm on April 27, 2022: member
    Rebased on top of the bitcoin-core/gui#589.
  32. hebasto force-pushed on May 23, 2022
  33. hebasto commented at 7:05 am on May 23, 2022: member
    Rebased on top of the bitcoin-core/gui#586.
  34. hebasto force-pushed on May 23, 2022
  35. hebasto commented at 8:14 am on May 23, 2022: member
    Fixed builds which are configured with --disable-wallet.
  36. hebasto referenced this in commit 8898906370 on May 24, 2022
  37. sidhujag referenced this in commit c24c674950 on May 28, 2022
  38. DrahtBot added the label Needs rebase on May 31, 2022
  39. hebasto force-pushed on May 31, 2022
  40. DrahtBot removed the label Needs rebase on May 31, 2022
  41. DrahtBot added the label Needs rebase on Jun 2, 2022
  42. laanwj referenced this in commit bc83710fdc on Jun 23, 2022
  43. fanquake commented at 11:05 am on August 10, 2022: member
    Can you rebase this to reflect the current state of / remaining changes needed for integration? Seems that some of the changes here have now made it in (via the GUI repo).
  44. hebasto force-pushed on Aug 10, 2022
  45. DrahtBot removed the label Needs rebase on Aug 10, 2022
  46. hebasto force-pushed on Aug 10, 2022
  47. qt6: Fix builds which are configured with `--disable-wallet` fb872edc19
  48. build: Pass `qt_lib_path` from depends to the `configure` script 1c6e80c296
  49. build, qt: Integrate DBus flags into QT_{INCLUDES,LIBS} c38f1602bb
  50. build: Add `--with-gui=qt6` configure option 8ce5666b02
  51. build: Bump Qt up to 6.2.4 LTS e494e9fec5
  52. Drop `qttools` temporarily ec9ea20e0e
  53. ci: Adjust CI task for Qt 6 e9861cebfd
  54. hebasto force-pushed on Aug 10, 2022
  55. hebasto commented at 11:01 pm on August 10, 2022: member

    Can you rebase this to reflect the current state of / remaining changes needed for integration? Seems that some of the changes here have now made it in (via the GUI repo).

    Done :tiger2:

  56. DrahtBot commented at 10:36 pm on October 4, 2022: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.

    Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

  57. DrahtBot added the label Needs rebase on Oct 4, 2022
  58. fanquake commented at 11:37 am on December 5, 2022: member
    Should we close this (for now)? This is blocked on cmake, as we aren’t going to introduce a –with-qt6 option, or the other build system changes here. Looks like fb872edc1996527b6951899ff0127a6b815d0a41 could possibly be PR’d on it’s own? Otherwise I’m not sure this is reviewable / that useful to keep open?
  59. hebasto closed this on Dec 5, 2022
  60. bitcoin locked this on Dec 5, 2023
  61. hebasto deleted the branch on Oct 25, 2024


hebasto laanwj DrahtBot jarolrod luke-jr fanquake

Labels
GUI Build system Needs rebase

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