Skip to content

Conversation

@compnerd
Copy link
Member

@compnerd compnerd commented Feb 5, 2020

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

Resolves SR-NNNN.

This simplifies the ICU flags handling.  This inlines the use into just
the library case, as the ICU usage is for the standard library targets
only.  It uses CMake to properly add the include paths as a system
search path.  This should fix the Linux builds as the include flags were
being de-duplicated as we are now using CMake more than we were
previously.
This should repair the Windows build after swiftlang#29451.  The quoting
behaviour was incorrect and was constructing an invalid compiler
invocation.  Solve the issue by using `target_include_directories`
instead.  However, since this needs the target, hoist the flag
computation to the local sites.  This replicates more logic because of
the custom build trying to replicate the CMake build logic in CMake.
@compnerd
Copy link
Member Author

compnerd commented Feb 5, 2020

@swift-ci please test tensorflow

@compnerd
Copy link
Member Author

compnerd commented Feb 5, 2020

This fixes the last merge which didn't catch the fixes after the tag

Copy link
Contributor

@dan-zheng dan-zheng left a comment

Choose a reason for hiding this comment

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

LGTM if this fixes the Windows build! I'm not sure how to verify that.

@compnerd compnerd merged commit 0e1f332 into swiftlang:tensorflow Feb 5, 2020
@compnerd compnerd deleted the tensorflow-windows-fixes branch February 5, 2020 22:28
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.

2 participants