Bug 33542 - [Qt] Split the build process in two different .pro files.
Summary: [Qt] Split the build process in two different .pro files.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks: 33297
  Show dependency treegraph
 
Reported: 2010-01-12 11:50 PST by Jocelyn Turcotte
Modified: 2010-01-14 05:21 PST (History)
9 users (show)

See Also:


Attachments
Monolithic patch draft (83.08 KB, patch)
2010-01-12 11:54 PST, Jocelyn Turcotte
no flags Details | Formatted Diff | Diff
Patch V2 (82.43 KB, patch)
2010-01-13 07:58 PST, Jocelyn Turcotte
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jocelyn Turcotte 2010-01-12 11:50:39 PST
This allows qmake to be run once all source files are available.

It corrects:
- The incremental build problem on the bot where modified headers would not recompile dependent cpp files
- The incremental build problem on windows where unmodified idl files would be reparsed unnecessarily
Comment 1 Jocelyn Turcotte 2010-01-12 11:54:23 PST
Created attachment 46388 [details]
Monolithic patch draft

The patch adds two pro files to generate sources:
JavaScriptCore/DerivedSources.pro
WebCore/DerivedSources.pro

Most of the content of these files have been taken from
JavaScriptCore/JavaScriptCore.pri
WebCore/WebCore.pro

The build-webkit script have been modified accordingly and now allow the option --generate-only to stop after the source generation step.
Comment 2 Simon Hausmann 2010-01-12 14:25:11 PST
reminder to self: we should test this also on Symbian
Comment 3 Csaba Osztrogonác 2010-01-12 14:27:53 PST
Thx Jocelyn, I'll test it on Linux tomorrow.
Comment 4 Laszlo Gombos 2010-01-12 19:25:08 PST
Approach looks good to me. This is the kind of patch which gets old really fast, so we might want to make sure we coordinate well when it gets landed.

Can we move the "Derived source generator" rules (e.g. IDL_BINDINGS list) from WebCoreCommon.pri to DerivedSources.pro as I think WebCore.pro does not need to know about these rules ? 

