[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#988814: unblock: gtk+2.0/2.24.33-2



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
X-Debbugs-Cc: debian-gtk-gnome@lists.debian.org, debian-boot@lists.debian.org

Please unblock package gtk+2.0

[ Reason ]
Fix the graphical installer

[ Impact ]
* For the patch:
  If not applied, several scenarios cause the graphical installer to
  busy-loop and get stuck, unless we fix that some other way (such as
  a patch I have proposed for cdebconf). This creates a bad impression
  for new users and is clearly RC.

* For the changes in d/control{,.in}:
  If not applied, users who already have gnome-icon-theme installed could
  accidentally install GTK 2 apps without also installing librsvg2-common,
  and then complain that SVG icons don't work (on most systems it gets
  pulled in by adwaita-icon-theme but it's technically possible to not
  install that).

  Note that it's a Recommends and not a Depends, primarily to be nice
  to -ports architectures that lack Rust, secondarily to be nice to
  special-purpose minimal installations. Ordinary desktops that have GTK
  should definitely install this.

[ Tests ]
Cyril Brulebois, Philip Hands and I have tested the installer in a
few scenarios that previously got stuck, with an earlier build of
this package. This patched GTK fixes them, while printing warnings;
the UI looks reasonable.

A brief smoke-test of the final .deb with GIMP and Inkscape was also
successful.

strings /usr/lib/x86_64-linux-gnu/libgtk-x11-2.0.so indicates that the
hack for d-i has not affected the .deb's library.

There are also autopkgtests, which pass.

[ Risks ]
The patch is #ifdef DEBIAN_INSTALLER so it can't affect the installed
system, only the graphical installer (which is already considered
unreleasable due to the bug we're addressing here, so this isn't going
to make it worse). There is a risk that it might cut off the last few
pixels on the right side of a paragraph, or make the last few pixels
overlap the next widget to the right (or left in RTL languages), but
that seems a lot better than the status quo, and in practice it doesn't
actually happen for reasons I don't intend to bore you with.

The changes in d/control{,.in} just upgrade a Suggests to a Recommends,
mirroring similar changes in GTK 3. It was a transitive Recommends
already, via adwaita-icon-theme. Low risk, and easy to revert in the
unlikely event that it's a problem.

[ Checklist ]
  [x] all changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in testing

[ Other info ]
Given the impact of the d-i issue, I think it's safest to address it
in cdebconf (Cyril will probably handle that upload) *and* address it here.

unblock gtk+2.0/2.24.33-2
diffstat for gtk+2.0-2.24.33 gtk+2.0-2.24.33

 changelog                                                                   |   22 +++++
 control                                                                     |    4 -
 control.in                                                                  |    4 -
 patches/d-i/textlayout-Clamp-width-to-the-value-we-asked-for-as-a-hac.patch |   39 ++++++++++
 patches/series                                                              |    1 
 rules                                                                       |    1 
 6 files changed, 67 insertions(+), 4 deletions(-)

