RESOLVED FIXED 62260
[CMAKE][WK2] Add an option to build webkit2.
https://bugs.webkit.org/show_bug.cgi?id=62260
Summary [CMAKE][WK2] Add an option to build webkit2.
Ryuan Choi
Reported 2011-06-07 19:42:44 PDT
This is split by Bug 61999 and covers platform independent CMake build script.
Attachments
Patch (23.52 KB, patch)
2011-06-07 23:23 PDT, Ryuan Choi
no flags
Patch (24.11 KB, patch)
2011-06-08 05:37 PDT, Ryuan Choi
no flags
Patch (23.60 KB, patch)
2011-06-08 08:28 PDT, Ryuan Choi
no flags
Patch (23.41 KB, patch)
2011-06-09 02:01 PDT, Ryuan Choi
no flags
Patch (22.93 KB, patch)
2011-06-09 17:27 PDT, Ryuan Choi
no flags
Patch (22.96 KB, patch)
2011-06-12 07:20 PDT, Ryuan Choi
no flags
Patch (23.03 KB, patch)
2011-06-13 23:39 PDT, Ryuan Choi
no flags
Ryuan Choi
Comment 1 2011-06-07 23:23:48 PDT
Patrick R. Gansterer
Comment 2 2011-06-08 00:26:39 PDT
Comment on attachment 96385 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96385&action=review > Source/CMakeLists.txt:128 > +IF (NOT ENABLE_WEBKIT2) Why not building both at the same time? I'd prefere a ENABLE_WEBKIT1 and a second ENABLE_WEBKIT2
Ryuan Choi
Comment 3 2011-06-08 05:37:55 PDT
Ryuan Choi
Comment 4 2011-06-08 05:41:32 PDT
(In reply to comment #2) > (From update of attachment 96385 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96385&action=review > > > Source/CMakeLists.txt:128 > > +IF (NOT ENABLE_WEBKIT2) > > Why not building both at the same time? > I'd prefere a ENABLE_WEBKIT1 and a second ENABLE_WEBKIT2 How about ENABLE_WEBKIT? I add ENABLE_WEBKIT for legacy and ENABLE_WEBKIT2 for webkit2. if user choose both, we can get both. As a default, ENABLE_WEBKIT will be enabled. Thanks.
Patrick R. Gansterer
Comment 5 2011-06-08 05:48:22 PDT
Comment on attachment 96407 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96407&action=review > Source/CMakeLists.txt:16 > + SET(ENABLE_WEBKIT ) IMHO a MESSAGE(FATAL_ERROR "config error....") whould be better here. > Source/CMakeLists.txt:29 > + SET(DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources") I don't understand this line. How it related to WK2? > Source/CMakeLists.txt:101 > +IF (ENABLE_WEBKIT) > + SET(WebKit_LIBRARY_NAME WebKit) > +ENDIF () > + > +IF (ENABLE_WEBKIT2) > + SET(WebKit2_LIBRARY_NAME WebKit2) > +ENDIF () I don't see a problem in setting this variables all the time. This will make the configuration much simpler. (the same applies to the other similar changes too) > Source/WebKit2/CMakeLists.txt:94 > +SET(WebKit2_SOURCES Please add an empty line when the containig folder changes like we do at the other files: e.g. after "Platform/CoreIPC" to "Plaform", or "Platform" to "PluginProcess". This makes the long lines a little bit easier to read. > Source/WebKit2/CMakeLists.txt:410 > +SET (WebProcess_NAME ../Programs/WebProcess) > +SET (WebProcess_SOURCES "") > + > +SET (WebProcess_LIBRARIES Please removte the space after SET > Source/WebKit2/CMakeLists.txt:441 > + Is there an unneded empty line at the end?
Gyuyoung Kim
Comment 6 2011-06-08 06:48:55 PDT
Ryuan Choi
Comment 7 2011-06-08 08:28:11 PDT
Ryuan Choi
Comment 8 2011-06-08 08:40:12 PDT
(In reply to comment #5) > (From update of attachment 96407 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96407&action=review > > > Source/CMakeLists.txt:16 > > + SET(ENABLE_WEBKIT ) > > IMHO a MESSAGE(FATAL_ERROR "config error....") whould be better here. > Hmm, Is it better to keep Webkit as a default for convenience? IMO, Other engineers, who may not know webkit2, doesn't need to learn new option to build. > > Source/CMakeLists.txt:29 > > + SET(DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources") > > I don't understand this line. How it related to WK2? > I'll check once more. When I started, WebKit2 required it. Now I'm not sure. > > Source/CMakeLists.txt:101 > > +IF (ENABLE_WEBKIT) > > + SET(WebKit_LIBRARY_NAME WebKit) > > +ENDIF () > > + > > +IF (ENABLE_WEBKIT2) > > + SET(WebKit2_LIBRARY_NAME WebKit2) > > +ENDIF () > > I don't see a problem in setting this variables all the time. This will make the configuration much simpler. > (the same applies to the other similar changes too) Done. > > > Source/WebKit2/CMakeLists.txt:94 > > +SET(WebKit2_SOURCES > > Please add an empty line when the containig folder changes like we do at the other files: e.g. after "Platform/CoreIPC" to "Plaform", or "Platform" to "PluginProcess". > This makes the long lines a little bit easier to read. Thank you, I did. > > > Source/WebKit2/CMakeLists.txt:410 > > +SET (WebProcess_NAME ../Programs/WebProcess) > > +SET (WebProcess_SOURCES "") > > + > > +SET (WebProcess_LIBRARIES > > Please removte the space after SET Done. > > > Source/WebKit2/CMakeLists.txt:441 > > + > > Is there an unneded empty line at the end? Done.
Gyuyoung Kim
Comment 9 2011-06-08 08:41:29 PDT
Ryuan Choi
Comment 10 2011-06-08 08:43:27 PDT
(In reply to comment #9) > (From update of attachment 96425 [details]) > Attachment 96425 [details] did not pass efl-ews (efl): > Output: http://queues.webkit.org/results/8777664 I can't understand why it was failed. It looks fine in my pc. Gyuyoung, could you check this?
Ryuan Choi
Comment 11 2011-06-08 18:41:38 PDT
(In reply to comment #8) > (In reply to comment #5) > > > Source/CMakeLists.txt:29 > > > + SET(DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources") > > > > I don't understand this line. How it related to WK2? > > > I'll check once more. > When I started, WebKit2 required it. > Now I'm not sure. > To build InjectedBundleNodeHandle.cpp, DERIVED_SOURCES_DIR is needed. I think that it looks a bug. however, it's not related with this bug. I checked WebKit2/Gtk and it was same. I'll investigate other ports and create different bug if needed. below is error message without it. /workspace/webkits/WebKit2/Source/WebKit2/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp:37:31: fatal error: WebCore/HTMLNames.h: No such file or directory compilation terminated. make[2]: *** [WebKit2/CMakeFiles/WebKit2.dir/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp.o] 오류 1 make[2]: *** 끝나지 않은 작업을 기다리고 있습니다.... /workspace/webkits/WebKit2/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:48:33: fatal error: WebCore/JSDOMWindow.h: No such file or directory compilation terminated. make[2]: *** [WebKit2/CMakeFiles/WebKit2.dir/WebProcess/InjectedBundle/InjectedBundle.cpp.o] 오류 1 make[1]: *** [WebKit2/CMakeFiles/WebKit2.dir/all] 오류 2 make: *** [all] 오류 2
Lucas De Marchi
Comment 12 2011-06-08 20:10:06 PDT
Comment on attachment 96425 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96425&action=review > Source/WebKit2/CMakeLists.txt:427 > + LIST(APPEND WebKit2_SOURCES ${DERIVED_SOURCES_WEBKIT2_DIR}/${_name}Messages.h) You don't need to append the header to the sources list.
Ryuan Choi
Comment 13 2011-06-09 02:01:03 PDT
Ryuan Choi
Comment 14 2011-06-09 02:01:40 PDT
(In reply to comment #12) > (From update of attachment 96425 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96425&action=review > > > Source/WebKit2/CMakeLists.txt:427 > > + LIST(APPEND WebKit2_SOURCES ${DERIVED_SOURCES_WEBKIT2_DIR}/${_name}Messages.h) > > You don't need to append the header to the sources list. Thanks, I removed it.
Gyuyoung Kim
Comment 15 2011-06-09 03:05:01 PDT
Lucas De Marchi
Comment 16 2011-06-09 05:33:03 PDT
Comment on attachment 96563 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96563&action=review > Source/CMakeLists.txt:29 > +IF (ENABLE_WEBKIT2) > + SET(WEBKIT2_DIR "${CMAKE_SOURCE_DIR}/WebKit2") > + SET(DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources") This is needed in order to build Gtk too. Mind setting this unconditionally and setting the others using this var? Like this: SET(DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources") SET(DERIVED_SOURCES_WEBCORE_DIR ${DERIVED_SOURCES_DIR/WebCore) SET(DERIVED_SOURCES_JAVASCRIPTCORE_DIR ${DERIVED_SOURCES_DIR/JavaScriptCore) and so on > Source/WebKit2/CMakeLists.txt:410 > +SET(WebKit2_MESSAGES_IN_FILES > + ${WEBKIT2_DIR}/UIProcess/WebMediaCacheManagerProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebCookieManagerProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/Plugins/PluginProcessProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebInspectorProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebFullScreenManagerProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebDatabaseManagerProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebPageProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebIconDatabase.messages.in > + ${WEBKIT2_DIR}/UIProcess/Downloads/DownloadProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebContext.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebGeolocationManagerProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/DrawingAreaProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebKeyValueStorageManagerProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebResourceCacheManagerProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebProcessProxy.messages.in > + ${WEBKIT2_DIR}/UIProcess/WebApplicationCacheManagerProxy.messages.in > + ${WEBKIT2_DIR}/PluginProcess/WebProcessConnection.messages.in > + ${WEBKIT2_DIR}/PluginProcess/PluginControllerProxy.messages.in > + ${WEBKIT2_DIR}/PluginProcess/PluginProcess.messages.in > + ${WEBKIT2_DIR}/Shared/Plugins/NPObjectMessageReceiver.messages.in > + ${WEBKIT2_DIR}/WebProcess/ApplicationCache/WebApplicationCacheManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/MediaCache/WebMediaCacheManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/Plugins/PluginProxy.messages.in > + ${WEBKIT2_DIR}/WebProcess/WebProcess.messages.in > + ${WEBKIT2_DIR}/WebProcess/Geolocation/WebGeolocationManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/WebCoreSupport/WebDatabaseManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/IconDatabase/WebIconDatabaseProxy.messages.in > + ${WEBKIT2_DIR}/WebProcess/Authentication/AuthenticationManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/ResourceCache/WebResourceCacheManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/FullScreen/WebFullScreenManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/Cookies/WebCookieManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/KeyValueStorage/WebKeyValueStorageManager.messages.in > + ${WEBKIT2_DIR}/WebProcess/WebPage/WebPage.messages.in > + ${WEBKIT2_DIR}/WebProcess/WebPage/WebInspector.messages.in > + ${WEBKIT2_DIR}/WebProcess/WebPage/DrawingArea.messages.in > +) Since we are already in WebKit2 subdir, I think it's better not to keep the ${WEBKIT2_DIR} prefix. If you need it after in ADD_CUSTOM_COMMAND, use the WORKING_DIRECTORY setting. > Source/WebKit2/CMakeLists.txt:436 > +SET(WebProcess_NAME ../Programs/WebProcess) > +SET(WebProcess_SOURCES "") > + > +SET(WebProcess_LIBRARIES > + ${JavaScriptCore_LIBRARY_NAME} > + ${WebCore_LIBRARY_NAME} > + ${WebKit2_LIBRARY_NAME} > +) Did you forget to add the sources? > Source/WebKit2/CMakeLists.txt:452 > +ADD_DEFINITIONS(-DWTF_USE_CROSS_PLATFORM_CONTEXT_MENUS=1) > + Shouldn't this be specific to EFL?
Ryuan Choi
Comment 17 2011-06-09 17:27:41 PDT
Ryuan Choi
Comment 18 2011-06-09 17:30:45 PDT
(In reply to comment #16) > (From update of attachment 96563 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96563&action=review > > > Source/CMakeLists.txt:29 > > +IF (ENABLE_WEBKIT2) > > + SET(WEBKIT2_DIR "${CMAKE_SOURCE_DIR}/WebKit2") > > + SET(DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources") > > This is needed in order to build Gtk too. Mind setting this unconditionally and setting the others using this var? Like this: > SET(DERIVED_SOURCES_DIR "${CMAKE_BINARY_DIR}/DerivedSources") > SET(DERIVED_SOURCES_WEBCORE_DIR ${DERIVED_SOURCES_DIR/WebCore) > SET(DERIVED_SOURCES_JAVASCRIPTCORE_DIR ${DERIVED_SOURCES_DIR/JavaScriptCore) > > and so on I removed IF condition. > > > Source/WebKit2/CMakeLists.txt:410 > > +SET(WebKit2_MESSAGES_IN_FILES > > + ${WEBKIT2_DIR}/UIProcess/WebMediaCacheManagerProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebCookieManagerProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/Plugins/PluginProcessProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebInspectorProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebFullScreenManagerProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebDatabaseManagerProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebPageProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebIconDatabase.messages.in > > + ${WEBKIT2_DIR}/UIProcess/Downloads/DownloadProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebContext.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebGeolocationManagerProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/DrawingAreaProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebKeyValueStorageManagerProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebResourceCacheManagerProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebProcessProxy.messages.in > > + ${WEBKIT2_DIR}/UIProcess/WebApplicationCacheManagerProxy.messages.in > > + ${WEBKIT2_DIR}/PluginProcess/WebProcessConnection.messages.in > > + ${WEBKIT2_DIR}/PluginProcess/PluginControllerProxy.messages.in > > + ${WEBKIT2_DIR}/PluginProcess/PluginProcess.messages.in > > + ${WEBKIT2_DIR}/Shared/Plugins/NPObjectMessageReceiver.messages.in > > + ${WEBKIT2_DIR}/WebProcess/ApplicationCache/WebApplicationCacheManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/MediaCache/WebMediaCacheManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/Plugins/PluginProxy.messages.in > > + ${WEBKIT2_DIR}/WebProcess/WebProcess.messages.in > > + ${WEBKIT2_DIR}/WebProcess/Geolocation/WebGeolocationManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/WebCoreSupport/WebDatabaseManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/IconDatabase/WebIconDatabaseProxy.messages.in > > + ${WEBKIT2_DIR}/WebProcess/Authentication/AuthenticationManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/ResourceCache/WebResourceCacheManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/FullScreen/WebFullScreenManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/Cookies/WebCookieManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/KeyValueStorage/WebKeyValueStorageManager.messages.in > > + ${WEBKIT2_DIR}/WebProcess/WebPage/WebPage.messages.in > > + ${WEBKIT2_DIR}/WebProcess/WebPage/WebInspector.messages.in > > + ${WEBKIT2_DIR}/WebProcess/WebPage/DrawingArea.messages.in > > +) > > Since we are already in WebKit2 subdir, I think it's better not to keep the ${WEBKIT2_DIR} prefix. > > If you need it after in ADD_CUSTOM_COMMAND, use the WORKING_DIRECTORY setting. Thanks, I did. > > > Source/WebKit2/CMakeLists.txt:436 > > +SET(WebProcess_NAME ../Programs/WebProcess) > > +SET(WebProcess_SOURCES "") > > + > > +SET(WebProcess_LIBRARIES > > + ${JavaScriptCore_LIBRARY_NAME} > > + ${WebCore_LIBRARY_NAME} > > + ${WebKit2_LIBRARY_NAME} > > +) > > Did you forget to add the sources? No, WebProcess only requires platform specific one files. > > > Source/WebKit2/CMakeLists.txt:452 > > +ADD_DEFINITIONS(-DWTF_USE_CROSS_PLATFORM_CONTEXT_MENUS=1) > > + > > Shouldn't this be specific to EFL? Right, I removed. Thanks.
Ryuan Choi
Comment 19 2011-06-12 07:20:23 PDT
Ryuan Choi
Comment 20 2011-06-12 07:20:44 PDT
(In reply to comment #19) > Created an attachment (id=96879) [details] > Patch update patch to check ews.
Eric Seidel (no email)
Comment 21 2011-06-13 15:29:53 PDT
I'm happy to rs=me this, assuming it builds on all EWS bots.
Ryuan Choi
Comment 22 2011-06-13 23:39:29 PDT
WebKit Review Bot
Comment 23 2011-06-15 19:05:03 PDT
Comment on attachment 97074 [details] Patch Clearing flags on attachment: 97074 Committed r88991: <http://trac.webkit.org/changeset/88991>
WebKit Review Bot
Comment 24 2011-06-15 19:05:09 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.