This is the commit that caused the breakage in glib:
https://git.gnome.org/browse/glib/commit/gobject/gtype.c?id=c2055f22f4399a23d1c02a94f8b029212e37e162

Created attachment 240555 [details] [review]
0001-Custom-Interface-implementations-Add-interfaces-earl.patch

Here is an ABI-breaking and API-breaking patch that makes things work again:

    Custom Interface implementations: Add interfaces earlier.
    
    * glib/glibmm/class.[h|cc]: Add a clone_custom_type()
      overload that takes a list of pointers to add_interface()
      functions so we can call them on the GType just after
      registering it, but before instantiating the first GObject.
    * glib/glibmm/interface.cc:
      Interface::Interface(const Interface_Class& interface_class):
      Silently do nothing if the GObject has not been instantiated
      yet, because we do not need to add the interface here any more,
      but we should keep this code for compatiblity.
    * glib/glibmm/objectbase.h:
      Add a custom_type_list_add_interface_funcs_ list of pointers
      to add_interface() functions here, so it can be shared by
      both the Object and Interface classes.
    * glib/glibmm/object.cc: Default constructor: Pass the list of
      functions to the new clone_custom_type() method overload.
    * tools/m4/class_interface.m4: Default constructors:
      Add the add_interface() function to the list of functions.
    
    This breaks ABI by adding the list<> to ObjectBase.
    It also breaks API by, for instance, requiring a custom TreeModel
    to have Glib::Object as the last in its list of base classes,
    so that the default constructors for the interfaces can run first.

Created attachment 240556 [details] [review]
treemodelcustom_fix.patch

And here are the small changes that would be necessary in the custom treemodel example.

Maybe we could avoid the need to change the base class sequence if we instead do something like this:

ExampleTreeModel::ExampleTreeModel(unsigned int columns_count)
: Glib::ObjectBase( typeid(ExampleTreeModel) ), //register a custom GType.
  Glib::Object(&TreeModel::add_interface()) //The custom GType is actually registered here.

However,
1. That would get awkward when deriving-from/implementing multiple interfaces,
and
2. That's still an API break because it would need us to pass the extra parameter(s) to Glib::Object::Object().

Created attachment 240606 [details] [review]
gtype: interface-after-init exception for glibmm

glibmm has a pretty difficult-to-solve problem caused by our recent
change to deny addition of interfaces to classes after initialisation.

They're looking for a long-term workaround for the problem, but in the
meantime we can allow the registration to succeed (with warning) if the
class looks like it's being defined by gtkmm.

Ryan, I think you posted the patch in the wrong bug.  If referred to your post in the original glib post so that it can be considered.

That is, I've referred to the post with the attachment in this bug in the original glib bug.

Right, bug 687659. Ryan: can you please consider looking at the patch in it before committing the one you proposed here? Thanks

With that glib patch glibmm/gtkmm seems to work again. I see the warning from apps, but the UI now works.

I discussed with Andrés on IRC that we would add a separate exception for C# apps, so I'm going to go ahead with this patch here for now.

Comment on attachment 240606 [details] [review]
gtype: interface-after-init exception for glibmm

Attachment 240606 [details] pushed as c5307e4 - gtype: interface-after-init exception for glibmm

The temporary workaround in glib will fix things for us with glib 2.36, but this problem will come back to us in glib 2.37/38 when that workaround is removed. That gives us a couple of months to find a solution in glibmm.

I think we could avoid the ABI break in Glib::ObjectBase caused by the new std::list<> member, by storing something (a GList* maybe) in the GObject qdata, like we store the C++ instance there already. We could alternatively consider some global gtype-to-interfaces_to_add mapping. We already have a GType to C++ instance mapping for Glib::wrap().

That would still leave the need to change the order of base classes in application code, as in comment #3. I think that would affect almost every serious gtkmm application because it's fairly common to create custom CellRenderers or custom TreeModels.

So it would be good to find a way to avoid that. If we can't then we might need to do a new parallel installable version of glibmm and everything above it, which would annoy people almost as much.

Created attachment 241793 [details] [review]
Custom Interfaces: Implement derived interface properties.

This parallel patch can be used to implement the properties of the derived interfaces by overriding them in a custom base init function.  I think the same thing is possible now except that the place where the properties are overridden would be different and g_object_class_install_property() would have to be used insteaad of g_object_class_override_property() (which is how it is done in the patch).  If possible, I could do this in git now.

Created attachment 241794 [details] [review]
Changes to the glibmm interface implementation test to test the implementation of properties

These changes to the glibmm interface implementation test show what could be possible when implementing the interface properties.

Is this possible with older glibmm and glib versions? I mean, is this fixing a regression or just adding a new ability?

