[webkit-reviews] review requested: [Bug 196655] [CMake][WinCairo] Separate copied headers into different directories : [Attachment 366843] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Apr 8 15:09:11 PDT 2019


Konstantin Tokarev <annulen at yandex.ru> has asked  for review:
Bug 196655: [CMake][WinCairo] Separate copied headers into different
directories
https://bugs.webkit.org/show_bug.cgi?id=196655

Attachment 366843: Patch

https://bugs.webkit.org/attachment.cgi?id=366843&action=review




--- Comment #5 from Konstantin Tokarev <annulen at yandex.ru> ---
Comment on attachment 366843
  --> https://bugs.webkit.org/attachment.cgi?id=366843
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366843&action=review

> Source/JavaScriptCore/shell/PlatformWin.cmake:1
> +include_directories(./ PRIVATE ${JavaScriptCore_INCLUDE_DIRECTORIES}
${JavaScriptCore_PRIVATE_INCLUDE_DIRECTORIES}
${JavaScriptCore_PRIVATE_FRAMEWORK_HEADERS_DIR})

When we move to target-oriented project, all include_directories() and similar
directory-level directives shall be gone

> Source/cmake/OptionsWinCairo.cmake:42
> +set(WTF_FRAMEWORK_HEADERS_DIR
${CMAKE_BINARY_DIR}/FrameworkHeaders/WTF.framework/Headers)

I don't think we should use directory names with ".framework" extension here,
because this just leads to confusion and achieves nothing. On macOS and iOS
framework directories are created by built-in CMake features, and on other
platforms such "framework-like" bundles cannot act in macOS-like way

> Source/cmake/OptionsWinCairo.cmake:44
> +set(JavaScriptCore_PRIVATE_FRAMEWORK_HEADERS_DIR
${CMAKE_BINARY_DIR}/FrameworkHeaders/JavaScriptCore.framework/PrivateHeaders)

Now I see what did you mean with "FRAMEWORK" word in variable name, these
headers are logically INTERFACE, but not intended for all users. Are there any
targets inside WebKit tree which include only one of these header sets?


More information about the webkit-reviews mailing list