[Qt] [WK2] Fix build if WebKitTools are not available

• Component: Platform
• Status: RESOLVED FIXED

[Laszlo Gombos]

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.

[Andras Becsi]

Created attachment 75196 [details]
proposed fix

[Andras Becsi]

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.

[Simon Hausmann]

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?

[Andras Becsi]

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.

[Andras Becsi]

Created attachment 75698 [details]
updated patch

Updated to ToT.

[Andras Becsi]

(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.

[Laszlo Gombos]

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.

[Andras Becsi]

(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.

[Andras Becsi]

Comment on attachment 75698 [details]
updated patch

Clearing flags since I'm working on a new solution.

[Balazs Kelemen]

(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.

[Andras Becsi]

Created attachment 76216 [details]
proposed patch v3

[Andras Becsi]

Created attachment 76369 [details]
updated patch

Updated to ToT.

[WebKit Review Bot]

Comment on attachment 76369 [details]
updated patch

Clearing flags on attachment: 76369

Committed r73905: <http://trac.webkit.org/changeset/73905>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

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