Bug 163689 - [GTK] Bump glib to 2.50.1 (or find alternative solution to Gtk+ CUPS build issue)
Summary: [GTK] Bump glib to 2.50.1 (or find alternative solution to Gtk+ CUPS build is...
Status: RESOLVED LATER
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Manuel Rego Casasnovas
URL:
Keywords:
Depends on: 163765 164070
Blocks: 163591
  Show dependency treegraph
 
Reported: 2016-10-19 13:12 PDT by Manuel Rego Casasnovas
Modified: 2016-12-08 00:18 PST (History)
7 users (show)

See Also:


Attachments
Patch (4.71 KB, patch)
2016-10-19 13:13 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Adding --disable-libmount (4.94 KB, patch)
2016-10-20 00:13 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (3.18 KB, patch)
2016-10-27 09:46 PDT, Joanmarie Diggs (irc: joanie)
no flags Details | Formatted Diff | Diff
Patch (3.16 KB, patch)
2016-10-27 10:40 PDT, Joanmarie Diggs (irc: joanie)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel Rego Casasnovas 2016-10-19 13:12:15 PDT
In order to fix bug #163591, we need to apply an upstream patch into GTK+.
But GTK+ won't build anymore with that patch unless glib is in version 2.50.

So first we need to bump glib version and then we could fix the GTK+ and CUPS issue.
Comment 1 Manuel Rego Casasnovas 2016-10-19 13:13:19 PDT
Created attachment 292101 [details]
Patch
Comment 2 Michael Catanzaro 2016-10-19 13:42:16 PDT
Comment on attachment 292101 [details]
Patch

You need --disable-libmount to fix the build.

Please watch the test bots after you land this, just in case.
Comment 3 Manuel Rego Casasnovas 2016-10-20 00:13:35 PDT
Created attachment 292156 [details]
Adding --disable-libmount
Comment 4 WebKit Commit Bot 2016-10-20 01:32:57 PDT
Comment on attachment 292156 [details]
Adding --disable-libmount

Clearing flags on attachment: 292156

Committed r207589: <http://trac.webkit.org/changeset/207589>
Comment 5 WebKit Commit Bot 2016-10-20 01:33:01 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Michael Catanzaro 2016-10-20 16:37:31 PDT
It's too many new failures for now, when we're trying to get the bot green. If you want to try gardening them all, then we can try landing again, otherwise we can try this again once we get the bot green.
Comment 7 WebKit Commit Bot 2016-10-20 16:42:13 PDT
Re-opened since this is blocked by bug 163765
Comment 8 Carlos Garcia Campos 2016-10-20 22:18:23 PDT
(In reply to comment #6)
> It's too many new failures for now, when we're trying to get the bot green.
> If you want to try gardening them all, then we can try landing again,
> otherwise we can try this again once we get the bot green.

I'm surprised a glib upgrade caused differences in layout test results :-(
Comment 9 Manuel Rego Casasnovas 2016-10-21 00:55:24 PDT
We should probably rollout bug #163591 too, because otherwise you couldn't build GTK+ right now. :-/
Comment 10 Joanmarie Diggs (irc: joanie) 2016-10-27 09:46:52 PDT
Created attachment 293029 [details]
Patch
Comment 11 Michael Catanzaro 2016-10-27 09:56:31 PDT
Comment on attachment 293029 [details]
Patch

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

> Tools/gtk/jhbuild.modules:211
> +      <patch file="glib-use-upstream-gettext.patch" strip="1"/>

I don't get it, the patch doesn't do anything to switch to upstream gettext, so it's named wrongly at least. It looks like it just adds a deprecation warning.
Comment 12 Michael Catanzaro 2016-10-27 10:17:07 PDT
If the patch really fixes the build, then you could name it glib-deprecate-am-glib-gnu-gettext.patch

As background for the autotools stuff: AM_GLIB_GNU_GETTEXT is a downstream macro that GNOME used to use instead of AM_GNU_GETTEXT, the upstream macro, before intltool. AM_GNU_GETTEXT (gettext), AM_GLIB_GNU_GETTEXT (glib-gettext), and IT_PROG_INTLTOOL (intltool) are all different slightly-incompatible ways of translating applications. glib-gettext and intltool exist to work around gettext not supporting various important features. Nowadays gettext can do everything we need in GNOME, so we don't need the downstream things anymore. The patch you have here looks like it just adds a build warning to warn maintainers to stop using the AM_GLIB_GNU_GETTEXT macro and start using AM_GNU_GETTEXT instead. Actually doing that requires more work. GLib has itself not yet made the transition; if you check the git history, Javier tried multiple times to start using upstream gettext, but each time it got reverted, so GLib is still itself using glib-gettext. The confusion here is that the commit message of the patch deprecating AM_GLIB_GNU_GETTEXT does say "use upstream gettext" but it is actually a directive to users of GLib to start using upstream gettext, whereas you (reasonably) interpreted it to mean that GLib itself now uses upstream gettext directly.

All of the above is one good reason why meson now exists. :)
Comment 13 Joanmarie Diggs (irc: joanie) 2016-10-27 10:19:37 PDT
*** Configuring gtk+ *** [18/40]
/home/jd/checkout/WebKitGtk/WebKitBuild/DependenciesGTK/Source/gtk+-3.16.4/autogen.sh --prefix /home/jd/checkout/WebKitGtk/WebKitBuild/DependenciesGTK/Root --disable-introspection --enable-introspection --disable-Werror 
autoreconf: Entering directory `.'
autoreconf: configure.ac: not using Gettext
autoreconf: running: aclocal --force -I m4 ${ACLOCAL_FLAGS}
/home/jd/checkout/WebKitGtk/WebKitBuild/DependenciesGTK/Root/share/aclocal/glib-gettext.m4:39: error: m4_copy: won't overwrite defined macro: glib_DEFUN
/home/jd/checkout/WebKitGtk/WebKitBuild/DependenciesGTK/Root/share/aclocal/glib-gettext.m4:39: the top level
autom4te: /usr/bin/m4 failed with exit status: 1
aclocal: error: echo failed with exit status: 1
autoreconf: aclocal failed with exit status: 1
Failed to build GTK+ port dependencies with jhbuild
Died at ./Tools/Scripts/update-webkitgtk-libs line 24.
Comment 14 Joanmarie Diggs (irc: joanie) 2016-10-27 10:40:02 PDT
Created attachment 293036 [details]
Patch
Comment 15 WebKit Commit Bot 2016-10-27 11:17:04 PDT
Comment on attachment 293036 [details]
Patch

Clearing flags on attachment: 293036

Committed r207992: <http://trac.webkit.org/changeset/207992>
Comment 16 WebKit Commit Bot 2016-10-27 11:17:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 17 WebKit Commit Bot 2016-10-27 12:42:20 PDT
Re-opened since this is blocked by bug 164070
Comment 18 Philippe Normand 2016-11-16 01:08:28 PST
Why was this rolled out? The rollout patch mentions "aclocal version conflicts" which is a vague statement, at best.
Comment 19 Philippe Normand 2016-11-16 01:33:57 PST
(In reply to comment #13)
> *** Configuring gtk+ *** [18/40]
> /home/jd/checkout/WebKitGtk/WebKitBuild/DependenciesGTK/Source/gtk+-3.16.4/
> autogen.sh --prefix
> /home/jd/checkout/WebKitGtk/WebKitBuild/DependenciesGTK/Root
> --disable-introspection --enable-introspection --disable-Werror 
> autoreconf: Entering directory `.'
> autoreconf: configure.ac: not using Gettext
> autoreconf: running: aclocal --force -I m4 ${ACLOCAL_FLAGS}
> /home/jd/checkout/WebKitGtk/WebKitBuild/DependenciesGTK/Root/share/aclocal/
> glib-gettext.m4:39: error: m4_copy: won't overwrite defined macro: glib_DEFUN
> /home/jd/checkout/WebKitGtk/WebKitBuild/DependenciesGTK/Root/share/aclocal/
> glib-gettext.m4:39: the top level
> autom4te: /usr/bin/m4 failed with exit status: 1
> aclocal: error: echo failed with exit status: 1
> autoreconf: aclocal failed with exit status: 1
> Failed to build GTK+ port dependencies with jhbuild
> Died at ./Tools/Scripts/update-webkitgtk-libs line 24.

With this patch and the patch for gtk+3.16 applied I still get this error here.
Comment 20 Philippe Normand 2016-11-16 01:54:58 PST
Huh nevermind I also get this error without the patch :S
Comment 21 Philippe Normand 2016-11-16 02:00:48 PST
Forgot to remove Source/glib* before rebuilding. Sorry for the spam here :)
Comment 22 Michael Catanzaro 2016-11-16 08:35:34 PST
(In reply to comment #18)
> Why was this rolled out? The rollout patch mentions "aclocal version
> conflicts" which is a vague statement, at best.

It broke the buildbots since the glib tarball was generated with a different version of Automake than is installed on the buildbots. To try landing again, I would try removing autogen-sh="configure" from the glib module -- that should do the trick.

Of course, best of all would be to help gardening so that we can upgrade glib.
Comment 23 Philippe Normand 2016-12-08 00:18:33 PST
I suppose this patch is no longer needed since #163591 is fixed.