Created attachment 241857 [details]
Output of the modified test running in glib-2.34.1/glibmm-2.33.13

I don't think it's possible with older glib/glibmm versions.  If the interface test is modified according to the changes in the patch that modifies the test to test the interface properties and the construction order is fixed (so that the Glib::Object constructor goes first) and the patch to add interfaces earlier is not used, the attached output is produced when running the test using glib-2.34.1 and glibmm-2.33.13.

I think this would be adding something new and not fixing a regression.  I could be wrong because I only tested the versions of glib/glibmm found in ubuntu 12.10.

Created attachment 242138 [details] [review]
patch: Add interfaces to custom types before class_init.

This is an alternative to Murray's patch in comment 2.

Pros
----
- Does not break ABI. (Can be improved when ABI can be broken.)
- No modification of any m4 file (such as class_interface.m4), affecting many
  files generated by gmmproc.
- As long as glib accepts calls to g_type_add_interface_static() after
  class_init, i.e. after a GObject has been created, the Glib::Object derived
  class can be anywhere in the list of base classes: before, after or between
  interfaces.

Cons
----
- Adds a std::map with extra private data in objectbase.cc. (This is just to
  avoid breaking ABI.)
- When glib does not allow adding interfaces after class_init, it breaks API
  by requiring that the Glib::Object derived class is listed after the
  interfaces in the list of base classes.

Created attachment 242189 [details] [review]
Custom Interfaces: Implement derived interface properties in present.

This patch implements derived properties for custom interface types in the present.  If it's okay, I'll push this patch soon so that the derived properties of custom interface types works right.

Comment on attachment 242189 [details] [review]
Custom Interfaces: Implement derived interface properties in present.

Pushed to the master branch.  It can always be reverted or modified in git if it needs to be:

https://git.gnome.org/browse/glibmm/commit/?id=ce20003fd7ed3d6dd11433388598d6c1b1f41029
https://git.gnome.org/browse/glibmm/commit/?id=a88d2341144533ac698432b48f85b72642c0362e

I should say that I pushed it accidentally when adding the properties of GSettings to the Gio::Settings class:

https://git.gnome.org/browse/glibmm/commit/?id=0cc347ec414d92d41f28c4d3e6889cef6951e923

But now I think it's best to keep it since I was going to push it in a few days anyway.

Review of attachment 242138 [details] [review]:

OK. This avoid the ABI break due to the change of the ObjectBase instance size. The use of the shared std::map<> is funky, but I guess our shared map for Glib::wrap() is already doing much the same thing.

However, I guess that this still does break ABI because, if the application source code is not changed, and the app is not rebuilt, the app will still fail with a newer glib, regardless of the glibmm version used. It seems that the base classes will still have to be reordered and the app will need to be rebuilt.

::: glib/glibmm/class.h
@@ -54,2 +56,3 @@
   GType clone_custom_type(const char* custom_type_name) const;
 
+  typedef std::list<const Interface_Class*> type_list_interface_classes;

Could we have some docs for these methods, please, even if the old ones don't have any either.

::: glib/glibmm/objectbase.cc
@@ -37,0 +38,10 @@
+//TODO: At the next ABI break, replace ExtraObjectBaseData by a new data member in ObjectBase.
+// This is a new data member that can't be added to Glib::ObjectBase now,
+// because it would break ABI.
... 7 more ...

Shouldn't this be static?

@@ +48,3 @@
+// ObjectBase instances may be used in different threads.
+// Accesses to extra_object_base_data must be thread-safe.
+Glib::Threads::Mutex extra_object_base_data_mutex;

And shouldn't this be static too?

Created attachment 243913 [details] [review]
patch: Add interfaces to custom types before class_init.

An updated patch. José's added handling of interface properties made it more
complicated. Hope I got that part right. (At least as right as it is without my
patch.) I've tested with the modified test case in comment 14. 

Some questions on the interface properties:

1. Would it be a good idea to add
   g_param_value_set_default(iface_props[p], g_value) in
   Interface::Interface(const Interface_Class& interface_class) and
   Class::custom_class_init_function(void* g_class, void* class_data)?

2. Is Class::interface_finalize_function() really useful? It's not called in my
   tests, and if it were called, it wouldn't do anything, would it?
   It finalizes an interface, but the Class::properties_type vector belongs to
   custom_type.
   I tried to add it as a class_finalize function in derived_info in
   Class::clone_custom_type(), but g_type_register_static() did not accept
   that. A static type can't have a class finalizer.

3. The Class::properties_type vector that holds the values of the overridden
   properties of implemented interfaces is attached to the custom_type with
   g_type_set_qdata(). Those values are thus class data. They shall be instance
   data. Each instance of the custom class shall have its own set of property
   values.

