Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: build on ubuntu 22.10 due to new gcc 12 #5064

Merged
merged 3 commits into from
Nov 2, 2022

Conversation

knst
Copy link
Collaborator

@knst knst commented Oct 25, 2022

Issue being fixed or feature implemented

It fixes issues with build dependencies code on ubuntu 22.10 with new gcc 12

What was done?

2 backports have been merged: bitcoin#25436 bitcoin#17008
Afterwards need to update bls-dash also to this version: dashpay/bls-signatures#48 or this version: #5021 (or both depends on dash version)

How Has This Been Tested?

Built on Kubuntu 22.10 and run

Breaking Changes

no breaking changes

Checklist:

  • I have performed a self-review of my own code

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@knst knst modified the milestones: 18.1, 19 Oct 25, 2022
@knst knst marked this pull request as ready for review October 25, 2022 09:38
@UdjinM6 UdjinM6 modified the milestones: 19, 18.2 Oct 25, 2022
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.

pls see 096b148 and 9f48187

@knst
Copy link
Collaborator Author

knst commented Oct 25, 2022

pls see 096b148 and 9f48187

sorry, had uncommitted part of cherry-pick after conflict resolving. Should be fine now.

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.

tsan test job fails https://gitlab.com/dashpay/dash/-/jobs/3238658053, you need 20745 to "fix" it it seems (pls see da942c3 https://gitlab.com/UdjinM6/dash/-/jobs/3245134040)

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

@Jessicadrichardson
Copy link

Change approved

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for merging via merge commit (ci is happy)

laanwj and others added 3 commits November 2, 2022 10:24
880d4aa build: use BOOST_NO_CXX98_FUNCTION_BASE to suppress warnings (fanquake)
1bdbbbd build: suppress array-bounds errors in libxkbcommon (fanquake)

Pull request description:

  2 changes to better support building with GCC 12, which out of the box, is currently broken if you want to build using depends.
  Prevent `-Warray-bounds` errors when building libxkbcommon. i.e:
  ```bash
  src/xkbcomp/ast-build.c:82:27: error: array subscript 'ExprDef[0]' is partly outside array bounds of 'unsigned char[32]' [-Werror=array-bounds]
     82 |     expr->expr.value_type = type;
        |     ~~~~~~~~~~~~~~~~~~~~~~^~~~~~
  src/xkbcomp/ast-build.c:75:21: note: object of size 32 allocated by 'malloc'
     75 |     ExprDef *expr = malloc(size);
        |                     ^~~~~~~~~~~~
  ```

  It might be the case that these would be fixed by updating the
  package, but that would also require installing new build tools (meson),
  as well as potentially more dependencies (wayland), and it'd need
  testing with Qt. For now, just turn the errors into wanrings.

  Define `BOOST_NO_CXX98_FUNCTION_BASE` to prevent GCC warning about the use of `std::unary_function`. i.e:
  ```bash
  /bitcoin/depends/aarch64-unknown-linux-gnu/include/boost/container_hash/hash.hpp:131:33:
  warning: 'template<class _Arg, class _Result> struct std::unary_function' is deprecated [-Wdeprecated-declarations]
    131 |         struct hash_base : std::unary_function<T, std::size_t> {};
        |                                 ^~~~~~~~~~~~~~
  In file included from /usr/include/c++/12/bits/unique_ptr.h:37,
                   from /usr/include/c++/12/memory:76,
                   from ./init.h:10,
                   from init.cpp:10:
  /usr/include/c++/12/bits/stl_function.h:117:12: note: declared here
    117 |     struct unary_function
  ```

  Boost `container_hash` (included via functional -> multi_index) uses
  [`std::unary_function`, which was deprecated in C++11](https://en.cppreference.com/w/cpp/utility/functional/unary_function), and "removed" in
  C++17. It's use causes warnings with newer compilers, i.e GCC 12.1.

  Use the MACRO outlined in boostorg/container_hash#22, and added to Boost Config for GCC 12 in boostorg/config#430, to prevent it's use.

  [BOOST_NO_CXX98_FUNCTION_BASE](https://www.boost.org/doc/libs/master/libs/config/doc/html/boost_config/boost_macro_reference.html):
  > The standard library no longer supports std::unary_function and std::binary_function.
  > They were deprecated in C++11 and is removed from C++14.

  Guix Build (x86_64):
  ```bash

  ```

  Guix Build (arm64):
  ```bash

  ```

ACKs for top commit:
  laanwj:
    Code review ACK 880d4aa

Tree-SHA512: 10c4679c3eb788e9279acc4960731c55ae1568bd3df525d3c46f97d8b0319e7d8450b1638b6777d98111b5991dba5c787e95d80b1ac932e0b4779d4b8e74875e
02ac445 bump libevent to 2.1.11 in depends (stefanwouldgo)

Pull request description:

  this doesn't need patches on Android anymore like 2.1.8 did.

ACKs for top commit:
  laanwj:
    ACK 02ac445

Tree-SHA512: 1fbfe342ee15fa4c5cb417979bd6c443f7c7aa40a489accf8ccd7c919e5b08e859b3da6edeee3de484f6f156b35dd4e97c7e2c7971b59fc31029865585ccb296
d71e29e qa: Correct epoll_ctl data race suppression (Hennadii Stepanov)

Pull request description:

  Fixup of bitcoin#20218. Comments must start from the beginning of the line.

ACKs for top commit:
  MarcoFalke:
    review ACK d71e29e

Tree-SHA512: 4d8663ab505c347bcb62c2f118656e3343d5179825be0d1b86761ffdfdae1e7462002bf226a54dfc94be5885ce7f2633abaf70421ea35bf06eddad8e99fb9683
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.

5 participants