Bug 62749 – [GTK] Build WebKit2 unconditionally

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Martin Robinson]

We should build WebKit2 unconditionally. This has a couple big benefits:

1. This is how other ports work and many of the tools expect our build to work this way.
2. The bots would start detecting WebKit2 build issues.

[Martin Robinson]

Created attachment 97350 [details]
Patch

[Xan Lopez]

As commented on jabber, I'd like to see numbers of the difference in build times. I don't think we should regress significantly here, we are already doing (quite) badly.

[Martin Robinson]

Here are my build times before an after the application of this patch. 

Before:
===========================================================
 WebKit is now built (09m:13s). 
 To run GtkLauncher with this newly-built code, use the
 "Tools/Scripts/run-launcher" script.
===========================================================

After:

===========================================================
 WebKit is now built (10m:09s). 
 To run GtkLauncher with this newly-built code, use the
 "Tools/Scripts/run-launcher" script.
===========================================================

[Martin Robinson]

Perhaps a more illustrative example would be the difference in incremental build time after touching a single file in WebCore:

Before:

==========================================================
 WebKit is now built (00m:30s). 
 To run GtkLauncher with this newly-built code, use the
 "Tools/Scripts/run-launcher" script.
===========================================================

After:

===========================================================
 WebKit is now built (00m:32s). 
 To run GtkLauncher with this newly-built code, use the
 "Tools/Scripts/run-launcher" script.
===========================================================

So really the overhead is not something that should affect most builds.

[Carlos Garcia Campos]

Comment on attachment 97350 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=97350&action=review

You should also remove the AC_ARG_ENABLE(webkit2, But instead of removing it, I would simply change the default option to enable it by default, so that people can still disable it if they want to save build time or simply don't need wk2. Changing the AC_ARG_ENABLE macro to be something like this would be enough:

AC_ARG_ENABLE(webkit2,
            AC_HELP_STRING([--enable-webkit2],
                [build webkit2 [default=yes]]),
                [], [enable_webkit2="yes"])

> Source/WebKit2/GNUmakefile.am:-835
> -pkgconfig_DATA += Source/WebKit2/gtk/@WEBKITGTK_PC_NAME@2-@WEBKITGTK_API_VERSION@.pc
> -

why removing the pc file?

[Martin Robinson]

(In reply to comment #5)
> (From update of attachment 97350 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=97350&action=review
> 
> You should also remove the AC_ARG_ENABLE(webkit2, But instead of removing it, I would simply change the default option to enable it by default, so that people can still disable it if they want to save build time or simply don't need wk2. Changing the AC_ARG_ENABLE macro to be something like this would be enough:

Okay. I've done this. This made the patch quite a bit smaller.

> why removing the pc file?

Since it's on by default now, we shouldn't ship the PC file until we are ready to ship WebKit2 officially.

[Martin Robinson]

Created attachment 97837 [details]
Patch

[Collabora GTK+ EWS bot]

Comment on attachment 97837 [details]
Patch

Attachment 97837 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8907732

[Martin Robinson]

Looks like we should make GTK+ 3.x the default on the bots before landing this patch.

[Carlos Garcia Campos]

(In reply to comment #9)
> Looks like we should make GTK+ 3.x the default on the bots before landing this patch.

https://bugs.webkit.org/show_bug.cgi?id=63047

[Xan Lopez]

Comment on attachment 97837 [details]
Patch

OK.

[Martin Robinson]

Committed r94000: <http://trac.webkit.org/changeset/94000>

[Carlos Garcia Campos]

(In reply to comment #12)
> Committed r94000: <http://trac.webkit.org/changeset/94000>

This patch removes the rules to create the pkg-config file from configure and makefiles. That was already reworked in http://trac.webkit.org/changeset/92263, and it was part of the preparation for the gtk api. Would it be possible to add it again, please?

[Martin Robinson]

(In reply to comment #13)
> (In reply to comment #12)
> > Committed r94000: <http://trac.webkit.org/changeset/94000>
> 
> This patch removes the rules to create the pkg-config file from configure and makefiles. That was already reworked in http://trac.webkit.org/changeset/92263, and it was part of the preparation for the gtk api. Would it be possible to add it again, please?

I think it's important to disable installing pkgconfig files until we officially ship WebKit2. Would it be acceptable to re-enable it behind a configure flag? Another option would be to only enable WebKit2 when running build-webkit. What do you think?

[Carlos Garcia Campos]

(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > Committed r94000: <http://trac.webkit.org/changeset/94000>
> > 
> > This patch removes the rules to create the pkg-config file from configure and makefiles. That was already reworked in http://trac.webkit.org/changeset/92263, and it was part of the preparation for the gtk api. Would it be possible to add it again, please?
> 
> I think it's important to disable installing pkgconfig files until we officially ship WebKit2. Would it be acceptable to re-enable it behind a configure flag? Another option would be to only enable WebKit2 when running build-webkit. What do you think?

I don't see why, we are currently installing the headers and the .so when webkit is compiled with webkit2, why not install the pc file too? or why don't we disable the installation of the headers and .so then? I don't use build-webkit, if someone doesn't want webkit2 it can still be disabled with --disable-webkit2.