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.

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

Mark include as PUBLIC not SYSTEM PUBLIC #1081

Merged
merged 3 commits into from
Jul 6, 2023
Merged

Conversation

skmcgrail
Copy link
Member

@skmcgrail skmcgrail commented Jul 5, 2023

Fixes an issue seen in aws-lc-rs where this would cause CMake to generate the makefile / ninja-build descriptions with the directory included using -isystem rather then -I. This affects resolution order for headers as system includes are ranked lower in priority then -I includes.

It seemed appropriate to fix this here, BoringSSL similarly has this marked as just public: https://github.com/google/boringssl/blob/master/crypto/CMakeLists.txt#L304

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@skmcgrail skmcgrail requested a review from andrewhop July 5, 2023 23:47
andrewhop
andrewhop previously approved these changes Jul 6, 2023
@skmcgrail skmcgrail requested a review from justsmth July 6, 2023 00:30
@skmcgrail skmcgrail enabled auto-merge (squash) July 6, 2023 18:47
@skmcgrail skmcgrail merged commit 51917f8 into aws:main Jul 6, 2023
skmcgrail added a commit to skmcgrail/aws-lc that referenced this pull request Jul 7, 2023
* Mark include as PUBLIC not SYSTEM PUBLIC
* Update ssl target

(cherry picked from commit 51917f8)
@skmcgrail skmcgrail mentioned this pull request Jul 7, 2023
@skmcgrail skmcgrail deleted the fixcmake branch August 1, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants