Created attachment 176593 [details]
example

Most likely an issue with our GValue handling.  Enums are subclasses of int but some places in the code check for the actual int class instead of using isinstance.

After playing around with this for a bit I think this can't be fixed without side effects.  The problem is that GValue interfaces are ambiguous.  For treemodels a person very well may want to pass in a PyObject pointer (which is what is getting passed).  A fix would be to add an override which does the conversion with context.

Hello Guillaume,

this bug was reported against 2.28 with the static bindings. Is this still an issue with current PyGObject 3.2 with introspection bindings? Thanks!

Yes; try running
http://cgit.freedesktop.org/telepathy/telepathy-glib/tree/examples/client/python/text-handler.py

with this change:
-        TelepathyGLib.PROP_CHANNEL_TARGET_HANDLE_TYPE: int(TelepathyGLib.HandleType.CONTACT),
+        TelepathyGLib.PROP_CHANNEL_TARGET_HANDLE_TYPE: TelepathyGLib.HandleType.CONTACT,

I get:
** (process:10988): WARNING **: Cannot marshal type "HandleType" in variant

Thanks! That helps.

So the function in question expects

         <parameter name="filter" transfer-ownership="none">
            <doc xml:whitespace="preserve"> a %TP_HASH_TYPE_CHANNEL_CLASS</doc>
            <type name="GLib.HashTable" c:type="GHashTable*">
              <type name="utf8"/>
              <type name="GObject.Value"/>
            </type>
          </parameter>


I. e. an utf8 -> GValue hashtable.

Created attachment 212685 [details] [review]
[g-i] tests: Add flags and enum to GValue GHash in/out

This patch adds enum and flags to regress_test_ghash_gvalue_{return,in}() in gobject-introspection, which now has the same signature as add_handler_filter() in the example.

Created attachment 212686 [details] [review]
Test flags and enums in GHash values

I had expected that this test case reproduces the problem here, but it actually works fine:

test_hash_in (test_everything.TestEverything) ... ok
test_hash_return (test_everything.TestEverything) ... ok

i. e. both flags and enums as GHash values are marshalled correctly in both directions.

Even more, if I try to modify that to say

                'enum': int(Everything.TestEnum.VALUE2),

in the definition of "data" in test_hash_in(), which is what Guillaume did as a workaround, the test fails:

ERROR:/home/martin-scratch/gnome/share/gobject-introspection-1.0/tests/regress.c:1323:regress_test_ghash_gvalue_in: assertion failed: (G_VALUE_HOLDS_ENUM(value))

So, while this test proves that the handling is not generally broken, it does not reproduce Guillaume's bug yet.

The error message in the description makes me wonder where that actually comes from. I grepped for the error message 'Cannot marshal type' in glib, gobject-introspection, pygobject, and telepathy-glib, and I don't find it anywhere. The marshalling in pygobject happens using GValue, not GVariant.

Ah, the error message happens in dbus-glib, dbus/dbus-gobject.c,get_all_object_properties():

      variant_sig = _dbus_gvalue_to_signature (&value);
      if (variant_sig == NULL)
        {
          value_gtype = G_VALUE_TYPE (&value);
          g_warning ("Cannot marshal type \"%s\" in variant", g_type_name (value_gtype));
          g_value_unset (&value);
          continue;
        }

so may it be that _dbus_gtype_to_signature() (which is called from _dbus_gvalue_to_signature()) does not get along with enums?

I'll commit the test cases to g-i and pygobject to ensure that this stays working on the GI side. This should be forwarded to the freedesktop.org dbus/glib bug tracker.

(In reply to comment #10)
> so may it be that _dbus_gtype_to_signature() (which is called from
> _dbus_gvalue_to_signature()) does not get along with enums?

Correct: it doesn't like enums.

Mapping from D-Bus to a GEnum in dbus-glib would be a bad idea, because GEnums are usually range-checked, and remotely-triggered assertions are bad (I WONTFIX'd a feature request on that basis). Mapping in the other direction (from GEnum/GFlags to integer) might be OK though.

We're trying to replace all the GValues in telepathy-glib with GVariant, but it's a gradual process.