After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 685197 - Need to support (in) GError arguments
Need to support (in) GError arguments
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Simon Feltman
Python bindings maintainers
: 755916 757323 (view as bug list)
Depends on:
Blocks: 761592
 
 
Reported: 2012-10-01 10:41 UTC by Guillaume Desmottes
Modified: 2016-03-02 05:14 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
gerror: Add "_to_py" suffix to pygi_error_marshal (2.13 KB, patch)
2016-02-29 06:53 UTC, Simon Feltman 		committed Details | Review
gerror: Add support for marshaling GError from Python to C (8.84 KB, patch)
2016-02-29 06:53 UTC, Simon Feltman 		committed Details | Review

Description Guillaume Desmottes 2012-10-01 10:41:46 UTC 
Method taking a (const GError *) as argument can't currently be used in Pythonn as python-gobject see those as exceptions.

void                tp_handle_channels_context_fail     (TpHandleChannelsContext *self,
                                                         const GError *error);

http://telepathy.freedesktop.org/doc/telepathy-glib/telepathy-glib-handle-channels-context.html#tp-handle-channels-context-fail
Comment 1 Martin Pitt 2012-10-01 10:46:34 UTC 
Right now, gobject-introspection skips GError arguments completely in the "standard" case of passing a GError* which points to a NULL GError*. So in pygobject we could tell apart these two by looking at the type.

If you build the annotations for your project with that method, what does the .gir for this method look like?
Comment 2 Guillaume Desmottes 2012-10-01 10:48:43 UTC 
      <method name="fail"
              c:identifier="tp_handle_channels_context_fail"
              version="0.11.6">
        <doc xml:whitespace="preserve">Called by #TpBaseClientClassAddDispatchOperationImpl to raise a D-Bus error.</doc>
        <return-value transfer-ownership="none">
          <type name="none" c:type="void"/>
        </return-value>
        <parameters>
          <parameter name="error" transfer-ownership="none">
            <doc xml:whitespace="preserve">the error to return from the method</doc>
            <type name="GLib.Error" c:type="const GError*"/>
          </parameter>
        </parameters>
      </method>
Comment 3 Martin Pitt 2012-10-01 10:50:18 UTC 
That looks fine. So I guess we should be able to handle that on the pygobject side in principle.
Comment 4 Simon Feltman 2013-08-14 20:30:15 UTC 
Relevant places to fix this:
https://git.gnome.org/browse/pygobject/tree/gi/pygi-marshal-from-py.c?id=3.9.5#n1177
 and:
https://git.gnome.org/browse/pygobject/tree/gi/pygi-argument.c?id=3.9.5#n1243

These should also share a unified marshaling function placed in pygi-marshal-from-py.c
Comment 5 Simon Feltman 2013-08-14 20:35:09 UTC 
I experienced a bit of deja vu while writing that last comment. Sure enough I wrote almost the exact same thing in bug #616036

*** This bug has been marked as a duplicate of bug 616036 ***
Comment 6 Simon Feltman 2016-01-23 04:05:18 UTC 
Re-opening as this as bug 616036 is not exactly the same use case. bug 616036 is referring to callbacks.
Comment 7 Simon Feltman 2016-01-23 04:06:14 UTC 
*** Bug 755916 has been marked as a duplicate of this bug. ***
Comment 8 Simon Feltman 2016-01-23 04:07:14 UTC 
*** Bug 757323 has been marked as a duplicate of this bug. ***
Comment 9 Simon Feltman 2016-02-29 06:53:00 UTC 
Created attachment 322625 [details] [review]
gerror: Add "_to_py" suffix to pygi_error_marshal
Comment 10 Simon Feltman 2016-02-29 06:53:49 UTC 
Created attachment 322626 [details] [review]
gerror: Add support for marshaling GError from Python to C

I'd love to get this in for 3.20, so reviews welcome.
Comment 11 Simon Feltman 2016-02-29 07:14:52 UTC 
I've verified this also fixes the use cases in bug 755916 and bug 757323.
Comment 12 Christoph Reiter (lazka) 2016-03-01 11:01:21 UTC 
Review of attachment 322625 [details] [review]:

lgtm
Comment 13 Christoph Reiter (lazka) 2016-03-01 11:01:25 UTC 
Review of attachment 322625 [details] [review]:

lgtm
Comment 14 Christoph Reiter (lazka) 2016-03-01 11:02:06 UTC 
Review of attachment 322626 [details] [review]:

lgtm