Libguestfs - Sep 2022 - [p2v PATCH 7/7] gui.c: annotate GTK_INPUT_PURPOSE_PASSWORD with upstream GTK3 version

Apart from Dan's suggestions in patch 1, the series looks good to me.

FWIW RHEL 7 (the earliest distro with PCRE 2) has glib2 2.56.1 &
gtk3 3.22.30, so supporting any earlier versions also seems pointless,
so that might be another thing to review.  We could make USE_POPOVERS
unconditional, and make gui-gtk3-compat.h considerably less
complicated.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Mon, Sep 26, 2022 at 12:59:21PM +0100, Richard W.M. Jones
wrote:
> 
> Apart from Dan's suggestions in patch 1, the series looks good to me.
> 
> FWIW RHEL 7 (the earliest distro with PCRE 2) has glib2 2.56.1 &
> gtk3 3.22.30, so supporting any earlier versions also seems pointless,
> so that might be another thing to review.  We could make USE_POPOVERS
> unconditional, and make gui-gtk3-compat.h considerably less
> complicated.
Say you pick 3.22 as your official min and check this with pkg-config
in configure, then you can further define

  GDK_VERSION_MIN_REQUIRED=GDK_VERSION_3_22
  GDK_VERSION_MAX_ALLOWED=GDK_VERSION_3_22

The former will give you warnings if you use any API that was already
deprecated in 3.22 - this is something that should be re-written to
use the recommended modern alternative API.

The latter will give you warnings if you use any API that was introduced
after 3.22 - this is to prevent you accidentally introducing usage of
APIs newer than your min version

The same exists for glib via GLIB_VERSION_MIN_REQUIRED/MAX_ALLOWED.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

On 09/26/22 13:59, Richard W.M. Jones wrote:
> 
> Apart from Dan's suggestions in patch 1, the series looks good to me.
> 
> FWIW RHEL 7 (the earliest distro with PCRE 2) has glib2 2.56.1 &
> gtk3 3.22.30, so supporting any earlier versions also seems pointless,
> so that might be another thing to review.  We could make USE_POPOVERS
> unconditional, and make gui-gtk3-compat.h considerably less
> complicated.
I guess I could do this, but I don't feel comfortable about it with the
many other distro families we supposedly support. :/ We don't have such
CI, and build-testing manually is horribly inefficient (cue the
situation with GTK3 -- I've not built p2v with GTK3 thus far even on
Fedora). Debian at least has a package directory on the web, but I've
always had trouble with the other non-Fedora-like distros. (And even
regarding Debian, I don't know what major release we aim at.)

Honestly it doesn't feel like we're making a strong promise in
"dependencies.m4" to these other distros, given that we almost never
build or test on those distros. On the other hand it seriously
complicates things for development.

Laszlo

On 09/26/22 13:59, Richard W.M. Jones wrote:
> 
> Apart from Dan's suggestions in patch 1, the series looks good to me.
I've updated patch#1 (both the code and the commit message), and pushed
the series (28d7ce8c9db9..94f1f9df1994), taking your above statement as
an ACK (hope that's OK).

Because of the update to patch#1, I also retested the shutdown button /
action in the last dialog; for that I needed to go through a full in-VM
conversion... Hrmpf. :)
> FWIW RHEL 7 (the earliest distro with PCRE 2) has glib2 2.56.1 &
> gtk3 3.22.30, so supporting any earlier versions also seems pointless,
> so that might be another thing to review.  We could make USE_POPOVERS
> unconditional, and make gui-gtk3-compat.h considerably less
> complicated.
I'd like to look into the GTK3 simplification separately from this
series; I expect a few nuances here and there, so I wanted to sync the
GTK2 removal to the master branch at first.

Laszlo