[webkit-reviews] review granted: [Bug 135528] progress towards cmake on mac : [Attachment 235990] Patch

[bugzilla-daemon at webkit.org]

Gyuyoung Kim <gyuyoung.kim at webkit.org> has granted Alex Christensen
<achristensen at apple.com>'s request for review:
Bug 135528: progress towards cmake on mac
https://bugs.webkit.org/show_bug.cgi?id=135528

Attachment 235990: Patch
https://bugs.webkit.org/attachment.cgi?id=235990&action=review

------- Additional Comments from Gyuyoung Kim <gyuyoung.kim at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=235990&action=review


As an initial patch for supporting CMake on mac, I think there is no critical
issue except for minor nits.

BTW, don't you need to add PlatformMac.cmake for Source/WebKit2 dir ?

> Source/JavaScriptCore/CMakeLists.txt:17
> +    "${JAVASCRIPTCORE_DIR}/inspector/remote"

Wrong alphabet order.

> Source/JavaScriptCore/CMakeLists.txt:32
> +    "${DERIVED_SOURCES_DIR}/ForwardingHeaders"

ditto ?

> Source/JavaScriptCore/CMakeLists.txt:584
> +	   disassembler/UDis86Disassembler.cpp

Isn't upper case placed to top side ?

> Source/JavaScriptCore/PlatformMac.cmake:6
> +    ${COCOA_LIBRARY}

ditto.

> Source/cmake/OptionsMac.cmake:32
> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_CSS3_TEXT_LINE_BREAK OFF)

ditto.

> Source/cmake/OptionsMac.cmake:41
> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_DEVICE_ORIENTATION_iphoneos ON)

As far as I know, we have defined option values in
Source/cmake/WebKitFeatures.cmake, then use it for each port. Can these mac
port specific values be enabled when you build on mac ?