build: Bump minimum Qt version to 5.11.3 #24132

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:220123-qtmin changing 5 files +4 −8
  1. hebasto commented at 3:09 pm on January 23, 2022: member

    The current minimum Qt version is 5.9.5 which has been set in bitcoin/bitcoin#21286.

    Distro support:

    • centos 7 – unsupported since bitcoin/bitcoin#23511
    • centos 8 – 5.15.2
    • buster – 5.11.3
    • bullseye – 5.15.2
    • bionic5.9.5
    • focal – 5.12.8

    As another Ubuntu LTS is coming soon, it seems unreasonable to stick to Qt 5.9 which support ended on 2020-05-31. Anyway, it’s still possible to build Bitcoin Core GUI with depends on bionic system.

    Bumping the minimum Qt version allows to make code safer and more reliable, e.g.:

    An example of the patch using the functor-overload of QMetaObject::invokeMethod:

     0--- a/src/qt/walletmodel.cpp
     1+++ b/src/qt/walletmodel.cpp
     2@@ -349,7 +349,7 @@ bool WalletModel::changePassphrase(const SecureString &oldPass, const SecureStri
     3 static void NotifyUnload(WalletModel* walletModel)
     4 {
     5     qDebug() << "NotifyUnload";
     6-    bool invoked = QMetaObject::invokeMethod(walletModel, "unload");
     7+    bool invoked = QMetaObject::invokeMethod(walletModel, &WalletModel::unload);
     8     assert(invoked);
     9 }
    10 
    

    It uses the same new syntax as signal-slot connection with compile-time check. Also see bitcoin/bitcoin#16348.

    This PR is intended to be merged early after branching 23.x off.

  2. hebasto force-pushed on Jan 23, 2022
  3. hebasto marked this as ready for review on Jan 23, 2022
  4. DrahtBot added the label Build system on Jan 23, 2022
  5. DrahtBot added the label GUI on Jan 23, 2022
  6. DrahtBot commented at 9:32 pm on January 23, 2022: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #24337 (build: Do not define PROVIDE_FUZZ_MAIN_FUNCTION macro unconditionally by hebasto)
    • #23565 (doc: rewrite dependencies.md by fanquake)

    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. shaavan commented at 9:27 am on January 24, 2022: contributor

    I am concept ACK on upgrading the minimum required Qt version to a later version because, as it is mentioned, it makes the code more secure and reliable.

    However, I have one question I would like to ask. As you mentioned:

    it seems unreasonable to stick to Qt 5.9, which support ended on 2020-05-31

    I agree with the reasoning. However, when I checked for the Standard Support for Qt 5.11.3, I noticed it ended on 2019-05-22. So if we are upgrading the minimum version, why are we choosing the version whose support ended almost two years ago?

  8. hebasto commented at 10:03 am on January 24, 2022: member

    So if we are upgrading the minimum version, why are we choosing the version whose support ended almost two years ago?

    To support recent two Debian versions. No need to force users to upgrade their systems.

    However, when I checked for the Standard Support for Qt 5.11.3, I noticed it ended on 2019-05-22.

    In particular, Debian 10 packages are still supported.

  9. in ci/test/00_setup_env_native_qt5.sh:10 in e39a9da025 outdated
     6@@ -7,7 +7,7 @@
     7 export LC_ALL=C.UTF-8
     8 
     9 export CONTAINER_NAME=ci_native_qt5
    10-export DOCKER_NAME_TAG=ubuntu:18.04  # Check that bionic gcc-8 can compile our C++17 and run our functional tests in python3, see doc/dependencies.md
    11+export DOCKER_NAME_TAG=debian:buster  # Check that buster gcc-8 can compile our C++17 and run our functional tests in python3, see doc/dependencies.md
    


    fanquake commented at 3:57 am on January 25, 2022:
    Not sure this needs to be changed, but if it does, it should be separate. This also changes the version of GCC 8 being tested against, and the version of Python3 being used would no-longer testing our minimum required. Neither of these things are mentioned anywhere.

    hebasto commented at 10:01 pm on February 5, 2022:
    Thanks! Updated.
  10. fanquake commented at 4:03 am on January 25, 2022: member

    functor-parameter overload of QMetaObject::invokeMethod fixed https://bugreports.qt.io/browse/QTBUG-10907

    ~0. If this is all the motivation, that seems fairly weak.

  11. DrahtBot added the label Needs rebase on Jan 25, 2022
  12. MarcoFalke commented at 8:10 am on January 25, 2022: member

    ~0. If this is all the motivation, that seems fairly weak.

    Not sure what the invokeMethod will change in practice, but when it changes runtime errors into compile time errors (?), then it seems like a fine motivation. With 0.24 it should be fine to drop bionic-qt.

  13. hebasto commented at 9:43 pm on February 5, 2022: member

    @fanquake

    functor-parameter overload of QMetaObject::invokeMethod fixed https://bugreports.qt.io/browse/QTBUG-10907

    ~0. If this is all the motivation, that seems fairly weak. @MarcoFalke

    ~0. If this is all the motivation, that seems fairly weak.

    Not sure what the invokeMethod will change in practice, but when it changes runtime errors into compile time errors (?), then it seems like a fine motivation. With 0.24 it should be fine to drop bionic-qt.

    PR description has been updated with an example. Also see bitcoin/bitcoin#16348 by @promag.

  14. ci: Switch from bionic to buster
    This change is a prerequisite for the following bumping Qt minimum
    version to 5.11.3. It is required as bionic has Qt 5.9.5.
    
    Effectively, this also changes:
    - gcc from 8.4.0 to 8.3.0
    - python from 3.6.5 to 3.7.3
    e22d10b936
  15. build: Bump minimum Qt version to 5.11.3 956f7322f6
  16. hebasto force-pushed on Feb 5, 2022
  17. hebasto commented at 10:01 pm on February 5, 2022: member

    Updated e39a9da025e07f1ca33e28c934b29b3393a451a0 -> 956f7322f60db7b8be551c9074b4c633e514079d (pr24132.01 -> pr24132.02):

  18. DrahtBot removed the label Needs rebase on Feb 5, 2022
  19. MarcoFalke added this to the milestone 24.0 on Feb 6, 2022
  20. MarcoFalke commented at 9:50 am on February 6, 2022: member

    cr ACK 956f7322f60db7b8be551c9074b4c633e514079d

    The python3 CI change should be fine, as we still test python3.6 in the bionic nowallet task.

  21. MarcoFalke added the label DrahtBot Guix build requested on Feb 7, 2022
  22. laanwj commented at 1:57 pm on February 14, 2022: member
    Concept ACK
  23. DrahtBot commented at 4:08 am on February 17, 2022: member

    Guix builds

    File commit 8fe6f5a6fbcd8083d916cb630f35f8f5980d6825(master) commit afe511a19777e509b268abac20ee15d702fccd0b(master and this pull)
    SHA256SUMS.part 15fa6ceff410a599... 574e4c1c04af4882...
    *-aarch64-linux-gnu-debug.tar.gz 64e903c3ab4a158d... cc0c7e88b2bb3076...
    *-aarch64-linux-gnu.tar.gz 687cb2caced5a1d5... f015a3c27d230e05...
    *-arm-linux-gnueabihf-debug.tar.gz 278a4bd0ac3706e1... 65792c7b4773dc82...
    *-arm-linux-gnueabihf.tar.gz b1c7afb5e8ea8e9d... 7c17daa7d074d204...
    *-arm64-apple-darwin.tar.gz ac175d1ead43ec66... 889bec97ecfa4dc2...
    *-osx-unsigned.dmg 1e73babee30d4073... 286e4dff542797a5...
    *-osx-unsigned.tar.gz 8d8f0a29b9dccc1d... 82a736cab76a82ac...
    *-osx64.tar.gz 88222574867e6c1a... 27a50eb04aecaba8...
    *-powerpc64-linux-gnu-debug.tar.gz 9877ab4f41cbb5a2... e7f0a930e9aed9bd...
    *-powerpc64-linux-gnu.tar.gz a6ab4f51fafc8802... 132a078913e9676a...
    *-powerpc64le-linux-gnu-debug.tar.gz ce1958eed5123e00... 45212b9e449df479...
    *-powerpc64le-linux-gnu.tar.gz df479c7c8799f5a9... 13aa73fb82c27b28...
    *-riscv64-linux-gnu-debug.tar.gz bd591a23d9f5cf73... 115f2a226593957c...
    *-riscv64-linux-gnu.tar.gz 78c2f64ad952a8a3... 6f667c38cc77ae6d...
    *-x86_64-linux-gnu-debug.tar.gz 5e2bbc5d33779d93... 06143d05287c36f5...
    *-x86_64-linux-gnu.tar.gz 469ec6f702d64710... 24928c0e48b78608...
    *.tar.gz 1a95f4a3d9e2225b... bf4b4cc342c6192a...
    guix_build.log 72a3e57c4499ea20... ab2a9ff51f57d776...
    guix_build.log.diff b0e3e66831305c42...
  24. DrahtBot removed the label DrahtBot Guix build requested on Feb 17, 2022
  25. promag commented at 11:10 pm on February 22, 2022: member

    Concept ACK.

    Then we can also ditch GUIUtil::ObjectInvoke.

  26. gruve-p commented at 12:07 pm on March 6, 2022: contributor
    Concept ACK
  27. MarcoFalke requested review from fanquake on Mar 7, 2022
  28. fanquake approved
  29. fanquake commented at 2:53 pm on March 7, 2022: member
    ACK 956f7322f60db7b8be551c9074b4c633e514079d
  30. fanquake merged this on Mar 7, 2022
  31. fanquake closed this on Mar 7, 2022

  32. hebasto deleted the branch on Mar 7, 2022
  33. sidhujag referenced this in commit 7d6cbab5e1 on Mar 7, 2022
  34. DrahtBot locked this on Mar 7, 2023

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-12-22 06:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me