Skip to content

Conversation

@PastaPastaPasta
Copy link
Member

Issue being fixed or feature implemented

See commit

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@PastaPastaPasta PastaPastaPasta added this to the 22 milestone Oct 4, 2024
@PastaPastaPasta PastaPastaPasta force-pushed the backport/bump-boost-min branch from facd1fb to b7b48cd Compare October 4, 2024 20:33
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debug build fails for me with

test/util/setup_common.cpp:444:9: error: use of undeclared identifier 'BOOST_ASSERT'
  444 |         BOOST_ASSERT(cbTx.has_value());
      |         ^
test/util/setup_common.cpp:447:13: error: use of undeclared identifier 'BOOST_ASSERT'
  447 |             BOOST_ASSERT(false);
      |             ^
test/util/setup_common.cpp:450:13: error: use of undeclared identifier 'BOOST_ASSERT'
  450 |             BOOST_ASSERT(false);
      |             ^

backporting 24395 (23a4d5d9) fixes it.

@UdjinM6
Copy link

UdjinM6 commented Oct 4, 2024

Hmm... lots of failures in CI.. there must be smth else then. Also, it looks like 24395 caused high CPU usage bitcoin#27586 and was reverted in 27724... so we need that too (or drop 24395 for now) 🙈

@UdjinM6
Copy link

UdjinM6 commented Oct 4, 2024

let's try 27783 (887241a)?

@UdjinM6 UdjinM6 force-pushed the backport/bump-boost-min branch from 387d021 to 3147e67 Compare October 4, 2024 23:36
@github-actions
Copy link

github-actions bot commented Oct 7, 2024

This pull request has conflicts, please rebase.

MarcoFalke and others added 3 commits October 7, 2024 15:13
…alization

0d01272 build: don't use Boost multi_index serialization (fanquake)

Pull request description:

  We don't use the serialization or archiving facilities of multi_index.
  So globally disable support, which gives a minor improvement in build
  time, i.e less preprocessing work, given we don't link any Boost libs.

  See: https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/creation.html

  > Serialization capabilities are automatically provided by just linking with the appropriate Boost.Serialization library module: it is not necessary to explicitly include any header from Boost.Serialization, apart from those declaring the type of archive used in the process. If not used, however, serialization support can be disabled by globally defining the macro BOOST_MULTI_INDEX_DISABLE_SERIALIZATION. Disabling serialization for Boost.MultiIndex can yield a small improvement in build times, and may be necessary in those defective compilers that fail to correctly process Boost.Serialization headers.

ACKs for top commit:
  MarcoFalke:
    cr ACK 0d01272

Tree-SHA512: 87c664a2f142dc6b8f8598341f9829be3fda8cf614d73cc9a894c8033ee40c6daa9b50f4049ecb1f1e3aaf342568d9a5f5c65af1e04c36ee3a9cb46eca95767b
…tion to C++20

49a9091 build: Bump minimum required Boost to 1.73.0 to support C++20 (Hennadii Stepanov)

Pull request description:

  Boost versions <1.73 have C++20-specific bugs that were fixed in the following commits:
  - boostorg/signals2@15fcf21
  - boostorg/test@495c095

  I tested [`libboost1.71-dev`](https://packages.ubuntu.com/focal/libboost1.71-dev) in Ubuntu 20.04 and Boost 1.71, 1.72, 1.73 in our depends build system.

  Closes bitcoin#29063.

ACKs for top commit:
  fanquake:
    ACK 49a9091

Tree-SHA512: b8ebc08af85abfa3fda70961bd1136ee9e5149dd76a3f901e43acba624d231971873cba5cbf30837f9e5ab58790b8330f241a76cb76d8cf5dce5ad0cca33fba8
2484cac Add public Boost headers explicitly (Hennadii Stepanov)
fade2ad test: Avoid `BOOST_ASSERT` macro (Hennadii Stepanov)

Pull request description:

  To check symbols in the code base, run:
  ```
  git grep boost::multi_index::identity
  git grep boost::multi_index::indexed_by
  git grep boost::multi_index::tag
  git grep boost::make_tuple
  ```

  Hoping on the absence of conflicts with top-prio PRs :)

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 2484cac
  TheCharlatan:
    ACK 2484cac

Tree-SHA512: d122ab028eee76ee1c4609ed51ec8db0c8c768edcc2ff2c0e420a48e051aa71e99748cdb5d22985ae6d97c808c77c1a27561f0715f77b256f74c1c310b37694c
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK c0154c0

@PastaPastaPasta PastaPastaPasta requested a review from knst October 8, 2024 13:45
Copy link
Collaborator

@knst knst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK c0154c0

@PastaPastaPasta PastaPastaPasta merged commit 0c39d14 into dashpay:develop Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants