Bug 50242 - [Qt] [WK2] Fix build if WebKitTools are not available
Summary: [Qt] [WK2] Fix build if WebKitTools are not available
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P3 Normal
Assignee: Andras Becsi
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-11-30 08:39 PST by Laszlo Gombos
Modified: 2010-12-13 06:17 PST (History)
9 users (show)

See Also:


Attachments
proposed fix (7.26 KB, patch)
2010-11-30 13:44 PST, Andras Becsi
no flags Details | Formatted Diff | Diff
proposed fix v2 (16.64 KB, patch)
2010-12-06 06:51 PST, Andras Becsi
no flags Details | Formatted Diff | Diff
updated patch (16.54 KB, patch)
2010-12-06 08:24 PST, Andras Becsi
no flags Details | Formatted Diff | Diff
proposed patch v3 (23.36 KB, patch)
2010-12-10 10:22 PST, Andras Becsi
no flags Details | Formatted Diff | Diff
updated patch (18.49 KB, patch)
2010-12-13 04:28 PST, Andras Becsi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2010-11-30 08:39:26 PST
QtWebKit releases (both source and binary releases) does not include components from the WebKitTools directory. The WebKit1 build system is engineered so that it detects if WebKitTools is available and if it does not than it does not build it (see the "exists" tests in WebKit.pro).

It seems to me that building WebKit2 core components are tied to WebTools - for example the ualist_copier and qrc_copier targets in WebKit2/DerivedSources.pro are related to components under WebKitTools. 

This issue is might be a blocker for productizing WebKit2 for QtWebKit.
Comment 1 Andras Becsi 2010-11-30 13:44:09 PST
Created attachment 75196 [details]
proposed fix
Comment 2 Andras Becsi 2010-12-01 00:21:10 PST
Comment on attachment 75196 [details]
proposed fix

I realized that WebKitTools/Scripts/generate-forwarding-headers.pl is needed during the build process of WebKit2, so it needs to be moved to WebKit2/Scripts to be able to generate all the forwarding headers in a pre-build phase if WebKitTools is not present.
Comment 3 Simon Hausmann 2010-12-01 00:22:23 PST
Hmm, I'm thinking that perhaps we should move away from this concept that a source release does not contain the entire WebKit source tree, apart from obvious toplevels such as WebKitSite :).

Now that WebKit isn't part of the Qt source tree anymore, what do we really gain? A little bit of disk space in exchange for a more complex build system?
Comment 4 Andras Becsi 2010-12-06 06:51:16 PST
Created attachment 75688 [details]
proposed fix v2

Updated the patch and moved the forwarding header generator script to WebKit2/Scripst not to break the WebKit2 build.
Comment 5 Andras Becsi 2010-12-06 08:24:38 PST
Created attachment 75698 [details]
updated patch

Updated to ToT.
Comment 6 Andras Becsi 2010-12-06 08:41:14 PST
(In reply to comment #3)
> Hmm, I'm thinking that perhaps we should move away from this concept that a source release does not contain the entire WebKit source tree, apart from obvious toplevels such as WebKitSite :).
> 
> Now that WebKit isn't part of the Qt source tree anymore, what do we really gain? A little bit of disk space in exchange for a more complex build system?

I agree with you that having the whole WebKit tree in Qt would make our lives easier, nonetheless I uploaded a patch because I think this would make the build system more consistent.

We can discuss whether this is a feasible change and if we need it but I don't think this makes the build system more complex.
Comment 7 Laszlo Gombos 2010-12-06 12:05:02 PST
Comment on attachment 75698 [details]
updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=75698&action=review

> I agree with you that having the whole WebKit tree in Qt would make our lives easier, nonetheless I uploaded a patch because I think this would make the build system more consistent.

Exactly. We should keep the make system modular so that build files for independent make targets are decoupled (minimize the reference between them).

> WebKit2/DerivedSources.pro:47
> +    !exists($$DIR): system($$QMAKE_MKDIR $$OUTPUT_DIR/WebKitTools/MiniBrowser/qt)

I was hoping to move this rule to a pr* file located under WebKitTools/MiniBrowser - we can than just remove the guards. If the WebKitTools/MiniBrowser directory does not exists than this rule will be naturally not executed.

> WebKit2/DerivedSources.pro:113
> +    ualist_copier.input = $${SRC_ROOT_DIR}WebKitTools/QtTestBrowser/useragentlist.txt

Ditto. Perhaps we can have a DerivedSources.pro for the MiniBrowser, or add this to MiniBrowser.pro.
Comment 8 Andras Becsi 2010-12-06 13:41:37 PST
(In reply to comment #7)
> > WebKit2/DerivedSources.pro:113
> > +    ualist_copier.input = $${SRC_ROOT_DIR}WebKitTools/QtTestBrowser/useragentlist.txt
> 
> Ditto. Perhaps we can have a DerivedSources.pro for the MiniBrowser, or add this to MiniBrowser.pro.

Previously I tried to add these to MiniBrowser.pro, but I couldn't find a way to add the rule as a pre-dependency that runs first in the build step so I added it to DerivedSources.

DerivedSources.pro in MiniBrowser sounds like a good idea to me.
Comment 9 Andras Becsi 2010-12-07 10:39:29 PST
Comment on attachment 75698 [details]
updated patch

Clearing flags since I'm working on a new solution.
Comment 10 Balazs Kelemen 2010-12-07 15:43:05 PST
(In reply to comment #6)
> (In reply to comment #3)
> > Hmm, I'm thinking that perhaps we should move away from this concept that a source release does not contain the entire WebKit source tree, apart from obvious toplevels such as WebKitSite :).
> > 
> > Now that WebKit isn't part of the Qt source tree anymore, what do we really gain? A little bit of disk space in exchange for a more complex build system?
> 
> I agree with you that having the whole WebKit tree in Qt would make our lives easier, nonetheless I uploaded a patch because I think this would make the build system more consistent.
> 

We are going into the direction to not having WebKit in the Qt source
release at all, isn't we? However, I agree that the patch improves the
consistency of the build system as long as we only need that script for
WebKit2.
Comment 11 Andras Becsi 2010-12-10 10:22:22 PST
Created attachment 76216 [details]
proposed patch v3
Comment 12 Andras Becsi 2010-12-13 04:28:05 PST
Created attachment 76369 [details]
updated patch

Updated to ToT.
Comment 13 WebKit Review Bot 2010-12-13 05:17:04 PST
Comment on attachment 76369 [details]
updated patch

Clearing flags on attachment: 76369

Committed r73905: <http://trac.webkit.org/changeset/73905>
Comment 14 WebKit Review Bot 2010-12-13 05:17:10 PST
All reviewed patches have been landed.  Closing bug.
Comment 15 WebKit Review Bot 2010-12-13 06:17:33 PST
http://trac.webkit.org/changeset/73905 might have broken GTK Linux 64-bit Debug
The following tests are not passing:
editing/selection/extend-by-character-002.html