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

Sjors · 2024-04-22T11:00:18Z

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

My Intel iMac from 2017 runs macOS 13.6.6 and has clang 15.0.0. It's installed at /usr/bin/clang so not Homebrew.

Xcode 14.3.1 and newer ship with clang 15.0:
https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_(since_SwiftUI_framework)

The system requirements for that is macOS Ventura 13.0 or later: https://developer.apple.com/documentation/xcode-release-notes/xcode-14_3_1-release-notes#

Homebrew itself officially supports macOS 12 or later, but may still work on macOS 11: https://docs.brew.sh/Installation

Fwiw macOS 11 Big Sur last got an update in September 2023, so Apple has not entirely written it off: https://en.wikipedia.org/wiki/MacOS_Big_Sur

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

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

Sjors commented Apr 22, 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

Sjors commented Apr 22, 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