WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(24.11 KB, patch)
2011-06-08 05:37 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Patch
(23.60 KB, patch)
2011-06-08 08:28 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Patch
(23.41 KB, patch)
2011-06-09 02:01 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Patch
(22.93 KB, patch)
2011-06-09 17:27 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Patch
(22.96 KB, patch)
2011-06-12 07:20 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Patch
(23.03 KB, patch)
2011-06-13 23:39 PDT
,
Ryuan Choi
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Ryuan Choi
Comment 1
2011-06-07 23:23:48 PDT
Created
attachment 96385
[details]
Patch
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
Created
attachment 96407
[details]
Patch
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
Comment on
attachment 96407
[details]
Patch
Attachment 96407
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/8777630
Ryuan Choi
Comment 7
2011-06-08 08:28:11 PDT
Created
attachment 96425
[details]
Patch
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
Comment on
attachment 96425
[details]
Patch
Attachment 96425
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/8777664
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
Created
attachment 96563
[details]
Patch
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
Comment on
attachment 96563
[details]
Patch
Attachment 96563
[details]
did not pass efl-ews (efl): Output:
http://queues.webkit.org/results/8812631
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
Created
attachment 96671
[details]
Patch
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
Created
attachment 96879
[details]
Patch
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
Created
attachment 97074
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug