Skip to content

build: treat ICU includes as system headers #6219

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

Merged
merged 1 commit into from
Dec 12, 2016

Conversation

compnerd
Copy link
Member

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

These headers should be treated as system headers as we do not control them.

Use target_include_directories so that we can use the SYSTEM option to
indicate that they are system headers. It also simplifies the invocation by not
directly modifying the underlying property.

These headers should be treated as system headers as we do not control them.

Use target_include_directories so that we can use the `SYSTEM` option to
indicate that they are system headers.  It also simplifies the invocation by not
directly modifying the underlying property.
@compnerd
Copy link
Member Author

CC @llvm-beanz @modocache @hughbe

@compnerd
Copy link
Member Author

@swift-ci please test

@hughbe
Copy link
Contributor

hughbe commented Dec 12, 2016

I tried something like this without the SYSTEM and it didn't work - I presume that's what makes this work.
Again, this will merge conflict with my windows port - we should get that merged soon after this one

@modocache
Copy link
Contributor

I tried this out, and it works for Android builds as well (after reverting 647c1d9, which as @hpux735 mentioned causes compilation errors on Android).

@compnerd compnerd merged commit 90b329b into swiftlang:master Dec 12, 2016
@compnerd compnerd deleted the icu-include-flags branch December 12, 2016 21:45
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.

4 participants