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 756352 - g_arg_info_get_closure broken for non callback args
g_arg_info_get_closure broken for non callback args
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
2.43.x
Other Linux
: Normal major
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-10-10 16:51 UTC by Christoph Reiter (lazka)
Modified: 2015-10-10 20:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-maintransformer-Fix-regression-in-callback-closure-a.patch (1.05 KB, patch)
2015-10-10 19:49 UTC, Colin Walters
none Details | Review
0001-maintransformer-Fix-regression-in-callback-closure-a.patch (7.86 KB, patch)
2015-10-10 20:31 UTC, Colin Walters
accepted-commit_now Details | Review

Description Christoph Reiter (lazka) 2015-10-10 16:51:37 UTC
https://git.gnome.org/browse/gobject-introspection/commit/?id=0a134a608f5b471c3a12739785e149ceaf90df27 broke PyGObject, see bug 756109.

It makes g_arg_info_get_closure() return the closure index of itself for user_data args but it should return -1 as it isn't a callback.
Comment 1 Colin Walters 2015-10-10 19:49:16 UTC
Created attachment 313026 [details] [review]
0001-maintransformer-Fix-regression-in-callback-closure-a.patch
Comment 2 Giovanni Campagna 2015-10-10 19:53:34 UTC
Mh wait,
the problem is that g_arg_info_get_closure("user_data") returns "user_data".
This has always been the case for arguments marked with (closure).

With your change g_arg_info_get_closure("user_data") returns "callback", which seems more broken to me.

I need to check what gjs does...
Comment 3 Jasper St. Pierre (not reading bugmail) 2015-10-10 19:54:45 UTC
Review of attachment 313026 [details] [review]:

eh, I don't think it should be called "closure_name" then.
Comment 4 Christoph Reiter (lazka) 2015-10-10 19:56:12 UTC
(PyGObject tests pass again with the above patch)
Comment 5 Giovanni Campagna 2015-10-10 19:59:05 UTC
Oh ok, I found the problem:

before the patch, the g_arg_info_get_closure(i) == i case was only for callbacks, not for functions.

Comment in the g-i code:

                # For callbacks, (closure) appears without an
                # argument, and tags a parameter that is a closure. We
                # represent it (weirdly) in the gir and typelib by
                # setting param.closure_name to itself.

I think the right fix (ie, to restore previous behavior) is to remove the line setting 
param.closure_name = param.argname
Comment 6 Colin Walters 2015-10-10 20:31:44 UTC
Created attachment 313027 [details] [review]
0001-maintransformer-Fix-regression-in-callback-closure-a.patch
Comment 7 Colin Walters 2015-10-10 20:32:41 UTC
Yes, but just deleting that would break the nullable logic.  This patch should fix both hopefully.
Comment 8 Giovanni Campagna 2015-10-10 20:40:33 UTC
Review of attachment 313027 [details] [review]:

It took me a moment for me to be convinced, but this patch is right.

Please discard the pygobject patch, which is wrong now.