Realize full Mac CMake build
Created attachment 441239 [details] Patch
It lives! (Patch is based off of the one in bug 225070.)
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Created attachment 441272 [details] Patch
This certainly can't land with the #if 1 in Tools/MiniBrowser/mac/WK1BrowserWindowController.h, but the error there is weird, as its #imports of WKL things are conflicting with WebKit.framework headers: > In file included from .../Tools/MiniBrowser/mac/WK1BrowserWindowController.m:34: > In file included from .../WebKitBuild/Release/WebKitLegacy/Headers/WebKitLegacy/WebPreferencesPrivate.h:1: > In file included from .../Source/WebKitLegacy/mac/WebView/WebPreferencesPrivate.h:29: > In file included from .../WebKitBuild/Release/WebKitLegacy/Headers/WebKitLegacy/WebPreferences.h:1: > .../Source/WebKitLegacy/mac/WebView/WebPreferences.h:167:38: error: property has a previous declaration > @property (nonatomic, strong) NSURL *userStyleSheetLocation; > ^ > /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/WebKit.framework/Headers/WebPreferences.h:167:38: note: property declared here > @property (nonatomic, strong) NSURL *userStyleSheetLocation; > ^
Comment on attachment 441272 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441272&action=review > Source/JavaScriptCore/PlatformMac.cmake:-40 > -set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS} "-compatibility_version 1 -current_version ${WEBKIT_MAC_VERSION} -force_load ${CMAKE_BINARY_DIR}/lib/libWTF.a -force_load ${CMAKE_BINARY_DIR}/lib/libbmalloc.a") As strange as it is, this needs to stay. > Source/WebCore/PlatformMac.cmake:-941 > -set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS} "-compatibility_version 1 -current_version ${WEBKIT_MAC_VERSION} -force_load ${CMAKE_BINARY_DIR}/lib/libPAL.a") ditto > Tools/WebKitTestRunner/mac/TestControllerMac.mm:73 > +static NSMenu *gCurrentPopUpMenu = nil; This could be a WeakObjCPtr
Created attachment 441276 [details] Patch
Okay, so judging from EWS, the Tools changes I was working with might have been based on false assumptions, so I'm stashing those for now and we can focus on just having WebCore and WebKit fully buildable for this patch.
(In reply to Alex Christensen from comment #6) > Comment on attachment 441272 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=441272&action=review > > > Source/JavaScriptCore/PlatformMac.cmake:-40 > > -set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS} "-compatibility_version 1 -current_version ${WEBKIT_MAC_VERSION} -force_load ${CMAKE_BINARY_DIR}/lib/libWTF.a -force_load ${CMAKE_BINARY_DIR}/lib/libbmalloc.a") > > As strange as it is, this needs to stay. > > > Source/WebCore/PlatformMac.cmake:-941 > > -set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS} "-compatibility_version 1 -current_version ${WEBKIT_MAC_VERSION} -force_load ${CMAKE_BINARY_DIR}/lib/libPAL.a") > > ditto The OBJECT libraries are a cross platform way of doing GCC's `--whole-archive` flag, which `-force_load` is roughly equivalent to. When there's an OBJECT library it just builds out all the .o files. Then in this case the .o files for bmalloc and WTF get linked into JavaScriptCore and the .o files for PAL get linked into WebCore. If this behavior wasn't working then there would be linker errors.
Comment on attachment 441276 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441276&action=review > Source/cmake/WebKitMacros.cmake:245 > + if (APPLE) Ross can you add a FIXME here for https://bugs.webkit.org/show_bug.cgi?id=231774 This was done to prevent WebCore from ending up in the `_LIBRARIES` when using the `-umbrella` flag which was causing a linker error but I have some ideas on how to better handle this which I want to try out in that linked bug.
Created attachment 441294 [details] Patch
Comment on attachment 441294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441294&action=review > Source/WebCore/Modules/applepay/ApplePaySetupWebCore.h:30 > +#include "ActiveDOMObject.h" ApplePaySetupWebCore.h is a private header, so this is still not OK.
Comment on attachment 441294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441294&action=review >> Source/WebCore/Modules/applepay/ApplePaySetupWebCore.h:30 >> +#include "ActiveDOMObject.h" > > ApplePaySetupWebCore.h is a private header, so this is still not OK. Its my understanding that WebCore doesn't have any public headers only private so this would be a valid change. Did something change here?
"Private" means that it's not part of public Xcode SDK, but unlike "Project" headers, these are part of internal SDK. I do see that there is another non-framework-style include next to this one, and I don't know why exactly this works, but it's still a problem with CMake build as proposed if it requires to use the wrong include form.
This is in fact the *only* file under WebCore/Modules with a `<WebCore/...>` include, so it certainly looks like a mistake...
(In reply to Ross Kirsling from comment #15) > This is in fact the *only* file under WebCore/Modules with a `<WebCore/...>` > include, so it certainly looks like a mistake... In fact, it and testing/cocoa/WebViewVisualIdentificationOverlay.h are the only files in the whole of WebCore which are doing this in the Mac build. My understanding is that a <WebCore/...> include is only used to consume a framework from the outside, so fixing this syntax should be desirable even without talking about CMake.
Created attachment 441436 [details] Patch
I have split out source changes from this patch; we can continue that discussion in a separate non-CMake-related ticket.
Created attachment 441439 [details] Patch
Comment on attachment 441439 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=441439&action=review > Source/WebKit/ChangeLog:11 > + * SourcesCocoa.txt: > + * WebKit.xcodeproj/project.pbxproj: Could you please add an explanation for this change? As it's the only non-CMake change here, I'm just curious.
(In reply to Alexey Proskuryakov from comment #20) > Comment on attachment 441439 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=441439&action=review > > > Source/WebKit/ChangeLog:11 > > + * SourcesCocoa.txt: > > + * WebKit.xcodeproj/project.pbxproj: > > Could you please add an explanation for this change? As it's the only > non-CMake change here, I'm just curious. Platform/IPC/cocoa/ sources are listed in SourcesCocoa.txt except for this one which was added as "Sources" in Xcode instead. A hackier temporary solution would be to list this one file in PlatformMac.cmake instead, to avoid editing the xcodeproj -- I suppose I could do that for this patch, because there are other files which ought to be moved similarly in a subsequent patch.
Created attachment 441456 [details] Patch for landing
Committed r284298 (243096@main): <https://commits.webkit.org/243096@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441456 [details].