GNOME Bugzilla – Bug 683099
Support GParamSpec signal arguments
Last modified: 2013-01-14 11:23:22 UTC
Created attachment 223044 [details] [review] Support marshalling GParamSpec signal arguments See also https://bugzilla.gnome.org/show_bug.cgi?id=681565 Looks like there are some other codepaths where a GParamSpec needs to be wrapped to a python type and where it is currently treated like a GObject, notably when a GParamSpec occurs as a signal arg, such as in gstreamer's GstObject::deep-notify signal
Created attachment 223269 [details] [review] attempt for a test case This is an attempt to provide a test case for this. However, it fails a lot earlier, in pyg_value_from_pyobject() it is trying to do g_value_set_param(value, PYGLIB_CPointer_GetPointer(obj, NULL)); but PYGLIB_CPointer_GetPointer(obj, NULL) == NULL in this case, so the generated GValue is invalid. So the marshalling from Python to C for GParamSpec signal arguments is broken. However, your patch fixes the other direction (marshalling from C to Python), so I guess we have to reduce the test case for this to just this direction, and keep this as a full test case as part of bug 682355?
Created attachment 223272 [details] [review] testcase for C -> Python direction Even with this reduced test case, which only checks C -> Python marshalling this patch does not help at all, as this now crashes with testTestReturnParamSpec (test_signal.TestCMarshaller) ... ** (process:6095): WARNING **: Unsupported fundamental type: GParam make[2]: *** [check-local] Trace/Breakpoint ausgelöst (Speicherauszug erstellt) in gi_cclosure_marshal_generic() from g-i. Do you have a reproducer script etc. which can be used to test that this patch actually does something useful? Thanks!
Created attachment 223289 [details] Script to reproduce C -> python unsupported marshalling Supplied patch indeed intends to fix a C to Python marshalling bug, but one where the signal is triggered in C and the ParamSpec is an argument to be marshalled to a Python callback. This is (apparently) another code path than the test case, where the invoked callback is a C one, whose return value has to be marshalled to C. In summary, this python script will fail without the supplied patch, but works fine with the patch.
FWIW/FTR, the problem encountered by the (restricted) test case seems resolved by the patch to the newly reported gobject-introspection bug #683265.
Comment on attachment 223272 [details] [review] testcase for C -> Python direction Indeed, with the patch from bug 683265 applied the "testcase for C -> Python direction" works just fine. So this code path is indeed independent from your patch. I committed the test case part from this now, as the g-i fix just landed in trunk.
Created attachment 223327 [details] [review] testcase for Python -> C direction This is the other half of the test case which checks the "python signal argument -> C" direction. As stated above, it fails with testTestReturnParamSpec (test_signal.TestCMarshaller) ... (runtests.py:13547): GLib-GObject-CRITICAL **: g_value_unset: assertion `G_IS_VALUE (value)' failed in pyg_value_from_pyobject(), when it is trying to do g_value_set_param(value, PYGLIB_CPointer_GetPointer(obj, NULL)); PYGLIB_CPointer_GetPointer(obj, NULL) == NULL in this case, so the generated GValue is invalid. So the marshalling from Python to C for GParamSpec signal arguments is broken. This is orthogonal to your patch, but I keep the test case here anyway because this code path needs fixing as well.
Confirming that the attached script reproduces the problem and the patch works. Thanks! I'll spend some time to write a proper test case for this.
Comment on attachment 223044 [details] [review] Support marshalling GParamSpec signal arguments Committed your patch with a test case. Thanks! http://git.gnome.org/browse/pygobject/commit/?id=0d099bdb3f4bbd962e5e60b583673d9e6f5673cc
Comment on attachment 223289 [details] Script to reproduce C -> python unsupported marshalling This reproducer script is obsolete, the same approach (which does not need GStreamer) went into the test suite.
Keeping bug open for the Python -> C direction.
Created attachment 233432 [details] [review] testcase for Python -> C direction Update Python -> C direction test case to current git head.
Fixed the Python -> C direction as well: http://git.gnome.org/browse/pygobject/commit/?id=f9429192cb1002725a11a75a7b8f9300375b9caf