Skip to content

doc: Document minimum required libc++ version #22339

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.

Sign up for GitHub

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

Jump to bottom
Merged
fanquake merged 1 commit into from
Jun 28, 2021
Merged

doc: Document minimum required libc++ version #22339

fanquake merged 1 commit into from
Jun 28, 2021

Conversation

@hebasto
Copy link
Member

@hebasto hebasto commented Jun 25, 2021 •
edited
Loading

The minimum required libc++ version to compile the current master is 7.0.

See #22324.

@hebasto
Copy link
Member Author

hebasto commented Jun 25, 2021

Should this be tagged for 22.0?

@hebasto hebasto mentioned this pull request Jun 25, 2021
Merged
@fanquake fanquake added the Docs label Jun 25, 2021
fanquake
fanquake reviewed Jun 25, 2021
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

The libc++ link in in the OP goes to the libstdc++ manual.

doc/dependencies.md Outdated
| Berkeley DB | [4.8.30](https://www.oracle.com/technetwork/database/database-technologies/berkeleydb/downloads/index.html) | 4.8.x | No | | |
| Boost | [1.71.0](https://www.boost.org/users/download/) | [1.64.0](https://github.com/bitcoin/bitcoin/pull/22320) | No | | |
| Clang | | [5.0+](https://releases.llvm.org/download.html) (C++17 support) | | | |
| Clang | | [8.0+](https://releases.llvm.org/download.html) (C++17 support with P0083R3 proposal) | | | |
Copy link
Member

@fanquake fanquake Jun 25, 2021

Choose a reason for hiding this comment

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

Most Clang usage will still be on top of libstdc++, and it's still possible to compile master with Clang 5 and libstdc++ 7+. i.e on Bionic.

Copy link
Member Author

@hebasto hebasto Jun 25, 2021

Choose a reason for hiding this comment

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

... libstdc++ 7+. i.e on Bionic.

To be precise, 7.5.0.

Copy link
Member Author

@hebasto hebasto Jun 25, 2021

Choose a reason for hiding this comment

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

with Clang 5 ... on Bionic.

Do you mean clang 6.0?

Copy link
Member

@fanquake fanquake Jun 25, 2021

Choose a reason for hiding this comment

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

Do you mean clang 6.0?

No. I mean clang-5.0.

Copy link
Member Author

@hebasto hebasto Jun 25, 2021

Choose a reason for hiding this comment

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

Thanks! Updated.

@hebasto
Copy link
Member Author

hebasto commented Jun 25, 2021

The libc++ link in in the OP goes to the libstdc++ manual.

Thanks! Fixed.

@hebasto hebasto force-pushed the 210625-std branch 2 times, most recently from b9b9765 to 4ffb9f4 Compare June 25, 2021 05:46
@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 25, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

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.

@DrahtBot DrahtBot mentioned this pull request Jun 25, 2021
Closed
@sipa
Copy link
Member

sipa commented Jun 25, 2021

Oh, this function is just a convenience. It's easy to drop it (or provide a workaround).

@sipa sipa mentioned this pull request Jun 25, 2021
Merged
@sipa
Copy link
Member

sipa commented Jun 25, 2021

See #22342 for an alternative.

This was referenced Jun 25, 2021
Merged
Draft
@hebasto
Copy link
Member Author

hebasto commented Jun 26, 2021

Closed in favor of #22342.

@hebasto hebasto closed this Jun 26, 2021
@sipa sipa modified the milestone: 22.0 Jun 26, 2021
@bitcoin bitcoin deleted a comment from gremen1918 Jun 26, 2021
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Jun 26, 2021
Merge bitcoin#22342: Avoid the use of P0083R3 std::set::merge
9c3751a 
6cf4ea7 Avoid the use of P0083R3 std::set::merge (Pieter Wuille)

Pull request description:

  This use was introduced in bitcoin#21365, but as pointed out in bitcoin#22339, this causes compatibility problems.

  Just avoid its use for now.

ACKs for top commit:
  jonatack:
    re-ACK 6cf4ea7
  benthecarman:
    ACK 6cf4ea7
  hebasto:
    ACK 6cf4ea7, successfully compiled on the following systems:

Tree-SHA512: 2b3fdcadb7de98963ebb0b192bd956aa68526457fe5b374c74a69ea10d5b68902763148f11abbcc471010bcdc799e0804faef5f8e8ff8a509b3a053c0cb0ba39
@maflcko
Copy link
Member

maflcko commented Jun 26, 2021

Does it still make sense to document the minimum libc++ of 7.0? See #22324

@hebasto
Copy link
Member Author

hebasto commented Jun 26, 2021

Does it still make sense to document the minimum libc++ of 7.0? See #22324

Right! Wiil re-open and update accordingly.

@hebasto hebasto reopened this Jun 26, 2021
@hebasto hebasto marked this pull request as draft June 26, 2021 08:41
@hebasto hebasto changed the title doc: Update actual minimum supported compiler versions doc: Document minimum required libc++ version Jun 26, 2021
@hebasto hebasto marked this pull request as ready for review June 26, 2021 09:48
@hebasto
Copy link
Member Author

hebasto commented Jun 26, 2021

@MarcoFalke

Updated, renamed.

@hebasto hebasto mentioned this pull request Jun 26, 2021
Closed
@maflcko
Copy link
Member

maflcko commented Jun 26, 2021

cr ACK 22c398f

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 27, 2021
Merge bitcoin#22342: Avoid the use of P0083R3 std::set::merge
654f896 
6cf4ea7 Avoid the use of P0083R3 std::set::merge (Pieter Wuille)

Pull request description:

  This use was introduced in bitcoin#21365, but as pointed out in bitcoin#22339, this causes compatibility problems.

  Just avoid its use for now.

ACKs for top commit:
  jonatack:
    re-ACK 6cf4ea7
  benthecarman:
    ACK 6cf4ea7
  hebasto:
    ACK 6cf4ea7, successfully compiled on the following systems:

Tree-SHA512: 2b3fdcadb7de98963ebb0b192bd956aa68526457fe5b374c74a69ea10d5b68902763148f11abbcc471010bcdc799e0804faef5f8e8ff8a509b3a053c0cb0ba39
@fanquake
Copy link
Member

fanquake commented Jun 28, 2021

The commit message here needs changing. It mentions #21365 and requiring P0083R3, which is no-longer true, as that change has been reverted. It also doesn't change "minimum supported compiler versions", it's adding a note about libc++.

@hebasto
Copy link
Member Author

hebasto commented Jun 28, 2021

@fanquake

The commit message here needs changing. It mentions #21365 and requiring P0083R3, which is no-longer true, as that change has been reverted. It also doesn't change "minimum supported compiler versions", it's adding a note about libc++.

Thanks! Fixed.

fanquake
fanquake approved these changes Jun 28, 2021
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK 18c2027

@fanquake fanquake merged commit ac238f7 into bitcoin:master Jun 28, 2021
@hebasto hebasto deleted the 210625-std branch June 28, 2021 03:36
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 28, 2021
@fanquake
Merge bitcoin#22339: doc: Document minimum required libc++ version
94ab163
gwillen pushed a commit to ElementsProject/elements that referenced this pull request Jun 1, 2022
@apoelstra
Merge ac238f7 into merged_master (Bitcoin PR bitcoin/bitcoin#22339)
0c75e5a
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@fanquake fanquake fanquake approved these changes

Assignees

No one assigned

Labels

Docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@hebasto @DrahtBot @sipa @maflcko @fanquake