build: use C++17 in depends #20471

pull fanquake wants to merge 5 commits into bitcoin:master from fanquake:depends_c++17 changing 5 files +6 −6
  1. fanquake commented at 3:45 am on November 24, 2020: member

    In packages where we are passing -std=c++11 switch to -std=c++17, or, -std=c++1z in the case of Qt.

    This PR also contains a commit that improves debug output when building Qt for debugging (DEBUG=1).

    Now we’ll get output like this:

    0g++ -c -pipe -ffunction-sections -O2 -fPIC -std=c++11 -fno-exceptions <lots more> ../../corelib/kernel/qcoreapplication.cpp
    

    rather than just:

    0compiling ../../corelib/kernel/qcoreapplication.cpp
    

    Note that when you look at the DEBUG output for these changes when building Qt, you’ll see objects being compiled with a mix of C++11 and C++17. The breakdown is roughly:

    1. qmake built with -std=c++11:
    0Creating qmake...
    1make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/qmake'
    2g++ -c -o project.o   -std=c++11 -ffunction-sections -O2 -g <trim> <trim>/qt/5.9.8-4110fa99945/qtbase/qmake/project.cpp
    3
    4# when qmake, Qt also builds some of it's corelib, such as corelib/global/qmalloc.cpp
    5g++ -c -o qmalloc.o   -std=c++11 -ffunction-sections -O2 -g <trim> <trim>/qt/5.9.8-4110fa99945/qtbase/src/corelib/global/qmalloc.cpp
    
    1. qmake is run, and passed our build options, including -c++std:
    0make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase'
    1<trim>qt/5.9.8-4110fa99945/qtbase/bin/qmake -o Makefile qtbase.pro -- -bindir <trim>/native/bin -c++std c++1z -confirm-license <trim>
    
    1. After some cleaning and configuring, we actually start to build Qt, as well as it’s tools and internal libs:
     0Building qt...
     1make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/src'
     2
     3# build libpng, zlib etc
     4gcc -c -m64 -pipe -pipe -O1 <trim> -o .obj/png.o png.c
     5
     6# build libQt5Bootstrap, using C++11, which again compiles qmalloc.cpp
     7make[2]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qtbase/src/tools/bootstrap'
     8g++ -c -pipe -ffunction-sections -O2 -fPIC -std=c++11 <trim> -o .obj/qmalloc.o ../../corelib/global/qmalloc.cpp
     9
    10# build a bunch of tools like moc, rcc, uic, qfloat16-tables, qdbuscpp2xml, using C++11
    11g++ -c -pipe -O2 -std=c++11 -fno-exceptions -Wall -W <trim> -o .obj/rcc.o rcc.cpp
    12
    13# from here, Qt is compiled with -std=c++1z, including qmalloc.cpp, for the third and final time:
    14g++ -c -include .pch/Qt5Core <trim> -g -Og -fPIC -std=c++1z -fvisibility=hidden <trim> -o .obj/qmalloc.o global/qmalloc.cpp
    
    1. Finally, build tools like lrelease, lupdate, etc, but back to using -std=c++11
    0make[1]: Entering directory '<trim>/qt/5.9.8-4110fa99945/qttools/src/linguist/lrelease'
    1g++ -c -pipe -O2 -std=c++11 -fno-exceptions -Wall -W <trim> -o .obj/translator.o ../shared/translator.cpp
    

    If you dump the debug info from the built Qt libs, they should also tell you that they were compiled with C++17:

    0objdump -g bitcoin/depends/x86_64-pc-linux-gnu/lib/libQt5Core.a
    1GNU C++17 9.3.0 -m64 -mtune=generic -march=x86-64 -g -O1 -Og -std=c++17 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -fasynchronous-unwind-tables -fstack-protector-strong -fstack-clash-protection -fcf-protection
    
  2. depends: build bdb with -std=c++17 2dde55702d
  3. depends: build Boost with -std=c++17 2374f2fbef
  4. depends: build zeromq with -std=c++17 e2c500636c
  5. builds: don't pass -silent to qt when building in debug mode
    This means we'll get build output like this when building with DEBUG=1:
    
    g++ -c -pipe -ffunction-sections -O2 -fPIC -std=c++11 -fno-exceptions <lots more> ../../corelib/kernel/qcoreapplication.cpp
    
    rather than just:
    
    compiling ../../corelib/kernel/qcoreapplication.cpp
    104e859c97
  6. depends: build qt in c++17 mode 2f5dfe4a7f
  7. fanquake added the label Build system on Nov 24, 2020
  8. MarcoFalke commented at 7:11 am on November 24, 2020: member
    Is this needed for anything or does this change anything about the binaries? I presume it is needed for qt5.15?
  9. laanwj commented at 8:35 am on November 24, 2020: member

    I think it’s safer with regard to ABI to compile them with the same standard as the application, definitely for boost. We’ve seen some problems in that regard in the past.

    And especially building with an older C++XX than the application itself could cause some things to be missing that are conditional on the C++XX version.

    Note that when you look at the DEBUG output for these changes when building Qt, you’ll see objects being compiled with a mix of C++11 and C++17. The breakdown is roughly:

    Glad you got it to work eventually to build the library with C++17. It doesn’t matter that the intermediate tools are built with C++11. It’s a bit strange though that it seems to resist being built with C++17 at every step.

    Would this be solved by upgrading Qt?

  10. DrahtBot commented at 8:43 am on November 24, 2020: 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:

    • #19716 (build: Qt 5.15.x by fanquake)
    • #19245 ([WIP DONOTMERGE] Replace boost::filesystem with std::filesystem (in c++17) by kiminuo)

    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.

  11. practicalswift commented at 8:53 am on November 24, 2020: contributor
    Concept ACK for the reasons given by @laanwj
  12. fanquake commented at 2:45 am on November 25, 2020: member

    Would this be solved by upgrading Qt?

    It doesn’t look like it. #19716 (Qt 5.15.2) still behaves in the same way when built in c++1z mode.

  13. laanwj commented at 8:54 am on November 25, 2020: member

    It doesn’t look like it. #19716 (Qt 5.15.2) still behaves in the same way when built in c++1z mode.

    OK, good to know. That should be in for 0.22 anyway. I suppose for that version we can also just use c++17 in not need the 1z temporary name.

    Code review ACK https://github.com/bitcoin/bitcoin/pull/20471/commits/2f5dfe4a7ff12b6b57427374142cdf7e266b73bc

  14. fanquake commented at 9:00 am on November 25, 2020: member

    I suppose for that version we can also just use c++17 in not need the 1z temporary name.

    Yep, we will be able to. 5.15.x understands -c++std c++17.

  15. laanwj added the label Needs gitian build on Nov 25, 2020
  16. laanwj added the label Needs Guix build on Nov 25, 2020
  17. hebasto commented at 11:32 am on November 25, 2020: member
    Concept ACK.
  18. hebasto approved
  19. hebasto commented at 12:08 pm on November 25, 2020: member
    ACK 2f5dfe4a7ff12b6b57427374142cdf7e266b73bc, I have reviewed the code and it looks OK, I agree it can be merged.
  20. practicalswift commented at 1:26 pm on November 25, 2020: contributor
    cr ACK 2f5dfe4a7ff12b6b57427374142cdf7e266b73bc: patch looks correct
  21. DrahtBot commented at 1:33 am on November 27, 2020: member

    Guix builds

    File commit afdfd3c8c1ce96adae11809e3989de381137fee9(master) commit b76f624945387049dd4b594f553f9be0669664e9(master and this pull)
    *-aarch64-linux-gnu-debug.tar.gz 321e361023e8ea7b... 9da9c1f62c371d60...
    *-aarch64-linux-gnu.tar.gz b054d1129516836e... 3e3d8bba7451f10d...
    *-arm-linux-gnueabihf-debug.tar.gz cdd6185a356e7a80... a2cf788ddeb262a0...
    *-arm-linux-gnueabihf.tar.gz 4db9b1126b6601b7... 0d437227eb7d4c8d...
    *-riscv64-linux-gnu-debug.tar.gz df4a6eb0267f7140... ef06c5f32f589def...
    *-riscv64-linux-gnu.tar.gz 3ff6d83bd5983e2b... d21af7048218cd49...
    *-win-unsigned.tar.gz 0dbc83ed5f22a4db... ab0c761f0ed98d6f...
    *-win64-debug.zip 539b210aa13b56bc... 48a47fabb87385a8...
    *-win64-setup-unsigned.exe 5ff779deff3db721... ae0cd605e85aa4e9...
    *-win64.zip 4e50c84031e8b116... 812b84470b2be3fb...
    *-x86_64-linux-gnu-debug.tar.gz 345daa571d3557b0... 9c6ea737387d490d...
    *-x86_64-linux-gnu.tar.gz 60655c6fb26e8633... 8005e3e97e21aa4c...
    *.tar.gz 0ddcf17cb856ed92... aaa4ca0f4fa2267d...
    guix_build.log 6670400b0a6511d0... 366fb8ef0ebc8e02...
    guix_build.log.diff e1a27f2e3a782baf...
  22. DrahtBot removed the label Needs Guix build on Nov 27, 2020
  23. fjahr commented at 1:34 pm on November 29, 2020: member
    Code review ACK 2f5dfe4a7ff12b6b57427374142cdf7e266b73bc
  24. DrahtBot commented at 3:16 am on November 30, 2020: member

    Gitian builds

    File commit e2ff5e7b35d71195278d2a2ed9485f141de33d7a(master) commit 68f7a5a7322c6ad12f0cce29178a3458de2a46f3(master and this pull)
    bitcoin-core-linux-22-res.yml 44e52af36236879f... 77a2fc07bd68d2a6...
    bitcoin-core-osx-22-res.yml 71d5fb0e6642d6b0... ae6c70bc0ad8d2d0...
    bitcoin-core-win-22-res.yml 0148a2f5e7295184... 4f7dec9db3ccf145...
    *-aarch64-linux-gnu-debug.tar.gz 383e84aaa96f342a... 66a5b3246caffba3...
    *-aarch64-linux-gnu.tar.gz 3558211891c0484a... 7b103e2df103f0ad...
    *-arm-linux-gnueabihf-debug.tar.gz 5ebbb475a590480a... 673dd40fac20ae5a...
    *-arm-linux-gnueabihf.tar.gz 60951d893e2688b6... 73c0f0cd908a56ba...
    *-osx-unsigned.dmg 7cba6edda598c96c... 815a84ed2581b08f...
    *-osx64.tar.gz ea2ab5c893780a2e... 3307b01251079806...
    *-riscv64-linux-gnu-debug.tar.gz 0c20a43923efcd5f... b63971f914f260c2...
    *-riscv64-linux-gnu.tar.gz 8d3632d81c77eff1... 36291e1691bc7eeb...
    *-win64-debug.zip 72c35e133cf9a0db... d633c76c2989b5fd...
    *-win64-setup-unsigned.exe 55c17bfff54273e5... 2e09afb90ef49085...
    *-win64.zip f7bf6737c7f741c3... 4a493666259a77e0...
    *-x86_64-linux-gnu-debug.tar.gz abf91baef5c44686... 9e9cdb28b5876893...
    *-x86_64-linux-gnu.tar.gz 75fda490174df335... 131a16fd7423e578...
    *.tar.gz 793a2c7af39452d5... 9d6a50076e1ceeb1...
    linux-build.log 9c5461a914e1f4c0... e21e0db664dfb49f...
    osx-build.log 5adaf67afbaf00fc... 9e059e1e692cca81...
    win-build.log 756101ecccffabf0... 2dd93bcee2b46dfd...
    bitcoin-core-linux-22-res.yml.diff 38fea8873446f606...
    bitcoin-core-osx-22-res.yml.diff 27f413453571eba2...
    bitcoin-core-win-22-res.yml.diff dad9947bdcba36eb...
    linux-build.log.diff 532c1e5988509e79...
    osx-build.log.diff c3201f7816e04e86...
    win-build.log.diff 8d907cadd7fbbced...
  25. DrahtBot removed the label Needs gitian build on Nov 30, 2020
  26. fanquake merged this on Nov 30, 2020
  27. fanquake closed this on Nov 30, 2020

  28. sidhujag referenced this in commit 9c2687c204 on Nov 30, 2020
  29. fanquake referenced this in commit 0a13d15c14 on Dec 2, 2020
  30. fanquake deleted the branch on Feb 9, 2021
  31. MarkLTZ referenced this in commit 39550b91a6 on Feb 16, 2021
  32. kittywhiskers referenced this in commit 7f1f7d97d1 on Jul 13, 2021
  33. kittywhiskers referenced this in commit b6d1f19212 on Jul 15, 2021
  34. kittywhiskers referenced this in commit c776d049d2 on Jul 15, 2021
  35. kittywhiskers referenced this in commit fa46a063dd on Jul 17, 2021
  36. kittywhiskers referenced this in commit 640ed7b3e2 on Sep 11, 2021
  37. kittywhiskers referenced this in commit 4f8e447634 on Sep 11, 2021
  38. kittywhiskers referenced this in commit 4906903e85 on Sep 15, 2021
  39. kittywhiskers referenced this in commit c81f430289 on Sep 15, 2021
  40. thelazier referenced this in commit bc5e95cf6c on Sep 19, 2021
  41. thelazier referenced this in commit 10a5e36a7b on Sep 25, 2021
  42. gades referenced this in commit 49d8a684ba on May 2, 2022
  43. DrahtBot locked this on Aug 16, 2022

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-17 09:12 UTC

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