Bug 135528 – progress towards cmake on mac

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Alex Christensen]

Here's some more work to review.  I'm not sure if copying all the API headers to the derived sources directory is similar to anything else that is done or if there's a better place for them, but they need to be copied to somewhere to be accessed by #include <JavaScriptCore/APIHeader.h>

[Alex Christensen]

Created attachment 235919 [details]
Patch

[Martin Robinson]

Comment on attachment 235919 [details]
Patch

Where is APPLE defined or is it a CMake builtin?

[Martin Robinson]

Comment on attachment 235919 [details]
Patch

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

> Source/JavaScriptCore/CMakeLists.txt:596
> +if (APPLE)
> +    list(APPEND JavaScriptCore_SOURCES
> +        API/JSAPIWrapperObject.mm
> +        API/JSContext.mm
> +        API/JSManagedValue.mm
> +        API/JSStringRefCF.cpp
> +        API/JSValue.mm
> +        API/JSVirtualMachine.mm
> +        API/JSWrapperMap.mm
> +        API/ObjCCallbackFunction.mm
>      )

If these are specific to the Mac port they should probably be moved to the PlatformMac.cmake file.

> Source/WTF/wtf/CMakeLists.txt:269
> +if (APPLE)
> +    list(APPEND WTF_SOURCES
> +        text/cf/AtomicStringCF.cpp
> +        text/cf/StringCF.cpp
> +        text/cf/StringImplCF.cpp
> +        text/cf/StringViewCF.cpp
> +        text/mac/StringMac.mm
> +        text/mac/StringImplMac.mm
> +        text/mac/StringViewObjC.mm
> +        mac/MainThreadMac.mm
> +        mac/DeprecatedSymbolsUsedBySafari.mm
> +    )
> +endif ()

Ditto. Platform-specific source files should go in the Platform*.cmake files.

[Mark Rowe (bdash)]

(In reply to comment #0)
> Here's some more work to review.  I'm not sure if copying all the API headers to the derived sources directory is similar to anything else that is done or if there's a better place for them, but they need to be copied to somewhere to be accessed by #include <JavaScriptCore/APIHeader.h>

Shouldn't the API headers be copied in to the framework wrapper and accessed from there?

[Alex Christensen]

Created attachment 235924 [details]
Patch

[Alex Christensen]

Created attachment 235988 [details]
Patch

[Alex Christensen]

jsc compiles, links, and runs on Mac with this.  No FTL yet :(
I started into WebCore, but that's WIP.

[Alex Christensen]

Created attachment 235990 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 235990 [details]
Patch

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 ?

[Alex Christensen]

Fixed some alphabetization and committed to http://trac.webkit.org/changeset/172014