Bump minimum required Boost version due to migration to C++20 #29066

1. 
   hebasto commented at 6:25 pm on December 12, 2023: member

   Boost versions <1.73 have C++20-specific bugs that were fixed in the following commits:

   • https://github.com/boostorg/signals2/commit/15fcf213563718d2378b6b83a1614680a4fa8cec
   • https://github.com/boostorg/test/commit/495c095dc063052ce54f2fe9217fe0fc69ced5f1

   I tested libboost1.71-dev in Ubuntu 20.04 and Boost 1.71, 1.72, 1.73 in our depends build system.

   Closes #29063.

2. 
   DrahtBot commented at 6:25 pm on December 12, 2023: 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	fanquake
   Stale ACK	maflcko

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

3. DrahtBot added the label Docs on Dec 12, 2023
4. hebasto force-pushed on Dec 12, 2023
5. 
   maflcko commented at 6:39 pm on December 12, 2023: member

   https://github.com/boostorg/signals2/commit/15fcf213563718d2378b6b83a1614680a4fa8cec

   This one could be fixed alternatively (in theory) by nuking signals2, but I don’t think this will happen any time soon.

6. 
   maflcko commented at 6:39 pm on December 12, 2023: member

   lgtm ACK 1e0ed3bc0a99c08384642ad81ed4771bc98208b0
7. hebasto force-pushed on Dec 12, 2023
8. hebasto renamed this:
   doc: Bump minimum required Boost version due to migration to C++20
   Bump minimum required Boost version due to migration to C++20
   on Dec 12, 2023
9. 
   hebasto commented at 7:41 pm on December 12, 2023: member

   Oops… Forgot to update a version in the AX_BOOST_BASE macro.

   Fixed now.

   Sorry.

10. 
    fanquake commented at 7:41 pm on December 12, 2023: member

    This one could be fixed alternatively (in theory) by nuking signals2, but I don’t think this will happen any time soon.

    I know a [WIP] branch for this “exists” in some form. cc @theuni

11. hebasto added the label Build system on Dec 12, 2023
12. maflcko added this to the milestone 27.0 on Dec 12, 2023
13. maflcko removed the label Docs on Dec 12, 2023
14. 
    maflcko commented at 7:59 pm on December 12, 2023: member

    Should be fine to squash a 2-line diff into one commit?
15. build: Bump minimum required Boost to 1.73.0 to support C++20

    Boost versions <1.73 have C++20-specific bugs that were fixed in the
    following commits:
    - https://github.com/boostorg/signals2/commit/15fcf213563718d2378b6b83a1614680a4fa8cec
    - https://github.com/boostorg/test/commit/495c095dc063052ce54f2fe9217fe0fc69ced5f1

    49a90915aa
16. hebasto force-pushed on Dec 12, 2023
17. 
    hebasto commented at 8:00 pm on December 12, 2023: member

    Should be fine to squash a 2-line diff into one commit?

    Sure :)

18. fanquake approved
19. 
    fanquake commented at 1:06 pm on December 13, 2023: member

    ACK 49a90915aa3ee8e3a7e163f23a55de931faf8523
20. DrahtBot requested review from maflcko on Dec 13, 2023
21. fanquake merged this on Dec 13, 2023
22. fanquake closed this on Dec 13, 2023

    ---

23. hebasto deleted the branch on Dec 13, 2023
24. 
    theuni commented at 4:02 pm on December 14, 2023: member

    This one could be fixed alternatively (in theory) by nuking signals2, but I don’t think this will happen any time soon.

    I know a [WIP] branch for this “exists” in some form. cc @theuni

    It’s not a WIP so much as a POC: https://github.com/theuni/bitcoin/commits/replace-boost-signals/

    That implements the subset of signals that we rely on as a drop-in replacement. It passes tests and would serve as a good guide, but I don’t think that’s how we’d want to do it.

    I could look at picking it up again and doing it right if there’s interest.

25. 
    maflcko commented at 4:19 pm on December 14, 2023: member

    I was looking in moving boost signals2 into one translation unit (because it is so heavy and keeps the compiler busy), to only expose the stuff that is needed, but if it is possible to re-implement signals2 in 150 LOC, that seems preferable?
26. 
    theuni commented at 4:48 pm on December 14, 2023: member

    @maflcko IIRC I wanted to fix #26442 before going forward with the replacement, as that makes the implementation more straightforward and greatly simplifies testing. Any interest in taking a look there?

Contributors
hebasto DrahtBot maflcko fanquake theuni

Review Requested
maflcko

Labels
Build system

Milestone
27.0