build: no-longer allow GCC-10 in C++20 check #30228

1. 
   fanquake commented at 9:48 am on June 5, 2024: member

   Reverts part of fa67f096bdea9db59dd20c470c9e32f3dac5be94, now that we require a minimum of GCC 11.

   See also: #28349 (comment).

2. build: no-longer allow GCC-10 in C++20 check

   Reverts part of fa67f096bdea9db59dd20c470c9e32f3dac5be94, now that we
   require a minimum of GCC 11.
   
   See also:
   https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1745143612.

   232928b58a
3. 
   DrahtBot commented at 9:48 am on June 5, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko, theuni

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. DrahtBot added the label Build system on Jun 5, 2024
5. 
   maflcko commented at 9:56 am on June 5, 2024: member

   Is there any user facing difference? If so, it would be good to show it in a comment.
6. 
   fanquake commented at 10:00 am on June 5, 2024: member

   Is there any user facing difference? If so, it would be good to show it in a comment.

   0./configure CC=gcc-10 CXX=g++-10
   1<snip>
   2configure: error: *** A compiler with support for C++20 language features is required.
   

7. 
   maflcko commented at 10:09 am on June 5, 2024: member

   Otherwise it would continue to compile (and miscompile RenameOver)?
8. 
   fanquake commented at 10:13 am on June 5, 2024: member

   Otherwise it would continue to compile (and miscompile RenameOver)?

   I don’t know where it would fail first, but I don’t see the point of having it continue past configure in this case. There are multiple devs using clang-14 who’ve been confused by clang-14 failing to compile code, and unsure what the issue was. So fixing the case here for GCC seems more useful than random compile errors.

9. 
   maflcko commented at 10:43 am on June 5, 2024: member

   I am asking, because this may need backport, if compilation succeeds. I assumed it would fail, which is why I left it as-is. However, if this compiles for users using g++10, then it seems dangerous. It would expose them to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98985 and possibly other bugs.
10. 
    fanquake commented at 10:53 am on June 5, 2024: member

    The first compile failure I see is in util/time.cpp:

     0util/time.cpp: In function ‘std::string FormatISO8601DateTime(int64_t)’:
     1util/time.cpp:50:24: error: ‘year_month_day’ in namespace ‘std::chrono’ does not name a type
     2   50 |     const std::chrono::year_month_day ymd{days};
     3      |                        ^~~~~~~~~~~~~~
     4util/time.cpp:51:24: error: ‘hh_mm_ss’ in namespace ‘std::chrono’ does not name a type
     5   51 |     const std::chrono::hh_mm_ss hms{secs - days};
     6      |                        ^~~~~~~~
     7util/time.cpp:52:63: error: ‘ymd’ was not declared in this scope
     8   52 |     return strprintf("%04i-%02u-%02uT%02i:%02i:%02iZ", signed{ymd.year()}, unsigned{ymd.month()}, unsigned{ymd.day()}, hms.hours().count(), hms.minutes().count(), hms.seconds().count());
     9      |                                                               ^~~
    10util/time.cpp:52:120: error: ‘hms’ was not declared in this scope
    11   52 |     return strprintf("%04i-%02u-%02uT%02i:%02i:%02iZ", signed{ymd.year()}, unsigned{ymd.month()}, unsigned{ymd.day()}, hms.hours().count(), hms.minutes().count(), hms.seconds().count());
    12      |                                                                                                                        ^~~
    13util/time.cpp: In function ‘std::string FormatISO8601Date(int64_t)’:
    14util/time.cpp:59:24: error: ‘year_month_day’ in namespace ‘std::chrono’ does not name a type
    15   59 |     const std::chrono::year_month_day ymd{days};
    16      |                        ^~~~~~~~~~~~~~
    17util/time.cpp:60:47: error: ‘ymd’ was not declared in this scope
    18   60 |     return strprintf("%04i-%02u-%02u", signed{ymd.year()}, unsigned{ymd.month()}, unsigned{ymd.day()});
    19      |                                               ^~~
    20make[2]: *** [Makefile:11756: util/libbitcoin_util_a-time.o] Error 1
    

11. 
    maflcko commented at 11:29 am on June 5, 2024: member

    Thanks for checking. Can you re-try on 27.x, because https://github.com/bitcoin/bitcoin/commit/c3530254c926a5ab9b710512bbbd29e5cd5b10f7 is not in 27.x.
12. 
    fanquake commented at 11:41 am on June 5, 2024: member

    Can you re-try on 27.x, because https://github.com/bitcoin/bitcoin/commit/c3530254c926a5ab9b710512bbbd29e5cd5b10f7 is not in 27.x.

    27.x & fccd32efe6e2950b2c74fdec2ade54040ca32a2c compiles with gcc-10 (Ubuntu 10.5.0-4ubuntu2) 10.5.0.

13. maflcko added this to the milestone 28.0 on Jun 5, 2024
14. 
    maflcko commented at 12:03 pm on June 5, 2024: member

    Oh, 27.x remains on g++-10, so no backport needed. I was wrong. See https://github.com/bitcoin/bitcoin/blob/27.x/doc/dependencies.md
15. 
    maflcko commented at 12:04 pm on June 5, 2024: member

    utACK 232928b58a82e3f15307deba1ae921ae2960ccc8

    Seems fine to fail early here, instead of running into a util/time compile failure later on.

16. theuni approved
17. 
    theuni commented at 2:20 pm on June 5, 2024: member

    utACK 232928b58a82e3f15307deba1ae921ae2960ccc8
18. fanquake merged this on Jun 6, 2024
19. fanquake closed this on Jun 6, 2024

    ---

20. fanquake deleted the branch on Jun 6, 2024

Contributors
fanquake DrahtBot maflcko theuni

Labels
Build system

Milestone
28.0