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

Re: More cdebconf patches


I'm sorry I didn't follow up to earlier mails about cdebconf patches,
haven't been able to process enough other requests…

Regis Boudin <regis@boudin.name> (2016-01-10):
> I just pushed a couple of patches for cdebconf-gtk. Besides removing
> conditional code to handle old versions of glib, the main change is
> for the display of text in the banner, done directly with cairo, thus
> removing the last bit of deprecated GDK code. I believe I've
> reproduced the same behaviour ; but if you notice something different
> I might have missed, please shout.

IIRC we mostly use this for things like rescue mode, did you check this
with various languages/fonts (esp. CJK)?

> The next patch could be the one attached. It makes it possible to
> chose at configure time to compile for either GTK+2 or GTK+3, and in
> this version compiles the udeb for GTK+2, and the deb for GTK+3.

Great, I suppose this totally obsoletes my pu/gtk3 branch then. I'm not
totally convinced by building one with gtk2 and the other with gtk3

Some things which come to mind for a gtk3 switch in d-i:
 - we need theming
 - we need to check theme=dark has some equivalent
 - we now need to check that ctrl-+/- still works
 - we need to update the freeze hints
 - we need to make sure there are no uninstallable udebs; this used to
   be the case, that was solved a few times, but we now have a bogus
   dependency on libepoxy0 (#788703).
 - [ what am I forgetting? ]

Once everything is ready I think switching both cdebconf deb/udeb to
gtk3 at the same time would make more sense. (OTOH getting some gtk3
testing through the deb would be nice; but I'm not sure how common that
is, and how useful that'll be to test it from outside d-i.)

> Unfortunately the #if in the code are rather ugly ; but my attempts at
> making it cleaner ended with something even less readable, so I left
> them that way. And there's only 6 occurrences, so it's not that bad.

Those are fine with me.

> Any thought about whether I should merge this patch ? If yes, about
> enabling GTK+3 for the deb ?

The src/modules/frontend/gtk/Makefile.in changes look like something
that might have been needed during development, but that shouldn't be
needed anymore now that we have a clean patch?

For configure.ac, I think I'd prefer if we could align things a bit
to get better readability (but that's entirely subjective, and not a
reason not to merge the patch in its current state). I've tweaked this
part of your diff to show what I mean:

> --- a/configure.ac
> +++ b/configure.ac
> @@ -148,15 +148,18 @@ if test -z "$FRONTEND_MODULES"; then
>      PKG_CHECK_EXISTS([slang], [FRONTEND_MODULES="$FRONTEND_MODULES slang"], [AC_MSG_WARN([cannot build Slang frontend])])
>  fi
>  for frontend in $FRONTEND_MODULES; do
>      case $frontend in
> -      gtk) PKG_CHECK_MODULES([GTK_X11],[glib-2.0 >= 2.31 gtk+-x11-2.0 >= 2.24 pango gdk-pixbuf-2.0]);;
> -      ncurses) PKG_CHECK_MODULES([NCURSES],[ncurses]);;
> -      newt) PKG_CHECK_MODULES([NEWT],[libnewt slang]);;
> -      slang) PKG_CHECK_MODULES([SLANG],[slang]);;
> -      *);;
> +      gtk|gtk2) PKG_CHECK_MODULES([GTK_X11],[glib-2.0 >= 2.31 gtk+-x11-2.0 >= 2.24 pango gdk-pixbuf-2.0],
> +                                  [FRONTEND_MODULES_BUILD="$FRONTEND_MODULES_BUILD gtk"]);;
> +      gtk3)     PKG_CHECK_MODULES([GTK_X11],[glib-2.0 >= 2.31 gtk+-x11-3.0 pango gdk-pixbuf-2.0],
> +                                  [FRONTEND_MODULES_BUILD="$FRONTEND_MODULES_BUILD gtk"]);;
> +      ncurses)  PKG_CHECK_MODULES([NCURSES],[ncurses],
> +                                  [FRONTEND_MODULES_BUILD="$FRONTEND_MODULES_BUILD ncurses"]);;
> +      newt)     PKG_CHECK_MODULES([NEWT],[libnewt slang],
> +                                  [FRONTEND_MODULES_BUILD="$FRONTEND_MODULES_BUILD newt"]);;
> +      slang)    PKG_CHECK_MODULES([SLANG],[slang],
> +                                  [FRONTEND_MODULES_BUILD="$FRONTEND_MODULES_BUILD slang"]);;
>      esac
>  done


Attachment: signature.asc
Description: Digital signature

Reply to: