build: Bump clang minimum supported version to 15 #29165

maflcko · 2024-01-02T11:03:43Z

Most supported operating systems ship with clang-15 (or later), so bump the minimum to that and allow new code to drop workarounds for previous clang bugs.

For reference:

https://packages.debian.org/bookworm/clang-15
https://packages.ubuntu.com/jammy/clang-15
CentOS-like 8/9 Stream: All Clang versions from 15 to 17
FreeBSD 12/13: All Clang versions from 15 to 16
OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (clang17); No idea about OpenSuse Leap

On operating systems where the clang version is not shipped by default, the user would have to use GCC, or install clang in a different way. For example:

https://packages.debian.org/bullseye/g++ (g++-10)
https://packages.ubuntu.com/focal/g++-10
https://apt.llvm.org/, or nix, or guix, or compile clang from source, ...

DrahtBot · 2024-01-02T11:03:46Z

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	hebasto, fanquake

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#29077 ([WIP, DO NOT MERGE!!11111!!!!!] build: Require libc++-16 or later by maflcko)
#29071 (refactor: Remove Span operator==, Use std::ranges::equal by maflcko)
#28687 (C++20 std::views::reverse by stickies-v)
#25972 (build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set by fanquake)

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.

hebasto · 2024-01-02T11:13:53Z

maflcko · 2024-01-02T11:30:34Z