Would WebCore.pri be a better filename instead of WebCoreCommon.pri (as we have WebKit.pri instead of WebKitCommon.pri and JavaScriptCore.pri instead of JavaScriptCoreCommon.pri) ?
Comment 5 Jocelyn Turcotte 2010-01-13 01:34:20 PST
(In reply to comment #4)
> Can we move the "Derived source generator" rules (e.g. IDL_BINDINGS list) from
> WebCoreCommon.pri to DerivedSources.pro as I think WebCore.pro does not need to
> know about these rules ? 
> 
The reason it is needed it to compute the list of generated files to add to SOURCES, depending on the list of features in input.

> Would WebCore.pri be a better filename instead of WebCoreCommon.pri (as we have
> WebKit.pri instead of WebKitCommon.pri and JavaScriptCore.pri instead of
> JavaScriptCoreCommon.pri) ?
I agree
Comment 6 Csaba Osztrogonác 2010-01-13 03:04:17 PST
Jocelyn, it's a very good and useful patch on the whole.

I tested your patch on linux, and you should only make some minor changes to work correctly. (I will test it on windows too ASAP.)

qmake processes WebKit.pro after generating sources finished. The directory of generated sources is WebCore/generated and it make WebKit.pri to add standalone_package oprion to config, but it shouldn't. (In this way building of DRT, standalone jsc, ... is disabled)

Webkit.pri:
...
CONFIG(QTDIR_build): CONFIG += standalone_package
else:exists($$PWD/WebCore/generated): CONFIG += standalone_package
...

I suggest we should move all generated sources into WebKitBuild directory instead of into the source tree. It would help to make a clean build simple with rm -rf WebKitBuild even as now.

With your patch I got 3 touch event layout test failed. It might be caused some changes in build scripts after your patch. I will find it.

fast/dom/prototype-inheritance.html:
-PASS inner.TouchEvent.isInner is true
-PASS inner.TouchEvent.constructor.isInner is true

fast/dom/Window/window-properties.html:
-window.TouchEvent [object TouchEventConstructor]
-window.TouchEvent.prototype [printed above as window.Event.prototype]

fast/js/global-constructors.html:
-PASS TouchEvent.toString() is '[object TouchEventConstructor]'
Comment 7 Csaba Osztrogonác 2010-01-13 03:20:41 PST
Standalone package problem caused by http://trac.webkit.org/changeset/52734
Comment 8 Simon Hausmann 2010-01-13 04:07:43 PST
No worries about the standalone_package stuff. It's mostly a playground for me right now and I'll be happy to adjust it after we land this in the trunk. (sharing an office with Jocelyn helps if I have questions ;-)
Comment 9 Simon Hausmann 2010-01-13 04:09:41 PST
(In reply to comment #2)
> reminder to self: we should test this also on Symbian

Passed. armv6_urel against 5.0 SDK with Raptor built perfectly.
Comment 10 Csaba Osztrogonác 2010-01-13 04:11:46 PST
(In reply to comment #8)
> No worries about the standalone_package stuff. It's mostly a playground for me
> right now and I'll be happy to adjust it after we land this in the trunk.
> (sharing an office with Jocelyn helps if I have questions ;-)

We should fix it before landing because QtBuildbot would be very red without standalone jsc and DRT. :)
Comment 11 Simon Hausmann 2010-01-13 04:58:55 PST
(In reply to comment #10)
> (In reply to comment #8)
> > No worries about the standalone_package stuff. It's mostly a playground for me
> > right now and I'll be happy to adjust it after we land this in the trunk.
> > (sharing an office with Jocelyn helps if I have questions ;-)
> 
> We should fix it before landing because QtBuildbot would be very red without
> standalone jsc and DRT. :)

Sorry, I misread. Your'e right, _those_ standalone builds need to work indeed.
Comment 12 Csaba Osztrogonác 2010-01-13 05:44:31 PST
I try it on my windows too, but unfortunately it isn't work, because GENERATED_SOURCES_DIR changed to absolute path (with drive letter). Windows cd command can't handle drive letters. It tolerates slashes instead of backslashes, but it has problems with prefix slash as root directory.

before your patch:
perl -ne "print lc" ..\..\..\WebCore\css\CSSPropertyNames.in T:/webkit_org/WebCore/css/SVGCSSPropertyNames.in > generated\release\CSSPropertyNames.in && cd generated\release && perl T:/webkit_org/WebCore/css/makeprop.pl && del CSSPropertyNames.strip CSSPropertyNames.in CSSPropertyNames.gperf

after your patch:
perl -ne "print lc" ..\..\WebCore\css\CSSPropertyNames.in T:/webkit_org/WebCore/css/DashboardSupportCSSPropertyNames.in T:/webkit_org/WebCore/css/SVGCSSPropertyNames.in > T:/webkit_org/WebCore/generated/CSSPropertyNames.in && cd T:/webkit_org/WebCore/generated && perl T:/webkit_org/WebCore/css/makeprop.pl && del CSSPropertyNames.in CSSPropertyNames.gperf
The system cannot find the path specified.
mingw32-make: *** [../../WebCore/generated/CSSPropertyNames.cpp] Error 1
Failed to generate derived sources!
Comment 13 Csaba Osztrogonác 2010-01-13 07:20:58 PST
I found a fix for this Windows XP related problem (Vista and Win7 works correctly)

We should replace slahses to bakcslashes in JavaScriptCore.pro, DerivedSources.pro and WebCoreCommon.pri:

GENERATED_SOURCES_DIR = $$PWD/generated
GENERATED_SOURCES_DIR = $$replace(GENERATED_SOURCES_DIR, "/", $$QMAKE_DIR_SEP)
Comment 14 Jocelyn Turcotte 2010-01-13 07:58:03 PST
Created attachment 46463 [details]
Patch V2

Updated patch:
- Move generated sources back to the build directory (WebKitBuild)
- Remove --generate-only option and add a root DerivedSources.pro
- Corrects a linking problem when using Qt without phonon support
Comment 15 WebKit Review Bot 2010-01-13 08:03:57 PST
Attachment 46463 [details] did not build on qt:
Build output: http://webkit-commit-queue.appspot.com/results/185416
Comment 16 Simon Hausmann 2010-01-13 08:09:35 PST
Committed r53187: <http://trac.webkit.org/changeset/53187>
Comment 17 George Guo 2010-01-13 12:06:47 PST
The version 53194 which included all these changes did not build on QtS60. 

Here is the error message: 
L:\WebKit\WebKitTools\Scripts>perl build-webkit --qt --debug --minimal
Generating derived sources

Calling 'qmake -r DEFINES+=ENABLE_CHANNEL_MESSAGING=0 DEFINES+=ENABLE_DATABASE=0
 DEFINES+=ENABLE_DATALIST=0 DEFINES+=ENABLE_DOM_STORAGE=0 DEFINES+=ENABLE_EVENTS
OURCE=0 DEFINES+=ENABLE_FILTERS=0 DEFINES+=ENABLE_ICONDATABASE=0 DEFINES+=ENABLE
_JAVASCRIPT_DEBUGGER=0 DEFINES+=ENABLE_OFFLINE_WEB_APPLICATIONS=0 DEFINES+=ENABL
E_SHARED_WORKERS=0 DEFINES+=ENABLE_SVG=0 DEFINES+=ENABLE_SVG_ANIMATION=0 DEFINES
+=ENABLE_SVG_AS_IMAGE=0 DEFINES+=ENABLE_SVG_FONTS=0 DEFINES+=ENABLE_SVG_FOREIGN_
OBJECT=0 DEFINES+=ENABLE_SVG_USE=0 DEFINES+=ENABLE_VIDEO=0 DEFINES+=ENABLE_WEB_S
OCKETS=0 DEFINES+=ENABLE_WORKERS=0 DEFINES+=ENABLE_XPATH=0 DEFINES+=ENABLE_XSLT=
0 -r L:/WebKit/DerivedSources.pro CONFIG-=release CONFIG+=debug' in L:\WebKit\We
bKitBuild\Debug

Reading L:/WebKit/JavaScriptCore/DerivedSources.pro [L:/WebKit/WebKitBuild/Debug
/JavaScriptCore]
Reading L:/WebKit/WebCore/DerivedSources.pro [L:/WebKit/WebKitBuild/Debug/WebCor
e]
Calling 'make  -f Makefile.DerivedSources.Debug generated_files' in L:\WebKit\We
bKitBuild\Debug/JavaScriptCore

make: Makefile.DerivedSources.Debug: No such file or directory
make: *** No rule to make target `Makefile.DerivedSources.Debug'.  Stop.
make: Entering directory `L:/WebKit/WebKitBuild/Debug/JavaScriptCore'
make: Leaving directory `L:/WebKit/WebKitBuild/Debug/JavaScriptCore'
Failed to generate JavaScriptCore's derived sources!

Did I do wrong? Is this an known issue? or should I file a bug?
Comment 18 Laszlo Gombos 2010-01-13 15:18:14 PST
build-webkit --qt --no-svg seems to fail as well for me.
Comment 19 Simon Hausmann 2010-01-14 01:49:12 PST
(In reply to comment #17)
> Did I do wrong? Is this an known issue? or should I file a bug?

Confirmed. I've filed bug 33658 for it.
Comment 20 Jocelyn Turcotte 2010-01-14 05:21:14 PST
(In reply to comment #18)
> build-webkit --qt --no-svg seems to fail as well for me.

Thanks, fixed in r53261