[Spice-devel] [PATCH spice-gtk v5 6/6] usb-device-widget: Migrate to GtkGrid from GtkBox

[Pavel Grunt]

Hi Daniel,

On Mon, 2017-02-20 at 13:04 +0000, Daniel P. Berrange wrote:
> On Mon, Feb 20, 2017 at 12:44:52PM +0100, Victor Toso wrote:
> > Hi,
> > 
> > On Fri, Feb 17, 2017 at 11:24:55AM +0100, Pavel Grunt wrote:
> > > GtkVBox is deprecated since Gtk 3.2, GtkBox is going to be
> > > deprecated. Just use the GtkGrid and GtkContainer api.
> > 
> > Sure
> > 
> > > ---
> > > GtkContainer is an abstract class, so let's use the Grid with
> > > the
> > > vertical orientation
> > 
> > At first I thought this was a ABI break but it was questioned in
> > the
> > public IRC and the SpiceUsbDeviceWidget should not be subclassed,
> > so
> > this should be fine. The chat is below for reference.
> 
> Saying on IRC that it shouldn't be subclassed doesn't mean that it
> hasn't
> been sub-classed, particularly if this rule about not-subclassing
> has not
> been documented anywhere for app developers to learn about it...
> 
> > pgrunt | i have this patch https://paste.fedoraproject.org/.. is
> > it abi
> >          break ? SpiceUsbDeviceWidget is publically a GtkWidget
> >          https://www.spice-space.org/api/spice-gtk/spice-gtk-Spice
> > UsbDeviceWidget.html,
> >          but internally I am changing it from GtkBox to
> > GtkContainer, so
> >          binary is different api is same
> > teuf   | yeah, depends if we consider it a valid/likely use case
> > to
> >          derive from SpiceUsbDeviceWidget or not
> > teuf   | (ie whether G_DECLARE_FINAL_TYPE could be used in the
> > header or
> >          not)
> > pgrunt | hm, it should be final
> > teuf   | note that it's glib 2.44+ only
> > pgrunt | well i can keep the type GtkBox and use the gtk_container
> > api
> >          anyway
> 
> Even if you keep the type GtkBox in the struct, but change the
> G_DEFINE_TYPE to use GTK_CONTAINER, 

by "i can keep the type GtkBox" i meant to keep GTK_TYPE_BOX

> instad of GTK_TYPE_BOX, that is
> still technically an API break as you've changed the semantics of
> the widget. ie if a subclass exists,

SpiceUsbDeviceWidget is considered a final class
https://cgit.freedesktop.org/spice/spice-gtk/commit/?id=bc3d12efb20423
d5b1ebd490658f687c4bd323fd
It is not intended to be subclassed.

>  and they are calling any of
> the gtk_box_* APIs those will break, 

It is Publicly declared as a GtkWidget, not GtkBox/Container/Grid

Pavel

> even though the struct still
> contains the GtkBox content. ie while ABI refers to compiled type
> sizes, API covers semantics of the types too, and you need to
> preserve both ABI + API if you are not changing the soname.
> 
> Regards,
> Daniel