The macOS CI fails (as expected). Moving to draft for now, until someone clarifies the macOS clang situation. (I don't use macOS)

fanquake · 2024-01-02T11:47:09Z

The macOS CI fails (as expected). Moving to draft for now, until someone clarifies the macOS clang situation. (I don't use macOS)

Xcode 14.3.1 (currently used in the CI) is based on ~ LLVM 14. It's failing because removing the fuzz workaround requires LLVM/Clang 15 or later. The GH image ships with newer Xcodes, https://github.com/actions/runner-images/blob/main/images/macos/macos-13-Readme.md#xcode, so I assume we could switch to using Xcode 15.x + in the CI (based on LLVM 16.x).

The multiprocess build is also failing, due to what I assume is a Debian packaging issue.

fanquake · 2024-01-02T11:50:21Z

The libbitcoinkernel clang-15 libc++ job is also failing, so I guess this isn't actually fixed with clang-15, regardless of macOS?

.github/workflows/ci.yml

ci/test/00_setup_env_i686_multiprocess.sh

f8ca135 build: Fix check whether `-latomic` needed (Hennadii Stepanov) Pull request description: Clang >=15 still might need linking against `libatomic`. We use `std::atomic<std::chrono::seconds>::compare_exchange_strong` in `net_processing.cpp`. Addresses the bitcoin/bitcoin#29165 (comment). ACKs for top commit: maflcko: lgtm ACK f8ca135 fanquake: ACK f8ca135 Tree-SHA512: ba8b6a88fd3471a206d068e8a000a053c99cb46d26bd04624418ddb066b3b9664a569ec8a1569af67c96b3e27f13dccbd5e24f985290ac072b6d74c92524e35d

maflcko · 2024-04-22T11:05:00Z

Can you add a note to doc/build-osx.md that users of macOS < 13 need to install clang via homebrew?

##29918 is an unrelated issue existing on current master, so I think a separate pull request would be more appropriate. Also, I don't have or use macOS/Apple, so someone else creating that pull request would be beneficial.

Sjors · 2024-04-22T14:02:09Z

My remark is orthogonal to #29918. macOS < 13 doesn't have clang 15. They also don't have clang 14, which was missed in #29208.

There's need to make a separate pull request for a single sentence:

At the bottom of step 3:

On macOS < 13 you additionally need to install a more recent version of clang:

    brew install clang

maflcko · 2024-04-22T14:10:27Z

There's need to make a separate pull request for a single sentence:

I don't think this is enough. Is it not required to specify the CC and CXX to ./configure? Something like CC=$(brew --prefix llvm)/bin/clang CXX=$(brew --prefix llvm)/bin/clang++? Or does brew overwrite the clang system compiler?

Another reason to fix this in a separate pull is to have the fix in before this pull is merged (which at this point is unclear when it can happen).

Sjors · 2024-04-22T14:40:42Z

Done in #29934.

Correction to version support above: clang 15.0 is shipped with Xcode 15, not 14. But its system requirements are fairly similar, macOS 13.5 or better: https://developer.apple.com/documentation/xcode-release-notes/xcode-15-release-notes

I don't have a macOS 11 or 12 machine to actually test if it still works. Given that homebrew dropped explicit support for macOS 11 I don't think we need to worry about that version, but it would be nice if someone can test if they can still build (clang 15) on macOS 12.

maflcko · 2024-04-22T14:48:19Z

Thanks, I'll rebase once the other pull is merged.

hebasto

ACK fa1964c.

maflcko · 2024-04-27T14:37:54Z

Rebased and taken out of draft. I've opened two pull requests against oss-fuzz to bump the compiler there (one globally and one for bitcoin-core), but they are waiting for merge. If they are not merged, I think we can consider oss-fuzz as unsupported and drop it temporarily. Oss-fuzz is nice, but it seems odd to consider it a blocker here.

fanquake · 2024-04-29T02:08:48Z

Yea. I think we can merge our independant oss-fuzz bump, then merge this, then revert the oss-fuzz change if/when the global oss-fuzz bump happens. That has dragged on a bit too long, and is just blocking things here.

Sjors · 2024-04-29T08:30:42Z

@maflcko the updated wording in #29934 is such this PR won't conflict with it.

fanquake

ACK fa1964c - oss-fuzz LLVM will either be bumped globally tomorrow, or we'll land our own bump.

8fd40d4 fixup! ci: Test CMake edge cases (Hennadii Stepanov) c08acdb fixup! ci: Test CMake edge cases (Hennadii Stepanov) Pull request description: See bitcoin#29165. ACKs for top commit: m3dwards: utACK 8fd40d4 Tree-SHA512: 4f74730de2de7767875357044763f9aa73ba0e973a9af2572989f51bf631cd082279f08b7141ef8e56765cecf327ee31406bd83d05eb2a5125d520f773d0210f

fanquake · 2024-06-10T13:27:16Z

Release note added in #30261.

f8ca135 build: Fix check whether `-latomic` needed (Hennadii Stepanov) Pull request description: Clang >=15 still might need linking against `libatomic`. We use `std::atomic<std::chrono::seconds>::compare_exchange_strong` in `net_processing.cpp`. Addresses the bitcoin#29165 (comment). ACKs for top commit: maflcko: lgtm ACK f8ca135 fanquake: ACK f8ca135 Tree-SHA512: ba8b6a88fd3471a206d068e8a000a053c99cb46d26bd04624418ddb066b3b9664a569ec8a1569af67c96b3e27f13dccbd5e24f985290ac072b6d74c92524e35d

, bitcoin#24337, bitcoin#27682, bitcoin#29208, bitcoin#29091, bitcoin#29165, bitcoin#29934, bitcoin#30261, partial bitcoin#27662, bitcoin#28210, bitcoin#28348, bitcoin#30263 (bump minimum compiler) 0861692 partial bitcoin#30263: Bump clang minimum supported version to 16 (Kittywhiskers Van Gogh) 5e7e563 merge bitcoin#30261: add release note for 29091 and 29165 (Kittywhiskers Van Gogh) 4d10993 merge bitcoin#29934: add LLVM instruction for macOS < 13 (Kittywhiskers Van Gogh) 7e4614a merge bitcoin#29165: Bump clang minimum supported version to 15 (Kittywhiskers Van Gogh) f397076 merge bitcoin#29091: Bump g++ minimum supported version to 11 (Kittywhiskers Van Gogh) 35d3357 merge bitcoin#29208: Bump clang minimum supported version to 14 (Kittywhiskers Van Gogh) c71e3df partial bitcoin#28348: Bump g++ minimum supported version to 10 (Kittywhiskers Van Gogh) b7da1ed partial bitcoin#28210: Bump clang minimum supported version to 13 (Kittywhiskers Van Gogh) 6b22832 partial bitcoin#27662: Bump minimum supported GCC to g++-9 (Kittywhiskers Van Gogh) f677769 merge bitcoin#27682: Bump minimum supported Clang to clang-10 (Kittywhiskers Van Gogh) f8e0339 docs: add `libgmp` as an optional dependency (Kittywhiskers Van Gogh) ebba607 docs: update version used to match `depends` (Kittywhiskers Van Gogh) 39b6f4b merge bitcoin#24337: Do not define `PROVIDE_FUZZ_MAIN_FUNCTION` macro unconditionally (Kittywhiskers Van Gogh) 2bd8422 merge bitcoin#23565: rewrite dependencies.md (Kittywhiskers Van Gogh) 56989ec merge bitcoin#24164: Bump minimum required clang/libc++ to 8.0 (Kittywhiskers Van Gogh) bac407f merge bitcoin#23060: increase minimum compiler and lib(std)c++ requirements (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #6516 ## Breaking Changes GCC 11.1 or later, or Clang 16.0 or later, are now required to compile Dash Core. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)** - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK 0861692 UdjinM6: utACK 0861692 Tree-SHA512: 6de3f8153482b4f379fd397dd3000688632356299c4f13a2f8af20d2f7337318bd3cd0b96182d6f378846b14981bc499ea32aa3d20cba6a0c7cf5f2a6e151937

DrahtBot added the Build system label Jan 2, 2024

maflcko force-pushed the 2401-clang-15- branch from b08682d to eac1a46 Compare January 2, 2024 11:07

maflcko marked this pull request as draft January 2, 2024 11:30

maflcko force-pushed the 2401-clang-15- branch from eac1a46 to 7825898 Compare January 2, 2024 11:57

DrahtBot added the CI failed label Jan 2, 2024

maflcko force-pushed the 2401-clang-15- branch from 7825898 to e58e752 Compare January 2, 2024 12:02

hebasto reviewed Jan 2, 2024

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

maflcko force-pushed the 2401-clang-15- branch 7 times, most recently from 7acb570 to fa88791 Compare January 2, 2024 16:15

DrahtBot removed the CI failed label Jan 2, 2024

This was referenced Jan 2, 2024

refactor: Use std::span over Span #29119

Closed

build: Bump g++ minimum supported version to 11 #29091

Merged

build: Require libc++-16 or later #29077

Closed

refactor: Remove Span operator==, Use std::ranges::equal #29071

Merged

maflcko marked this pull request as ready for review January 3, 2024 09:32

maflcko force-pushed the 2401-clang-15- branch from fa88791 to fa224a2 Compare January 3, 2024 09:32

fanquake reviewed Jan 3, 2024

View reviewed changes

ci/test/00_setup_env_i686_multiprocess.sh Outdated Show resolved Hide resolved

This was referenced Jan 4, 2024

Broken --enable-suppress-external-warning for Apple Clang 15 on x86_64 #29174

Closed

build: Fix check whether -latomic needed #29177

Merged

maflcko marked this pull request as ready for review April 27, 2024 12:53

build: Bump clang minimum supported version to 15

fa1964c

maflcko force-pushed the 2401-clang-15- branch from eeb25ba to fa1964c Compare April 27, 2024 12:54

hebasto approved these changes Apr 27, 2024

View reviewed changes

DrahtBot removed the CI failed label Apr 27, 2024

fanquake added the Needs release note label Apr 29, 2024

fanquake approved these changes Apr 29, 2024

View reviewed changes

fanquake merged commit 4d2d91a into bitcoin:master Apr 29, 2024
16 checks passed

hebasto mentioned this pull request Apr 29, 2024

cmake, ci: Bump clang version to 15 hebasto/bitcoin#159

Merged

maflcko deleted the 2401-clang-15- branch April 29, 2024 14:31

fanquake removed the Needs release note label Jun 10, 2024

kwvg added a commit to kwvg/dash that referenced this pull request Nov 10, 2024

merge bitcoin#29165: Bump clang minimum supported version to 15

c0806ea

kwvg added a commit to kwvg/dash that referenced this pull request Jan 7, 2025

merge bitcoin#29165: Bump clang minimum supported version to 15

17b6921

kwvg added a commit to kwvg/dash that referenced this pull request Feb 5, 2025

merge bitcoin#29165: Bump clang minimum supported version to 15

7e4614a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Bump clang minimum supported version to 15 #29165

build: Bump clang minimum supported version to 15 #29165

maflcko commented Jan 2, 2024 •

edited

Loading

DrahtBot commented Jan 2, 2024 •

edited

Loading

hebasto commented Jan 2, 2024

maflcko commented Jan 2, 2024

fanquake commented Jan 2, 2024

fanquake commented Jan 2, 2024

maflcko commented Apr 22, 2024

Sjors commented Apr 22, 2024

maflcko commented Apr 22, 2024

Sjors commented Apr 22, 2024

maflcko commented Apr 22, 2024

hebasto left a comment

maflcko commented Apr 27, 2024

fanquake commented Apr 29, 2024

Sjors commented Apr 29, 2024

fanquake left a comment

fanquake commented Jun 10, 2024

build: Bump clang minimum supported version to 15 #29165

build: Bump clang minimum supported version to 15 #29165

Conversation

maflcko commented Jan 2, 2024 • edited Loading

DrahtBot commented Jan 2, 2024 • edited Loading

Code Coverage

Reviews

Conflicts

hebasto commented Jan 2, 2024

maflcko commented Jan 2, 2024

fanquake commented Jan 2, 2024

fanquake commented Jan 2, 2024

maflcko commented Apr 22, 2024

Sjors commented Apr 22, 2024

maflcko commented Apr 22, 2024

Sjors commented Apr 22, 2024

maflcko commented Apr 22, 2024

hebasto left a comment

Choose a reason for hiding this comment

maflcko commented Apr 27, 2024

fanquake commented Apr 29, 2024

Sjors commented Apr 29, 2024

fanquake left a comment

Choose a reason for hiding this comment

fanquake commented Jun 10, 2024

maflcko commented Jan 2, 2024 •

edited

Loading

DrahtBot commented Jan 2, 2024 •

edited

Loading