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.

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

ci: Use clang-15 and IWYU v0.19 in "tidy" task #26766

Closed
wants to merge 4 commits into from
Closed

ci: Use clang-15 and IWYU v0.19 in "tidy" task #26766

wants to merge 4 commits into from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Dec 28, 2022

There was a hope that the new IWYU v0.19 gets rid of the entire contrib/devtools/iwyu/bitcoin.core.imp. Alas...

Based on #26763.

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 28, 2022 •
edited
Loading

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

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #26859 (fuzz: extend ConsumeNetAddr() to return I2P and CJDNS addresses by vasild)
  • #26850 (ci: Stop and remove CI container by MarcoFalke)
  • #26763 (ci: Treat IWYU violations as errors by hebasto)
  • #26705 (clang-tidy: Check headers and fixes them by hebasto)
  • #25152 (refactor: Split util/system into exception, shell, and fs-specific files by Empact)

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 added the Tests label Dec 28, 2022
@fanquake
Copy link
Member

I don't think we want to hop to non-LTS releases in the CI unless it's really required: #26296 (comment).

@hebasto
Copy link
Member Author

hebasto commented Dec 28, 2022

I don't think we want to hop to non-LTS releases in the CI unless it's really required: #26296 (comment).

Agree. That's why this PR is a draft for now.

maflcko
maflcko reviewed Dec 29, 2022
View reviewed changes
.cirrus.yml
<< : *GLOBAL_TASK_TEMPLATE
container:
image: ubuntu:jammy
image: ubuntu:kinetic
Copy link
Member

Choose a reason for hiding this comment

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

An alternative would be to use https://packages.debian.org/bookworm/clang-15 ?

Copy link
Member

Choose a reason for hiding this comment

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

Another alternative would be to use https://apt.llvm.org/

@DrahtBot DrahtBot mentioned this pull request Dec 29, 2022
Merged
@hebasto hebasto mentioned this pull request Dec 29, 2022
Merged
maflcko pushed a commit to bitcoin-core/gui that referenced this pull request Dec 29, 2022
Merge bitcoin/bitcoin#26770: ci: Move --enable-c++20 from "tidy" ta…
65de8ee 
…sk back to "ASan..." one

afc6052 ci: Move `--enable-c++20` from "tidy" task back to "ASan..." one (Hennadii Stepanov)

Pull request description:

  This PR reverts cc7335e from bitcoin/bitcoin#25528 partially.

  C++20 has introduced some new headers, and it is premature to consider them when using the IWYU tool.

  Required for bitcoin/bitcoin#26763 and bitcoin/bitcoin#26766.

  Related discussions:
  - bitcoin/bitcoin#25528 (comment)
  - bitcoin/bitcoin#26763 (comment)

ACKs for top commit:
  MarcoFalke:
    review only ACK afc6052

Tree-SHA512: 9c1d3317d0f7a94d46d1e442da1a91669cd9ed1e62a41579edc96e1f5447551b536d32eeb222caf277e85228482753be545e0a874208cb24f9a8491fce3b6d9f
@DrahtBot DrahtBot mentioned this pull request Dec 30, 2022
Closed
This was referenced Jan 9, 2023
Merged
Closed
Merged
@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

@hebasto hebasto closed this Feb 2, 2023
@hebasto hebasto mentioned this pull request Mar 23, 2023
Merged
fanquake added a commit to bitcoin-core/gui that referenced this pull request Mar 23, 2023
@fanquake
Merge bitcoin/bitcoin#27311: ci: Use clang-15 in "tidy" task
f380bb9 
8fe27fb ci: Use clang-15 in "tidy" task (Hennadii Stepanov)

Pull request description:

  Newer tools usually are better in terms of features and bug fixes.

  Requested in bitcoin/bitcoin#26642 (comment).

  Split from bitcoin/bitcoin#26766.

ACKs for top commit:
  MarcoFalke:
    lgtm ACK 8fe27fb

Tree-SHA512: 62be3307d488fc4f75c40c0fa095aaa091aade2d5fe85296b56751e006c801f9d58c72c5cee8c0a0b1ba1a43804e315a3301c03e6e394bb3f3eb9b763fbb6271
@fanquake
Copy link
Member

Removing up for grabs here, as we are now using clang-16 and IWYU 0.20.

@hebasto hebasto mentioned this pull request May 21, 2023
Closed
@bitcoin bitcoin locked and limited conversation to collaborators Apr 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@maflcko maflcko maflcko left review comments

Assignees
No one assigned
Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hebasto @DrahtBot @fanquake @maflcko