Skip to content

build: Fix -Xclang -internal-isystem option #29195

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
merged 2 commits into from
Jan 9, 2024
Merged

build: Fix -Xclang -internal-isystem option #29195

merged 2 commits into from
Jan 9, 2024

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jan 7, 2024 •
edited
Loading

@hebasto
build: Fix -Xclang -internal-isystem option
8decc5c 
LLVM Clang >=16.0 and Apple Clang >=15.0 do not recognize
`-Xclang -internal-isystem/usr/local/include` anymore.

For example, see: llvm/llvm-project@cbbe1d4
@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 7, 2024 •
edited
Loading

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 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:

  • #29165 (build: Bump clang minimum supported version to 15 by maflcko)

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 Jan 7, 2024
@hebasto
Copy link
Member Author

hebasto commented Jan 7, 2024

https://github.com/bitcoin/bitcoin/actions/runs/7437962301/job/20236177232:

checking whether C++ preprocessor accepts -Xclang -internal-isystem -Xclang /usr/local/include/... yes

This was referenced Jan 7, 2024
Closed
Merged
@maflcko maflcko removed the Tests label Jan 8, 2024
@maflcko
Copy link
Member

maflcko commented Jan 8, 2024

pull title should be "build: Fix -Xclang -internal-isystem option"? Otherwise lgtm

@hebasto hebasto changed the title ci: Switch native macOS CI job to Xcode 15.0 build: Fix -Xclang -internal-isystem option Jan 8, 2024
@hebasto hebasto changed the title build: Fix -Xclang -internal-isystem option build: Fix -Xclang -internal-isystem option Jan 8, 2024
@hebasto
Copy link
Member Author

hebasto commented Jan 8, 2024

pull title should be "build: Fix -Xclang -internal-isystem option"? Otherwise lgtm

Done.

@fanquake
Copy link
Member

fanquake commented Jan 8, 2024

The same Xcode version is used for the release binaries.

Can probably drop this, or be more specific, because we don't use Xcode to build release binaries (only the SDK, which is versioned differently).

@hebasto
Copy link
Member Author

hebasto commented Jan 8, 2024

The same Xcode version is used for the release binaries.

Can probably drop this, or be more specific, because we don't use Xcode to build release binaries (only the SDK, which is versioned differently).

Dropped.

fanquake
fanquake approved these changes Jan 9, 2024
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 d742be3. The same as what was done in #27328.

For example, see: llvm/llvm-project@cbbe1d4

I guess this is meant to be an example of someone else using -Xclang? However this commit is from > 10 years ago, and isn't related to, or an explanation for the recent change in Clang behaviour.

@hebasto
Copy link
Member Author

hebasto commented Jan 9, 2024

I guess this is meant to be an example of someone else using -Xclang?

Right, just an example. I forgot about #27328.

@fanquake fanquake merged commit 9e1306f into bitcoin:master Jan 9, 2024
@hebasto hebasto deleted the 240107-clang branch January 9, 2024 14:20
@hebasto hebasto mentioned this pull request Feb 16, 2024
Merged
glozow pushed a commit to glozow/bitcoin that referenced this pull request Feb 16, 2024
@hebasto @glozow
build: Fix -Xclang -internal-isystem option
72eab1c 
LLVM Clang >=16.0 and Apple Clang >=15.0 do not recognize
`-Xclang -internal-isystem/usr/local/include` anymore.

For example, see: llvm/llvm-project@cbbe1d4

Github-Pull: bitcoin#29195
Rebased-From: 8decc5c
glozow pushed a commit to glozow/bitcoin that referenced this pull request Feb 16, 2024
@hebasto @glozow
ci: Switch native macOS CI job to Xcode 15.0
40e7693 
Github-Pull: bitcoin#29195
Rebased-From: d742be3
@hebasto hebasto mentioned this pull request May 26, 2024
Merged
hebasto added a commit to bitcoin-core/gui-qml that referenced this pull request May 27, 2024
@hebasto
Merge #402: ci, build: Update GHA actions and fix build bugs
cf820e5 
a485533 build: Re-add `qt/guiconstants.h` header (Hennadii Stepanov)
87909c1 build: Add missed `IPAddressValueInput.qml` to `QML_RES_QML` (Hennadii Stepanov)
935ee29 build: Fix `-Xclang -internal-isystem` option (Hennadii Stepanov)
ac32ee9 ci: Update GHA actions (Hennadii Stepanov)

Pull request description:

  The second commit has been cherry-picked from bitcoin/bitcoin#29195.

  Link to github actions build artifacts:

  [![Build Artifacts](https://img.shields.io/badge/Build%20Artifacts-green
  )](https://github.com/bitcoin-core/gui-qml/actions/runs/9242930899)

ACKs for top commit:
  pablomartin4btc:
    ACK a485533
  johnny9:
    ACK a485533

Tree-SHA512: ca5222033d4fd1bae3a128f8037187cd5d1d0c3a262a3034dda3d88b22463342016a0162e5ca9415a9db76a6dde8c205a183edf7cbb0e1dd722b4742d05ce9b2
@bitcoin bitcoin locked and limited conversation to collaborators Jan 8, 2025
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
Build system macOS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Broken --enable-suppress-external-warning for Apple Clang 15 on x86_64
4 participants
@hebasto @DrahtBot @maflcko @fanquake