diff -Nru gtk+2.0-2.24.33/debian/changelog gtk+2.0-2.24.33/debian/changelog
--- gtk+2.0-2.24.33/debian/changelog	2020-12-28 16:50:40.000000000 +0000
+++ gtk+2.0-2.24.33/debian/changelog	2021-05-19 17:13:33.000000000 +0100
@@ -1,3 +1,25 @@
+gtk+2.0 (2.24.33-2) unstable; urgency=medium
+
+  * Team upload
+  * d/rules: Build udeb with extra CPPFLAGS.
+    This allows adding special-case code for debian-installer where
+    necessary.
+  * udeb: Clamp text layout width to no more than was requested.
+    This works around a relayout loop that makes the Debian installer hang.
+    To minimize the effect on installed systems, this is
+    #ifdef DEBIAN_INSTALLER; we currently have no evidence of similar
+    relayout loops outside the d-i environment. (Closes: #988786)
+  * Increase dependency on librsvg2-common from Suggests to Recommends.
+    This is not a hard dependency, but should be installed in nearly all
+    cases. Increasingly many icons are provided in SVG format, so
+    applications will appear broken if the SVG pixbuf loader is not
+    installed. See #980396 for more information.
+    adwaita-icon-theme already Recommends librsvg2-common, but people who
+    routinely do not install recommended packages will get a better hint
+    about how much will be broken by its removal if GTK also recommends it.
+
+ -- Simon McVittie <smcv@debian.org>  Wed, 19 May 2021 17:13:33 +0100
+
 gtk+2.0 (2.24.33-1) unstable; urgency=medium
 
   * Team upload
diff -Nru gtk+2.0-2.24.33/debian/control gtk+2.0-2.24.33/debian/control
--- gtk+2.0-2.24.33/debian/control	2020-12-28 16:50:40.000000000 +0000
+++ gtk+2.0-2.24.33/debian/control	2021-05-19 17:13:33.000000000 +0100
@@ -63,9 +63,9 @@
          shared-mime-info
 Provides: gtk2.0-binver-2.10.0
 Recommends: libgail-common,
+            librsvg2-common,
             libgtk2.0-bin
-Suggests: librsvg2-common,
-          gvfs
+Suggests: gvfs
 Multi-Arch: same
 Description: GTK graphical user interface library - old version
  GTK is a multi-platform toolkit for creating graphical user
diff -Nru gtk+2.0-2.24.33/debian/control.in gtk+2.0-2.24.33/debian/control.in
--- gtk+2.0-2.24.33/debian/control.in	2020-12-28 16:50:40.000000000 +0000
+++ gtk+2.0-2.24.33/debian/control.in	2021-05-19 17:13:33.000000000 +0100
@@ -63,9 +63,9 @@
          shared-mime-info
 Provides: @GTK_BINVER_DEP@
 Recommends: libgail-common,
+            librsvg2-common,
             @BIN_PKG@
-Suggests: librsvg2-common,
-          gvfs
+Suggests: gvfs
 Multi-Arch: same
 Description: GTK graphical user interface library - old version
  GTK is a multi-platform toolkit for creating graphical user
diff -Nru gtk+2.0-2.24.33/debian/patches/d-i/textlayout-Clamp-width-to-the-value-we-asked-for-as-a-hac.patch gtk+2.0-2.24.33/debian/patches/d-i/textlayout-Clamp-width-to-the-value-we-asked-for-as-a-hac.patch
--- gtk+2.0-2.24.33/debian/patches/d-i/textlayout-Clamp-width-to-the-value-we-asked-for-as-a-hac.patch	1970-01-01 01:00:00.000000000 +0100
+++ gtk+2.0-2.24.33/debian/patches/d-i/textlayout-Clamp-width-to-the-value-we-asked-for-as-a-hac.patch	2021-05-19 17:13:33.000000000 +0100
@@ -0,0 +1,39 @@
+From: Simon McVittie <smcv@debian.org>
+Date: Wed, 19 May 2021 11:36:58 +0100
+Subject: textlayout: Clamp width to the value we asked for, as a hack for d-i
+
+When we ask Pango to lay out text with a particular width in mind, if
+the width is really narrow then reducing it can result in Pango asking
+for *more* space. This can result in a relayout loop in the Debian
+installer. Avoid this by restricting the width to be no more than what
+we asked for, which might result in text being clipped or overlapping
+with an adjacent widget but is better than an infinite loop.
+
+Signed-off-by: Simon McVittie <smcv@debian.org>
+Bug-Debian: https://bugs.debian.org/988786
+---
+ gtk/gtktextlayout.c | 11 +++++++++++
+ 1 file changed, 11 insertions(+)
+
+diff --git a/gtk/gtktextlayout.c b/gtk/gtktextlayout.c
+index 31139d7..f8a44c0 100644
+--- a/gtk/gtktextlayout.c
++++ b/gtk/gtktextlayout.c
+@@ -2472,6 +2472,17 @@ gtk_text_layout_get_line_display (GtkTextLayout *layout,
+   display->width = PIXEL_BOUND (extents.width) + display->left_margin + display->right_margin;
+   display->height += PANGO_PIXELS (extents.height);
+ 
++#ifdef DEBIAN_INSTALLER
++  if (display->total_width > 0 && display->width > display->total_width)
++    {
++      g_warning ("%s: we asked Pango to wrap text for width %dpx but it "
++                 "now wants %dpx. Clamping result to %dpx!",
++                 G_STRFUNC, display->total_width, display->width,
++                 display->total_width);
++      display->width = display->total_width;
++    }
++#endif
++
+   /* If we aren't wrapping, we need to do the alignment of each
+    * paragraph ourselves.
+    */
diff -Nru gtk+2.0-2.24.33/debian/patches/series gtk+2.0-2.24.33/debian/patches/series
--- gtk+2.0-2.24.33/debian/patches/series	2020-12-28 16:50:40.000000000 +0000
+++ gtk+2.0-2.24.33/debian/patches/series	2021-05-19 17:13:33.000000000 +0100
@@ -7,3 +7,4 @@
 061_use_pdf_as_default_printing_standard.patch
 098_multiarch_module_path.patch
 Reinstate-marshallers-that-accidentally-became-part-of-th.patch
+d-i/textlayout-Clamp-width-to-the-value-we-asked-for-as-a-hac.patch
diff -Nru gtk+2.0-2.24.33/debian/rules gtk+2.0-2.24.33/debian/rules
--- gtk+2.0-2.24.33/debian/rules	2020-12-28 16:50:40.000000000 +0000
+++ gtk+2.0-2.24.33/debian/rules	2021-05-19 17:13:33.000000000 +0100
@@ -119,6 +119,7 @@
 			--enable-introspection \
 			--enable-man
 shared_udeb_configure_flags := $(configure_flags) \
+			CPPFLAGS="$(CPPFLAGS) -DDEBIAN_INSTALLER" \
 			--disable-introspection \
 			--disable-xcomposite \
 			--disable-xdamage \

Reply to: