Bug 135528

Summary: progress towards cmake on mac
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: WebKit Misc.Assignee: Alex Christensen <achristensen>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, bunhere, clopez, cmarcelo, commit-queue, gyuyoung.kim, mrobinson, mrowe, oliver, rakuco, sergio
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch gyuyoung.kim: review+

Alex Christensen
Reported 2014-08-01 16:02:03 PDT
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>
Attachments
Patch (6.58 KB, patch)
2014-08-01 16:07 PDT, Alex Christensen
no flags
Patch (5.43 KB, patch)
2014-08-01 16:52 PDT, Alex Christensen
no flags
Patch (13.82 KB, patch)
2014-08-04 16:05 PDT, Alex Christensen
no flags
Patch (28.37 KB, patch)
2014-08-04 16:57 PDT, Alex Christensen
gyuyoung.kim: review+
Alex Christensen
Comment 1 2014-08-01 16:07:19 PDT
Martin Robinson
Comment 2 2014-08-01 16:08:35 PDT
Comment on attachment 235919 [details] Patch Where is APPLE defined or is it a CMake builtin?
Martin Robinson
Comment 3 2014-08-01 16:09:51 PDT
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)
Comment 4 2014-08-01 16:15:57 PDT
(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
Comment 5 2014-08-01 16:52:57 PDT
Alex Christensen
Comment 6 2014-08-04 16:05:03 PDT
Alex Christensen
Comment 7 2014-08-04 16:09:26 PDT
jsc compiles, links, and runs on Mac with this. No FTL yet :( I started into WebCore, but that's WIP.
Alex Christensen
Comment 8 2014-08-04 16:57:40 PDT
Gyuyoung Kim
Comment 9 2014-08-04 17:18:50 PDT
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
Comment 10 2014-08-04 17:31:05 PDT
Fixed some alphabetization and committed to http://trac.webkit.org/changeset/172014
Note You need to log in before you can comment on or make changes to this bug.