Bug 66852 – [Qt] [PATCH] Improve the handling of $project in webkitdirs.pm

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Bartosz Fabianowski]

Created attachment 104983 [details]
Patch implementing the proposed improvements

The buildQMakeProject() method in Tools/Scripts/webkitdirs.pm takes a project type as its first parameter. Analogously to the Gtk port's buildGtkProject(), two project types are supported:

* build-webkit uses project type "WebKit"
* build-jsc uses project type "JavaScriptCore"

There are a number of bugs and hacks in the way these project types are handled and evaluated. The attached patch cleans up the handling a bit. The following changes are made:

* A FIXME in build-webkit is addressed by correctly passing "WebKit" instead of "Source/WebKit" as the project type
* The buildQMakeQtProject() method is retired whose only purpose was to sabotage the project type by replacing it with ""
* The handling of the project type in buildQMakeProject() is simplified, reducing it to what is needed for the two project types above only

In addition to the above improvements, buildQMakeQtProject() can be instructed to build the libraries ("JavaScriptCore", "WebCore", "WebKit/qt/Api") only by leaving the project type blank. Tools and examples are omitted from the build in this case. This is useful under e.g. FreeBSD where the Tools depend on an installed version of the Qt WekBit port, leading to circular dependencies.

I have tested this patch by checking that the following work:
* build-jsc -> builds JavaScriptCore only
* build-webkit -> buildQMakeQtProject, Tools, examples
* build-webkit with project type set to "" in the call to buildQMakeQtProject() -> buildQMakeQtProject only

The issues addressed and the general idea of the patch were discussed on #qtwebkit with bbandix who is CC'd on this bug report.

[Bartosz Fabianowski]

There were two typos in my initial submission. The tests performed and their results actually were:

I have tested this patch by checking that the following work:
* build-jsc -> builds JavaScriptCore only
* build-webkit -> builds WebKit libraries, Tools, examples
* build-webkit with project type set to "" in the call to buildQMakeQtProject() -> builds WebKit libraries only

[Balazs Kelemen]

> * build-webkit with project type set to "" in the call to buildQMakeQtProject() -> builds WebKit libraries only

Could we support a "libs" or whatever build option in build-webkit instead of handing over the task of creating a new wrapper script to the embedder? Apart from that, the change looks good to me.

[Zoltan Horvath]

(In reply to comment #1)

> * build-webkit with project type set to "" in the call to buildQMakeQtProject() -> builds WebKit libraries only

Do you mean in the call to buildQMakeProject? Since you have removed buildQMakeQtProject.

If I understand correctly you set the "" parameter by hand to get a library only build. If we really need this then it'd be good to support it in build-webkit with --only-webkit-library parameter.

[Andras Becsi]

The change looks good to me except that I agree with Balazs, adding an extra option like --lib-only or something similar would be a good improvement and would make the lives of developers easier.
Also, every patch needs a ChangeLog, you can read about that on http://trac.webkit.org/wiki/QtWebKitContrib#Submittingapatch

Please submit an updated patch.

(In reply to comment #1)
> There were two typos in my initial submission. The tests performed and their results actually were:
> 
> I have tested this patch by checking that the following work:
> * build-jsc -> builds JavaScriptCore only
> * build-webkit -> builds WebKit libraries, Tools, examples
> * build-webkit with project type set to "" in the call to buildQMakeQtProject() -> builds WebKit libraries only

[Andras Becsi]

Also, you might be interested in https://bugs.webkit.org/show_bug.cgi?id=57324

[Bartosz Fabianowski]

I will submit an updated patch as soon as time permits.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.