Bug 83693

Summary: [EFL][WK2] Fix WebKit2-EFL build
Product: WebKit Reporter: Mariusz Grzegorczyk <mariusz.g>
Component: WebKit2Assignee: Ryuan Choi <ryuan.choi>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, g.czajkowski, mrobinson, rakuco, ryuan.choi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Linux   
Bug Depends on:    
Bug Blocks: 61838    
Attachments:
Description Flags
Patch fixing webkit2 efl build
none
Patch
none
Patch
none
patch for landing
none
patch for landing2 none

Mariusz Grzegorczyk
Reported 2012-04-11 08:35:08 PDT
After applying all patches related with master bug "[EFL][WK2] Support for EFL port of WebKit2": https://bugs.webkit.org/show_bug.cgi?id=61838 build break occurs because of missing headers path, and changed directory for MiniBrowser from Programs to bin.
Attachments
Patch fixing webkit2 efl build (3.32 KB, patch)
2012-04-11 08:45 PDT, Mariusz Grzegorczyk 		no flags
DetailsFormatted DiffDiff
Patch (17.61 KB, patch)
2012-05-29 06:49 PDT, Ryuan Choi 		no flags
DetailsFormatted DiffDiff
Patch (17.98 KB, patch)
2012-05-29 09:43 PDT, Ryuan Choi 		no flags
DetailsFormatted DiffDiff
patch for landing (18.17 KB, patch)
2012-05-30 01:25 PDT, Ryuan Choi 		no flags
DetailsFormatted DiffDiff
patch for landing2 (18.11 KB, patch)
2012-05-30 02:27 PDT, Ryuan Choi 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Mariusz Grzegorczyk
Comment 1 2012-04-11 08:45:12 PDT
Created attachment 136675 [details] Patch fixing webkit2 efl build
Grzegorz Czajkowski
Comment 2 2012-04-11 23:42:40 PDT
Please rebase this patch.
Mariusz Grzegorczyk
Comment 3 2012-04-12 01:01:08 PDT
(In reply to comment #2) > Please rebase this patch. This patch doesn't need rebase because depends on bug 61850. Dependency/Blocks fields added.
Ryuan Choi
Comment 4 2012-05-29 06:49:49 PDT
Created attachment 144546 [details] Patch
Ryuan Choi
Comment 5 2012-05-29 06:51:27 PDT
(In reply to comment #4) > Created an attachment (id=144546) [details] > Patch Updated patch to catch latest changes and removed dependency of Bug 61850 from patch.
Raphael Kubo da Costa (:rakuco)
Comment 6 2012-05-29 07:20:13 PDT
Comment on attachment 144546 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144546&action=review Since the EWS does not test this, the most I can do is comment on the build system changes. > Source/WebKit2/CMakeLists.txt:497 > +SET(WebProcess_NAME ../bin/WebProcess) It's better to use SET_TARGET_PROPERTIES(... RUNTIME_OUTPUT_DIRECTORY ...) like other parts of the build system do. > Source/WebKit2/CMakeLists.txt:498 > +SET(WebProcess_SOURCES "") Is this empty definition needed? > Source/WebKit2/CMakeLists.txt:507 > +SET(ForwardingHeaders_NAME ) > +SET(ForwardingNetworkHeaders_NAME ) Are these empty definitions needed? > Source/cmake/OptionsEfl.cmake:42 > +ADD_DEFINITIONS(-DBUILDING_SOUP__=1) I see that this is checked in WK2, but I couldn't find any part of gtk's build system defining that. Is that expected?
Ryuan Choi
Comment 7 2012-05-29 09:43:10 PDT
Created attachment 144572 [details] Patch
Ryuan Choi
Comment 8 2012-05-29 09:46:54 PDT
(In reply to comment #6) > (From update of attachment 144546 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=144546&action=review > > Since the EWS does not test this, the most I can do is comment on the build system changes. > > > Source/WebKit2/CMakeLists.txt:497 > > +SET(WebProcess_NAME ../bin/WebProcess) > > It's better to use SET_TARGET_PROPERTIES(... RUNTIME_OUTPUT_DIRECTORY ...) like other parts of the build system do. > Thanks for the comment. I fixed. > > Source/WebKit2/CMakeLists.txt:498 > > +SET(WebProcess_SOURCES "") > > Is this empty definition needed? > > > Source/WebKit2/CMakeLists.txt:507 > > +SET(ForwardingHeaders_NAME ) > > +SET(ForwardingNetworkHeaders_NAME ) > > Are these empty definitions needed? > Removed them. > > Source/cmake/OptionsEfl.cmake:42 > > +ADD_DEFINITIONS(-DBUILDING_SOUP__=1) > > I see that this is checked in WK2, but I couldn't find any part of gtk's build system defining that. Is that expected? yes, GNUmakefile.am also defines it.
Raphael Kubo da Costa (:rakuco)
Comment 9 2012-05-29 16:33:32 PDT
Comment on attachment 144572 [details] Patch The build system part looks fine, the rest should supposedly be OK.
Ryuan Choi
Comment 10 2012-05-29 17:36:53 PDT
CCing mrobinson because I touched WebKit2/Gtk+ related files.
Martin Robinson
Comment 11 2012-05-29 17:40:51 PDT
The libsoup changes look fine to me, but I'm CCing Carlos as he's the original author of that code.
Carlos Garcia Campos
Comment 12 2012-05-29 23:49:43 PDT
Looks good to me too, I shoulnd't have used Gtk headers in soup file, sorry.
Gyuyoung Kim
Comment 13 2012-05-30 00:33:09 PDT
Comment on attachment 144572 [details] Patch Though I don't like to mix multiple patches personally, informal rs+ on my side in order to build EFL WebKit2 now. But, I think this patch should be reviewed by GTK or WK2 reviewers.
Carlos Garcia Campos
Comment 14 2012-05-30 00:54:00 PDT
Comment on attachment 144572 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144572&action=review Looks good, just a couple of things you can change before landing. > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:66 > +#if ENABLE(INPUT_TYPE_COLOR) > +#include "ColorChooser.h" > +#endif > + Use #include <WebCore/ColorChooser.h> like all other webcore headers in this file. You don't need the #if ENABLE(INPUT_TYPE_COLOR) because the header is already protected by that check. Remember to move the header to keep all headers sorted alpahbetically > Source/WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:78 > +#if ENABLE(WEB_INTENTS) > +#include "IntentRequest.h" > +#endif And same here for IntentRequest.h
Ryuan Choi
Comment 15 2012-05-30 01:25:07 PDT
Created attachment 144749 [details] patch for landing
Ryuan Choi
Comment 16 2012-05-30 01:25:42 PDT
Comment on attachment 144749 [details] patch for landing Thank you so much.
Build Bot
Comment 17 2012-05-30 02:03:08 PDT
Comment on attachment 144749 [details] patch for landing Attachment 144749 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12843781
Build Bot
Comment 18 2012-05-30 02:26:15 PDT
Comment on attachment 144749 [details] patch for landing Attachment 144749 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12845820
Ryuan Choi
Comment 19 2012-05-30 02:27:37 PDT
Created attachment 144765 [details] patch for landing2
WebKit Review Bot
Comment 20 2012-05-30 05:12:41 PDT
Comment on attachment 144765 [details] patch for landing2 Clearing flags on attachment: 144765 Committed r118914: <http://trac.webkit.org/changeset/118914>
WebKit Review Bot
Comment 21 2012-05-30 05:12:47 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.