4. Don't we need protected access methods to the overridden properties?
   Accesses through property_xxx() methods generated by _WRAP_PROPERTY are
   probably enough for read-write props, but what about read-only and write-
   only props? The class that implements the interface must be able to write
   a read-only prop, and read a write-only prop.


> However, I guess that this still does break ABI because, if the application
> source code is not changed, and the app is not rebuilt, the app will still
> fail with a newer glib, regardless of the glibmm version used. It seems that
> the base classes will still have to be reordered and the app will need to be
> rebuilt.

That's right. I don't know how to avoid that.
I called it an API break in comment 17. Anyway it will break applications with
custom implementation of interfaces, when glib no longer accepts
g_type_add_interface_static() after class_init.

Since the size of ObjectBase instances does not change, only applications with
custom implementation of interfaces will break.

If my patch is applied, applications with custom implementation of interfaces
can change the order of base classes right away, without coordinating it with
other changes of glib or glibmm. As long as glib accepts
g_type_add_interface_static() after class_init, the application's custom class
can have the Glib::Object derived base class either first or last in the list
of base classes.

Since the questions refer to what I had done, I'll reply to them:

(In reply to comment #21)
> Some questions on the interface properties:
> 
> 1. Would it be a good idea to add
>    g_param_value_set_default(iface_props[p], g_value) in
>    Interface::Interface(const Interface_Class& interface_class) and
>    Class::custom_class_init_function(void* g_class, void* class_data)?

I guess so.  I had quickly glanced at the function when perusing the docs but did not make a distinction between what the function does as opposed to what g_value_init() does.  g_value_init() initializes the GValue to to a default value, but probably not to the default value of the GValue according to the GParamSpec.  Thanks for pointing that out.

> 
> 2. Is Class::interface_finalize_function() really useful? It's not called in my
>    tests, and if it were called, it wouldn't do anything, would it?
>    It finalizes an interface, but the Class::properties_type vector belongs to
>    custom_type.
>    I tried to add it as a class_finalize function in derived_info in
>    Class::clone_custom_type(), but g_type_register_static() did not accept
>    that. A static type can't have a class finalizer.

It isn't really useful.  I too found that it was never called but I included it for completeness.  I too tried to remove the data in a custom class finalize function first (as the patch in comment 13 that attempts this for the future does) and found the same problem.  The GType docs explains why [1] so I reverted to trying to free the data in the interface finalize function (because there is no such mention of the interface finalize function never being called in the docs [2]).  When I noticed that it was never called (like the custom class finalize function would also not be called), I left it for completeness.

[1] https://developer.gnome.org/gobject/stable/gobject-Type-Information.html#GClassFinalizeFunc
[2] https://developer.gnome.org/gobject/stable/gobject-Type-Information.html#GInterfaceFinalizeFunc

> 
> 3. The Class::properties_type vector that holds the values of the overridden
>    properties of implemented interfaces is attached to the custom_type with
>    g_type_set_qdata(). Those values are thus class data. They shall be instance
>    data. Each instance of the custom class shall have its own set of property
>    values.

According to the g_type_add_interface_static() function docs [3], I think that the interface finalize function specified in the GInterfaceInfo structure is specific to the instance_type/interface_type combination and would apply to the custom type instances (see the info parameter description and the interface initializing and finalizing discussions here [4]).  Am I wrong?  Truly, it would be redundant if multiple interfaces are inherited and using a custom class finalize function might be better in this respect.  Since I read the docs above about the custom class finalize function not being called for static types and didn't notice such documentation for the interface finalize functions I used that instead.  I guess it can just be removed.

[3] https://developer.gnome.org/gobject/stable/gobject-Type-Information.html#g-type-add-interface-static
[4] https://developer.gnome.org/gobject/stable/gtype-non-instantiable-classed.html

I do notice a problem now with the functionality, however.  Ideally, the properties should be stored globally for the class and not for each instance.  It's something that should be corrected if possible.

> 
> 4. Don't we need protected access methods to the overridden properties?
>    Accesses through property_xxx() methods generated by _WRAP_PROPERTY are
>    probably enough for read-write props, but what about read-only and write-
>    only props? The class that implements the interface must be able to write
>    a read-only prop, and read a write-only prop.

Are the read-only and write-only properties not wrappable with _WRAP_PROPERTY()?  If not and normally there would be no access to these properties in the base class of the custom type, why should the custom type have access to them?  I may not be understanding clearly.

When I say 'I had done', I mean 'I have done'.  Sorry.

(In reply to comment #22)
> > 3. The Class::properties_type vector that holds the values of the overridden
> >    properties of implemented interfaces is attached to the custom_type with
> >    g_type_set_qdata(). Those values are thus class data. They shall be instance
> >    data. Each instance of the custom class shall have its own set of property
> >    values.

I'm not sure this is right.  The signature for the g_type_set/get_qdata() are as follows:

void g_type_set_qdata(GType type, GQuark quark, gpointer data);
gpointer g_type_get_qdata(GType type, GQuark quark);

It seems to me that the data is attached to a GType and not an instance.  I could be wrong.

> I do notice a problem now with the functionality, however.  Ideally, the
> properties should be stored globally for the class and not for each instance. 
> It's something that should be corrected if possible.

If the above corrective statement made by me is correct then the above problem does not really exist.  There would be no problem with properties being stored in instances and not for the type.

> 3.
You have misunderstood my comment. I say, just as you do, that the present
source code makes the overridden interface properties class data. But I say
that they shall be instance data. All properties that I've seen are instance
data. In glib and gtk+ classes, the values of properties (overridden or
directly installed) are usually stored among other private data in a private
instance data struct in the .c file. The description of Glib::Property<> says
"A property is a value associated with each instance of a type and some class
data for each property: Its unique name, ..."

> 4.
Read-only and write-only properties can be wrapped with _WRAP_PROPERTY().

For a read-only property, _WRAP_PROPERTY generates a property_xxx() method
returning Glib::PropertyProxy_ReadOnly<>, which can be used for reading, but
not for writing, the property value.

For a write-only property, _WRAP_PROPERTY generates a property_xxx() method
returning Glib::PropertyProxy_WriteOnly<>, which can be used for writing, but
not for reading, the property value.

These are public methods that determine what other objects can do with the
properties. But the object that owns the data should be able to both read and
write all kinds of properties.

Review of attachment 243913 [details] [review]:

OK. Please go ahead and push this to master.

At least this lets us call the current order of base classes deprecated before it will be broken by glib next time.

Created attachment 250589 [details]
Output from modified test case glibmm_interface_implementation.

I have pushed the patch in comment 21. (The interface.cc part of the pushed
patch is not identical to the patch in comment 21, because José made some
changes to interface.cc after I added comment 21.)
https://git.gnome.org/browse/glibmm/commit/?id=fc35fb6a7d16631b1d120631c4fc982f14e09a96


The attached file shows that we also have a problem with custom properties.
When I modify glibmm's test case tests/glibmm_interface_implementation with
José's patch in comment 14, I get the warning

(process:15831): GLib-GObject-WARNING **: Attempt to add property
gtkmm__CustomObject_12CustomAction::custom-property after class was initialised

Some checks have recently been added to g_object_class_install_property():
https://git.gnome.org/browse/glib/commit/?id=a8a9afe17c0ee484b65c6f75e0d22ad1ae2cd9b6
https://git.gnome.org/browse/glib/commit/?id=c1e32a5c59cf43d86c6e1b50ba570e01a549c026

This is described in the glib bug 698614.

Created attachment 278057 [details] [review]
gtype: remove interface-after-init exceptions

A year ago, we tried to remove support for adding interfaces on
already-initialised types.  There were problems with the C++ and C#
bindings at the time, so we added exceptions to give them a bit more
time to catch up.

It's already one cycle after when these exceptions were planned to be
removed, so let's take them out now.

The present situation concerning interfaces in glibmm classes:

There has never been a problem with adding interfaces in classes that only
wrap a glib or gtk+ class with interfaces. The interfaces are added by code in
glib or gtk+.

Before glibmm 2.37.5 a custom class with interfaces had to add the interfaces
after class_init. Example:
  class CustomConverter : public Glib::Object, public Gio::Converter

From glibmm 2.37.5, after the patch in comment 21 was pushed, a custom class
with interfaces can add the interfaces either after class_init,
  class CustomConverter : public Glib::Object, public Gio::Converter

or before class_init, by making the Glib::Object the last base class instead of
the first one,
  class CustomConverter : public Gio::Converter, public Glib::Object

When glib drops support for adding interfaces after class_init, only the
second alternative will work.

The necessary change in user code is described in glibmm's NEWS file.

So, can glib now drop the support for adding interfaces after class_init?
I suppose so, but Murray's opinion is more important than mine.
I also suppose this bug can be closed now.


BTW, I see that I did not mark the patch as committed, when I committed it.
I've done that now. Better late than never.

Attachment 278057 [details] pushed as 545b444 - gtype: remove interface-after-init exceptions

I've pushed a patch which is almost identical to José's attachment in
comment 14. The glibmm_interface_implementation test had to be changed to fix
'make check' now when glib does not allow adding interfaces after class_init.
https://git.gnome.org/browse/glibmm/commit/?id=99d79453da0630cf2eb205658bdeef0131b1026f