Bug 13499 – [qmake] Make it possible to use qmake for other ports

• Component: New Bugs
• Status: RESOLVED FIXED

[Holger Freyther]

qmake is a simple but still capable buildsystem. Besides being written using Qt there is no strict requirement on Qt and in theory and even in practice it can be used to build other stuff as well.

[Holger Freyther]

Created attachment 14200 [details]
Add some OUTPUT directories to the include path

On my build some auto-generated files were not found. I had to extend the INCLUDEPATH to look into these two directories.

[Holger Freyther]

Created attachment 14201 [details]
Scope files belonging to the Qt port

Scope files belonging to the Qt port to allow other ports to use qmake.

[Oliver Hunt]

Comment on attachment 14200 [details]
Add some OUTPUT directories to the include path

Is pcre.pri qt specific? (eg. is it only used for qmake?)

[Oliver Hunt]

Comment on attachment 14201 [details]
Scope files belonging to the Qt port

You'll probably need a person with more qt knowledge for this one sorry -- try lars, zackr or niko (i think he has reviewer privs)

[Holger Freyther]

(In reply to comment #4)
> (From update of attachment 14201 [details] [edit])
> You'll probably need a person with more qt knowledge for this one sorry -- try
> lars, zackr or niko (i think he has reviewer privs)
What is the appropriate way to get their attention? Set them on the CC list of this bug and wait for them to respond or mail them privately and point them to the bug?

[Holger Freyther]

(In reply to comment #3)
> (From update of attachment 14200 [details] [edit])
> Is pcre.pri qt specific? (eg. is it only used for qmake?)

The .pri gets included by a .pro file. I would say it is qmake specific and is not limited to the Qt port.

[Holger Freyther]

Looking at webkitdirs.pm I wonder if the Qt port really wants to force the name of the checkout to "WebKit". But that is up to them I guess.

[Holger Freyther]

Created attachment 14291 [details]
Add the OUTPUTDIR to the INCLUDEPATH.

Updated the patch

[Holger Freyther]

Created attachment 14292 [details]
Scope the qt-port. To allow other ports to reuse the qmake system

Updated the patch to apply to the current HEAD of trunk. Moved some files around, scoped some more defines

[Holger Freyther]

Created attachment 14293 [details]
Make it possible to compile the Gdk port using qmake

Add Gdk,Cairo,ImageDecoders to the qmake project files. Update the build-webkit script to allow building the Gdk/Gtk-Port using qmake.

[Holger Freyther]

Zack, here is an updated patch and as you wished I have added another port. If you agree with the changes I could fold the series and generate a proper ChangeLog, or feel free to create that yourself.

[Simon Hausmann]

I think it would be nice if the Qt port would remain the default for the qmake build. I like the ability of doing a quick build of WebKit without being forced to use build-webkit by just running qmake && make. With the proposed patches it is neccessary to pass "CONFIG+=qt-port". Just my two cents :)

[Holger Freyther]

(In reply to comment #12)
> I think it would be nice if the Qt port would remain the default for the qmake
> build. I like the ability of doing a quick build of WebKit without being forced
> to use build-webkit by just running qmake && make. With the proposed patches it
> is neccessary to pass "CONFIG+=qt-port". Just my two cents :)
> 

I sort of agree and build-webkit will already prefer the Qt port (when both QTDIR and the other env var is set).

so something like !gdk-port:CONFIG += qt-port in WebCore.pro should give the wanted effect?

[Mark Rowe (bdash)]

Comment on attachment 14291 [details]
Add the OUTPUTDIR to the INCLUDEPATH.

Why is this not needed for the Qt port already?

[Mark Rowe (bdash)]

Comment on attachment 14293 [details]
Make it possible to compile the Gdk port using qmake

+    if ((isGdk()) and ($path =~ /WebCore/)) {

I dont think you need so many parenthesis here.

+sub isGdk()
+{
+    return defined($ENV{'BUILD_GDK'})
+}

It would be great if this could be controlled by a build-webkit command-line argument.  It's more natural to do 'build-webkit --gdk' than 'BUILD_GDK=1 build-webkit' IMO.

+sub buildQMakeGdkProject($$)

Inside this function there are a few instances of "if(..)" -- our style guidelines call for a space before the parenthesis.

+gdk-port:CONFIG  += link_pkgconfig

The += is indented weirdly here.

+gdk-port:DEFINES += BUILDING_GDK__=1 BUILDING_CAIRO__
+gdk-port:CONFIG  += link_pkgconfig

Lining up the +='s with whitespace goes against our normal code style guidelines, so it may not be a good idea in build scripts either.

The duplication of the INCLUDEPATH and gdk-port:* settings in WebCore.pro and WebKit.pri isn't ideal. It'd be great if there were some way around it.

[Mark Rowe (bdash)]

Comment on attachment 14292 [details]
Scope the qt-port. To allow other ports to reuse the qmake system

This looks ok to me but I'm not a qmake expert. I'll let Zack decide whether this change is correct.

[Holger Freyther]

(In reply to comment #14)
> (From update of attachment 14291 [details] [edit])
> Why is this not needed for the Qt port already?
I don't know for sure. The build-webkit forces one to name the directory of the checkout WebKit. I happened to have directories with the name of WebKit-work-on-that and was forced to invoke qmake manually. Probably due my weird invocation and path setup I needed the patch. I argue that the patch in itself is correct (probably unusual) and allows me to build with my setup.

Yesterday I found the root-cause and wrote about it in Comment #7. The build-webkit script appends ../ until the directory is called WebKit. But the name of my checkout is WebKit-work-on-xyz so this part of the script fails and will cause malfunction on compilation. I can open another bug report to sanitize the behaviour of build-webkit.

[Holger Freyther]

Created attachment 14311 [details]
Scope the qt-port. To allow other ports to reuse the qmake system

Updated patch.
This makes WebKit.pri contain common options,and WebCore/WebCore.pro includes the WebKit.pri now. This will be used by the Gdk+ port to share these options between the GdkLauncher and the library.

Removes unncessary () from the build-webkit script

Make the qt-port the default port.

[Holger Freyther]

Created attachment 14312 [details]
Make it possible to compile the Gdk port using qmake

Update as requested.

The webkitdirs.pm/build-webkit script should follow the CodingStyle. I have decided to leave the prefix in as this is something both Qt and Gdk port will need to take a look. I have added --gdk support to the script, I was not able to use GetOption as I had to share the option between webkitdirs.pm and build-webkit

Simplify the INCLUDEPATH and other shared resources by sharing them in WebKit.pri

PS: For the Qt port I had to change a method signature from float,float to WebCore::FloatSize in DragImageQt.cpp and for the Gdk I had to add an empty stub for ImageBuffer::context. These two changes will not be found in the above patch.

[Holger Freyther]

Created attachment 14313 [details]
Make it possible to compile the Gdk port using qmake

For some reason the first three lines of the WebCore/WebCore.pro patch was missing.

[Simon Hausmann]

I think the patch looks fine, although I have one suggestion for perhaps a future patch: The duplicated qt-port lines could be simplified.

Old:
qt-port: FOO += BAR
qt-port: BLUB -= MEH
qt-port: SOME += MORE

New:

qt-port {
    FOO += BAR
    BLUB -= MEH
    SOME += MORE
}

I think that would look cleaner.

[Holger Freyther]

(In reply to comment #21)
> I think the patch looks fine, although I have one suggestion for perhaps a
> future patch: The duplicated qt-port lines could be simplified.

Simon, feel free to either poke zack to review:+ or do a review:- yourself and I can update to use 'scope {}'.

[Zack Rusin]

Comment on attachment 14311 [details]
Scope the qt-port. To allow other ports to reuse the qmake system

Of course without the DragImageQt.cpp patch.

Plus technically I'd give it a '-' because I wanted sources ordered alphabetically like they were and your patch breaks that but I don't feel like looking at this again. Please fix it after this lands.

[Holger Freyther]

Created attachment 14322 [details]
Folded patch to add Gdk port to the qmake buildsystem

Folded all three patches into one and generated the ChangeLog. Does this need review or can it be landed directly?
A new patch taken Simon's and Zack's comment into account will follow atfer this  patch has been landed.

[Simon Hausmann]

Landed in r21235. Zack, can you close this bug? (my bugzilla karma is not sufficient)