Progress towards CMake on Mac
Created attachment 263850 [details] Patch
Comment on attachment 263850 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263850&action=review > Source/WTF/ChangeLog:9 > + Add a space necessary for some files in WebKit1 compiled with -std=c99 with CMake. Isn't c++11 compulsory for all ports? How come it builds now and why should we change for Mac-cmake build?
Created attachment 263858 [details] Patch
(In reply to comment #2) > Comment on attachment 263850 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263850&action=review > > > Source/WTF/ChangeLog:9 > > + Add a space necessary for some files in WebKit1 compiled with -std=c99 with CMake. > > Isn't c++11 compulsory for all ports? How come it builds now and why should > we change for Mac-cmake build? I don't know why it builds now, but I think this space won't hurt anything, and there are definitely some C and ObjC files in WebKit1 that don't use C++11.
Comment on attachment 263858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263858&action=review > Source/WTF/ChangeLog:9 > + Add a space necessary for some files in WebKit1 compiled with -std=c99 with CMake. It seems odd to make the code uglier only for CMake build. If Xcode is able to build to existing code as is, I think we should get the CMake build to work with the existing code instead of updating the code.
Created attachment 263861 [details] Patch
All right, I dropped the WTF changes.
Comment on attachment 263861 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263861&action=review r=me with comment. > Source/WebKit2/UIProcess/Cocoa/WebProcessPoolCocoa.mm:56 > +#import "WebMemoryPressureHandlerIOS.h" Why is this needed? That header has already has #if PLATFORM(IOS).
(In reply to comment #8) > Comment on attachment 263861 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263861&action=review > > r=me with comment. > > > Source/WebKit2/UIProcess/Cocoa/WebProcessPoolCocoa.mm:56 > > +#import "WebMemoryPressureHandlerIOS.h" > > Why is this needed? That header has already has #if PLATFORM(IOS). It can't find the header. I don't want to add iOS include directories to the Mac CMake build.
http://trac.webkit.org/changeset/191473
Comment on attachment 263861 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263861&action=review > Source/WebKit2/CMakeLists.txt:776 > + "${WEBCORE_DIR}/page" I don't see why we need to add *page* include path again here, not 101 line. What problem ?
(In reply to comment #11) > Comment on attachment 263861 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263861&action=review > > > Source/WebKit2/CMakeLists.txt:776 > > + "${WEBCORE_DIR}/page" > > I don't see why we need to add *page* include path again here, not 101 line. > What problem ? I removed it in r191490 with build fix. If there is any problem, please let me know.
(In reply to comment #11) > Comment on attachment 263861 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263861&action=review > > > Source/WebKit2/CMakeLists.txt:776 > > + "${WEBCORE_DIR}/page" > > I don't see why we need to add *page* include path again here, not 101 line. > What problem ? WebKit2/UIProcess/Cocoa/DiagnosticLoggingClient.h needs to be included instead of WebCore/page/DiagnosticLoggingClient.h. I'll probably solve this by renaming one of the headers.
(In reply to comment #13) > (In reply to comment #11) > > Comment on attachment 263861 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=263861&action=review > > > > > Source/WebKit2/CMakeLists.txt:776 > > > + "${WEBCORE_DIR}/page" > > > > I don't see why we need to add *page* include path again here, not 101 line. > > What problem ? > WebKit2/UIProcess/Cocoa/DiagnosticLoggingClient.h needs to be included > instead of WebCore/page/DiagnosticLoggingClient.h. I'll probably solve this > by renaming one of the headers. I prefer renaming one of this header to depending on include path order.