GNOME Bugzilla – Bug 667244
Arrays of variants passed from C to Python always marshal to empty arrays
Last modified: 2012-07-12 09:20:31 UTC
If you have a GInterface method defined in C that takes an array of variants as an argument, and you implement that interface in Python, then the Python implementation always gets passed an empty array no matter what the caller passes in. Attaching tests case in a sec. Note: This bug is not a dupe of https://bugzilla.gnome.org/show_bug.cgi?id=638915. But is dual to it, in the sense that the error appears when passing arrays of variants in the *opposite* direction of that bug. That is, from C to Python.
Created attachment 204550 [details] [review] Patch: add relevant ginterface to gobject-introspection
Created attachment 204551 [details] [review] Patch: (failing) test case for pygobject Here's a test case for pygobject that fails. I'll look into a fix.
A little digging reveals that inside _pygi_closure_convert_ffi_arguments() that tag type GI_TYPE_TAG_ARRAY is not handled at all, and it goes into the default branch of the switch, which sets the argument to zero. Let's see if I can wrap my head around adding the support for arrays here...
Created attachment 204673 [details] [review] Fix C-, Ptr-, and Byte- Array handling for interfaces, properties, and signals This started out as a venture to fix the passing of arrays of variants into Python implementations of a GInterface. As it turned out there were lots of corner cases where arrays in general wasn't handled properly. The upshot is that _pygi_argument_to_object() now has the documented expectation of getting arrays packed in GArrays. This was implicit before and not correctly done on most call sites. The helper _pygi_argument_to_array() has been improved to work on any kind of array. And I've made sure all call sites of _pygi_argument_to_object() does the array conversion appropriately before calling _pygi_argument_to_object().
Created attachment 204674 [details] [review] Test passing arrays of variants to ginterface method with py impl Adds a test case that implements a ginterface, in Python, with a method that takes an array of variants as input. The test only passes with the improved array support from the previous commit.
All patches should be ready for review.
Unfortunately, the patch doesn't apply cleanly anymore, could you please rebase and provide a new patch? In addition, as your patch is quite extensive you should add more tests for all the array types you want to handle. We want to make sure tests cover all code paths.
Comment on attachment 204673 [details] [review] Fix C-, Ptr-, and Byte- Array handling for interfaces, properties, and signals Patch needs to be forward-ported.
Comment on attachment 204674 [details] [review] Test passing arrays of variants to ginterface method with py impl Patch needs more test cases, see Sebastian's comment 7.
Comment on attachment 204550 [details] [review] Patch: add relevant ginterface to gobject-introspection The g-i patch also does not apply any more, setting to "needs-work", too. >--- a/tests/gimarshallingtests.h >+++ b/tests/gimarshallingtests.h > [...] >-#define GI_MARSHALLING_TESTS_INTERFACE2_GET_IFACE(obj) (G_TYPE_INSTANCE_GET_INTERFACE2 ((obj), GI_MARSHALLING_TESTS_TYPE_INTERFACE2, GIMarshallingTestsInterface2Iface)) >+#define GI_MARSHALLING_TESTS_INTERFACE2_GET_IFACE(obj) (G_TYPE_INSTANCE_GET_INTERFACE ((obj), GI_MARSHALLING_TESTS_TYPE_INTERFACE2, GIMarshallingTestsInterface2Iface)) This looks odd and wrong to me. Possibly just an "oops" typo when you edited the source?
(In reply to comment #10) > (From update of attachment 204550 [details] [review]) > The g-i patch also does not apply any more, setting to "needs-work", too. > > >--- a/tests/gimarshallingtests.h > >+++ b/tests/gimarshallingtests.h > > [...] > >-#define GI_MARSHALLING_TESTS_INTERFACE2_GET_IFACE(obj) (G_TYPE_INSTANCE_GET_INTERFACE2 ((obj), GI_MARSHALLING_TESTS_TYPE_INTERFACE2, GIMarshallingTestsInterface2Iface)) > >+#define GI_MARSHALLING_TESTS_INTERFACE2_GET_IFACE(obj) (G_TYPE_INSTANCE_GET_INTERFACE ((obj), GI_MARSHALLING_TESTS_TYPE_INTERFACE2, GIMarshallingTestsInterface2Iface)) > > This looks odd and wrong to me. Possibly just an "oops" typo when you edited > the source? Ah, no, this is correct. It demonstrates that no-one was using GIMarshallingTestsInterface2 before this. It referenced a non-existing macro G_TYPE_INSTANCE_GET_INTERFACE2. I guess the extra "2" was from a previous bad copy-pasta.
Comment on attachment 204550 [details] [review] Patch: add relevant ginterface to gobject-introspection I updated the g-i test patch to current master, and committed. Thanks!
Thanks for your work! I fixed the patch to apply to current git master, and pushed: http://git.gnome.org/browse/pygobject/commit/?id=a31fabdc1
Ah, this is awesome. Thanks a bunch Martin! :-D