Bug 16618 – [GTK] build-webkit and run-webkit-tests autootools support

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Jan Alonzo]

Add support for autotools at least in the build-webkit and run-webkit-tests scripts.

[Jan Alonzo]

Created attachment 18539 [details]
autotools support for webkit tools

The attached patch adds support for the autotools build system. It's currently gtk-centric. Also, Since gtk+ can be built using 2 build systems, this patch checks if WEBKIT_AUTOTOOLS is set, then uses autotools if it is.

Cheers

[Alp Toker]

I (In reply to comment #1)
> Created an attachment (id=18539) [edit]
> autotools support for webkit tools
> 
> The attached patch adds support for the autotools build system. It's currently
> gtk-centric. Also, Since gtk+ can be built using 2 build systems, this patch
> checks if WEBKIT_AUTOTOOLS is set, then uses autotools if it is.
> 
> Cheers
> 

I landed the autotools part of this patch in r29670. Didn't have time to look over the scripts changes yet.

[Jan Alonzo]

Thanks alp.

[Jan Alonzo]

Created attachment 18564 [details]
updated patch

updated patch to make autotools not specific to gtk. also made some changes based on bdash and alp's feedback in #webkit-gtk.

[Mark Rowe (bdash)]

This is looking good.  One comment I have is that trying to align code with whitespace like:

+    push @options, $databaseSupport         ? "--enable-database"           : "--disable-database";

is messy.  In this instance, I think you could extract the conditional into a small helper function that generates the argument, so you could use it like:

    push @options, featureFlag($databaseSupport, "database");

(feel free to pick a better name, I didn't think too hard about that one).

It'd be good if you could add the missing space in the following line and remove the mention of Qt from it while you're touching the code around it:

797 846         die "The Gtk portbuilds JavaScriptCore/WebCore/WebKitQt in one shot! Only call it for 'WebKit'.\n";

[Jan Alonzo]

Created attachment 18576 [details]
updated patch to cleanup feature flags

Hi bdash, thanks for the feedback. I named it to autotoolsFlag to distinguish it from other build system flags. If you have a better naming suggestion, then just let me know. I also added the space and remove Qt as you suggested.

regards,

[Mark Rowe (bdash)]

Comment on attachment 18576 [details]
updated patch to cleanup feature flags

Looks good to me.

+        my @buildArgs = {"CONFIG+=gtk-port"};
+        push @buildArgs, "CONFIG-=qt";

This feels a little bit inconsistent, but I guess it's not really a big deal.

[Mark Rowe (bdash)]

Landed in r29993.