Bug 16618 - [GTK] build-webkit and run-webkit-tests autootools support
Summary: [GTK] build-webkit and run-webkit-tests autootools support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Enhancement
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2007-12-27 00:14 PST by Jan Alonzo
Modified: 2008-02-04 22:30 PST (History)
2 users (show)

See Also:


Attachments
autotools support for webkit tools (13.51 KB, patch)
2008-01-19 00:44 PST, Jan Alonzo
no flags Details | Formatted Diff | Diff
updated patch (12.42 KB, patch)
2008-01-20 02:38 PST, Jan Alonzo
no flags Details | Formatted Diff | Diff
updated patch to cleanup feature flags (13.74 KB, patch)
2008-01-21 10:11 PST, Jan Alonzo
mrowe: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Alonzo 2007-12-27 00:14:42 PST
Add support for autotools at least in the build-webkit and run-webkit-tests scripts.
Comment 1 Jan Alonzo 2008-01-19 00:44:24 PST
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
Comment 2 Alp Toker 2008-01-19 07:53:35 PST
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.
Comment 3 Jan Alonzo 2008-01-19 14:24:48 PST
Thanks alp.
Comment 4 Jan Alonzo 2008-01-20 02:38:53 PST
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.
Comment 5 Mark Rowe (bdash) 2008-01-20 22:43:02 PST
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";

Comment 6 Jan Alonzo 2008-01-21 10:11:10 PST
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,
Comment 7 Mark Rowe (bdash) 2008-01-26 03:00:51 PST
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.
Comment 8 Mark Rowe (bdash) 2008-02-04 22:30:01 PST
Landed in r29993.