Bug 182286 - [WPE] Update jhbuild dependencies
Summary: [WPE] Update jhbuild dependencies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WPE WebKit (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Charlie Turner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-30 04:21 PST by Charlie Turner
Modified: 2018-01-31 08:28 PST (History)
6 users (show)

See Also:


Attachments
Patch (5.79 KB, patch)
2018-01-30 04:28 PST, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (5.85 KB, patch)
2018-01-30 04:37 PST, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (9.93 KB, patch)
2018-01-30 10:50 PST, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch (10.00 KB, patch)
2018-01-30 11:59 PST, Charlie Turner
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-sierra-wk2 (2.72 MB, application/zip)
2018-01-30 13:43 PST, EWS Watchlist
no flags Details
Patch (9.99 KB, patch)
2018-01-31 05:19 PST, Charlie Turner
no flags Details | Formatted Diff | Diff
Patch for landing (9.99 KB, patch)
2018-01-31 05:19 PST, Charlie Turner
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Charlie Turner 2018-01-30 04:21:03 PST
[WPE] Update jhbuild dependencies
Comment 1 Charlie Turner 2018-01-30 04:28:35 PST
Created attachment 332648 [details]
Patch
Comment 2 Charlie Turner 2018-01-30 04:37:35 PST
Created attachment 332649 [details]
Patch
Comment 3 Michael Catanzaro 2018-01-30 06:14:15 PST
Comment on attachment 332649 [details]
Patch

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

> Tools/wpe/jhbuild.modules:61
> +    <branch module="/pub/GNOME/sources/glib/2.54/glib-2.54.3.tar.xz" version="2.54.3"

This will break RunLoop, see https://bugzilla.gnome.org/show_bug.cgi?id=761102. It looks OK if you add the patch for this.
Comment 4 Charlie Turner 2018-01-30 10:50:41 PST
Created attachment 332668 [details]
Patch
Comment 5 Carlos Alberto Lopez Perez 2018-01-30 11:00:16 PST
Comment on attachment 332668 [details]
Patch

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

> Tools/ChangeLog:25
> +
> +        The initial reason for performing these upgrades was that when
> +        visiting https://youtube.com, WPE was getting TLS certificate
> +        errors. After upgrading glib-networking, these were fixed, but the
> +        upgrade introduced dependencies on newer versions of the other
> +        packages upgraded in this commit.
> +
> +        The upgrade to glib caused a linking error in gstreamer, the
> +        following errors were being logged during linking,
> +
> +        //usr/lib/x86_64-linux-gnu/libpangoft2-1.0.so.0: undefined reference to `hb_glib_script_from_script'
> +        //usr/lib/x86_64-linux-gnu/libpangoft2-1.0.so.0: undefined reference to `hb_glib_get_unicode_funcs'
> +        //usr/lib/x86_64-linux-gnu/libpangoft2-1.0.so.0: undefined reference to `hb_glib_script_to_script'
> +
> +        This was fixed by removing the --with-glib=no from the harfbuzz
> +        package.

....

> Tools/wpe/jhbuild.modules:122
>    <autotools id="harfbuzz" autogen-sh="configure"
> -        autogenargs="--with-cairo=no --with-glib=no --with-freetype=yes --with-fontconfig=yes">
> +        autogenargs="--with-cairo=no --with-freetype=yes --with-fontconfig=yes">
>      <dependencies>
>        <dep package="freetype6"/>
>        <dep package="fontconfig"/>

Why is better passing "--with-glib=no" to the configure instead of adding a '<dep package="glib">' entry in the module build dependencies ?
Comment 6 Charlie Turner 2018-01-30 11:59:55 PST
Created attachment 332678 [details]
Patch
Comment 7 Charlie Turner 2018-01-30 12:01:43 PST
(In reply to Carlos Alberto Lopez Perez from comment #5)
> Comment on attachment 332668 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=332668&action=review
> 
> > Tools/ChangeLog:25
> > +
> > +        The initial reason for performing these upgrades was that when
> > +        visiting https://youtube.com, WPE was getting TLS certificate
> > +        errors. After upgrading glib-networking, these were fixed, but the
> > +        upgrade introduced dependencies on newer versions of the other
> > +        packages upgraded in this commit.
> > +
> > +        The upgrade to glib caused a linking error in gstreamer, the
> > +        following errors were being logged during linking,
> > +
> > +        //usr/lib/x86_64-linux-gnu/libpangoft2-1.0.so.0: undefined reference to `hb_glib_script_from_script'
> > +        //usr/lib/x86_64-linux-gnu/libpangoft2-1.0.so.0: undefined reference to `hb_glib_get_unicode_funcs'
> > +        //usr/lib/x86_64-linux-gnu/libpangoft2-1.0.so.0: undefined reference to `hb_glib_script_to_script'
> > +
> > +        This was fixed by removing the --with-glib=no from the harfbuzz
> > +        package.
> 
> ....

I don't know what your dots mean, but I rephrased this sentence.

> 
> > Tools/wpe/jhbuild.modules:122
> >    <autotools id="harfbuzz" autogen-sh="configure"
> > -        autogenargs="--with-cairo=no --with-glib=no --with-freetype=yes --with-fontconfig=yes">
> > +        autogenargs="--with-cairo=no --with-freetype=yes --with-fontconfig=yes">
> >      <dependencies>
> >        <dep package="freetype6"/>
> >        <dep package="fontconfig"/>
> 
> Why is better passing "--with-glib=no" to the configure instead of adding a
> '<dep package="glib">' entry in the module build dependencies ?

I don't have an answer to that question, I dropped to it rely on the auto package behavior and pick up the glib in the jhbuild. I switched to your suggestion.
Comment 8 Michael Catanzaro 2018-01-30 12:26:50 PST
Comment on attachment 332678 [details]
Patch

OK, but watch the bots when this lands because this is likely to both break and fix many tests. You'll need to update the expectations accordingly....
Comment 9 Charlie Turner 2018-01-30 12:34:09 PST
A(In reply to Michael Catanzaro from comment #8)
> Comment on attachment 332678 [details]
> Patch
> 
> OK, but watch the bots when this lands because this is likely to both break
> and fix many tests. You'll need to update the expectations accordingly....

Agreed, I didn't notice much running the test deltas locally, but I will be checking the results after it lands, which I'll do tomorrow when I can baby sit it.
Comment 10 EWS Watchlist 2018-01-30 13:43:51 PST
Comment on attachment 332678 [details]
Patch

Attachment 332678 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/6264721

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/navigation-redirect.https.html
Comment 11 EWS Watchlist 2018-01-30 13:43:53 PST
Created attachment 332697 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 12 Charlie Turner 2018-01-31 05:19:12 PST
Created attachment 332757 [details]
Patch

Rebase patch
Comment 13 Charlie Turner 2018-01-31 05:19:36 PST
Created attachment 332758 [details]
Patch for landing
Comment 14 Charlie Turner 2018-01-31 06:18:47 PST
Is this patch stuck in the commit queue because of comment 11 (which surely can't be caused by this patch, surely!)

https://trac.webkit.org/wiki/CommitQueue
Q: How long until a patch lands after I set commit-queue+?

10-15 minutes. Depends on if the ​tree is red or not. The commit-queue will not commit when buildbots for the ​core platforms are red.
Comment 15 WebKit Commit Bot 2018-01-31 07:55:41 PST
Comment on attachment 332758 [details]
Patch for landing

Clearing flags on attachment: 332758

Committed r227900: <https://trac.webkit.org/changeset/227900>
Comment 16 WebKit Commit Bot 2018-01-31 07:55:43 PST
All reviewed patches have been landed.  Closing bug.
Comment 17 Michael Catanzaro 2018-01-31 08:28:14 PST
The wiki page is wrong/obsolete. commit-queue normally takes an hour or thereabouts.