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 634202 - enum/quark pairing isn't working as intended
enum/quark pairing isn't working as intended
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
: 637025 (view as bug list)
Depends on:
Blocks: 625942
 
 
Reported: 2010-11-07 00:57 UTC by Jonathan Matthew
Modified: 2015-02-07 17:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
remove namespace symbol prefix before looking up enum type (1.69 KB, patch)
2010-11-07 01:31 UTC, Jonathan Matthew
none Details | Review
Maintransfomer: fix again paring error domains with unregistered enums (3.04 KB, patch)
2012-07-02 20:54 UTC, Giovanni Campagna
none Details | Review
Maintransfomer: fix again paring error domains with unregistered enums (4.44 KB, patch)
2012-07-02 21:17 UTC, Giovanni Campagna
committed Details | Review

Description Jonathan Matthew 2010-11-07 00:57:36 UTC
_pair_quarks_with_enums() tries to use self._uscore_type_names to find enum types for quarks, but it's currently doing it wrong:

            short = node.symbol[:-len('_quark')]

this is the full c symbol name, but things in self._uscore_type_names have the namespace prefix removed.
Comment 1 Jonathan Matthew 2010-11-07 01:31:57 UTC
Created attachment 173977 [details] [review]
remove namespace symbol prefix before looking up enum type
Comment 2 Colin Walters 2011-01-11 18:44:30 UTC
*** Bug 637025 has been marked as a duplicate of this bug. ***
Comment 3 Colin Walters 2011-01-11 18:45:51 UTC
Same comments on this as from 637025; nervous about the lack of tests.  Investigating...
Comment 4 Giovanni Campagna 2011-03-31 16:21:25 UTC
Did you make some progress on this?
The bug is clear, and his patch seems correct.
Comment 5 Colin Walters 2011-03-31 17:24:06 UTC
It sounds reasonable but I'm wary of committing things without tests.  Is this critical for anything in GNOME 3.0 or can it wait until 3.0.x or 3.2?
Comment 6 Giovanni Campagna 2011-03-31 17:29:27 UTC
Given that gjs has no support for GError codes, nothing using gjs depends on this. Don't know for PyGObject or Seed though.
Mine was just a ping to get it off my "opened bug list".
Comment 7 Jonathan Matthew 2011-03-31 21:21:38 UTC
I don't think I ever needed this for anything.  I was just trying to get g-ir-scanner to be reasonably quiet so I could see what my actual problems were.  It can probably wait until I get around to adding some tests for it.
Comment 8 Colin Walters 2012-07-02 19:41:24 UTC
Let's pick this back up again.  It's been a while.  What we really need is concrete examples of affected codebases, and ideally translate those to a test we can add to tests/scanner/regress.h or the like.
Comment 9 Colin Walters 2012-07-02 19:43:26 UTC
Giovanni says that the current code failed for GDBusError in http://bugzilla-attachments.gnome.org/attachment.cgi?id=216567.  However, if I revert Jasper's revert, regenerate Gio-2.0.gir, and run diff -u, I don't see any difference in the generated gir.
Comment 10 Giovanni Campagna 2012-07-02 20:10:47 UTC
That's not the patch Jasper reverted, he reverted the next one (64f38328, reverted by e4879c84), which affects non GType registered enums.
If you test GIO, which has all enums registered, you see no difference exactly because of 981f0111c39.
If you apply 64f38328 and then revert 981f0111, you should see a warning for each Gio error domain. If you revert 981f0111 while keeping e4879c84, you should see a warning for GDBusError only.
Comment 11 Colin Walters 2012-07-02 20:39:20 UTC
(In reply to comment #10)
> That's not the patch Jasper reverted, he reverted the next one (64f38328,
> reverted by e4879c84), which affects non GType registered enums.
> If you test GIO, which has all enums registered, you see no difference exactly
> because of 981f0111c39.
> If you apply 64f38328 and then revert 981f0111, you should see a warning for
> each Gio error domain. If you revert 981f0111 while keeping e4879c84, you
> should see a warning for GDBusError only.

Thanks, that's useful.  So one important factor is being registered as a GType.

I've added a regression test:
http://git.gnome.org/browse/gobject-introspection/commit/?id=3077744451293c0bd399866b2accf10fa6a30877

I can confirm that when I revert Jasper's revert, the test fails.
Comment 12 Giovanni Campagna 2012-07-02 20:54:09 UTC
Created attachment 217871 [details] [review]
Maintransfomer: fix again paring error domains with unregistered enums

Previous fix was wrong, as it called to_underscores_noprefix on a
prefixed type name. The actual fix is to call the transformer to
do the prefix / type_name split, and turn the latter to underscores.
Test case included.

Ok, this one includes another test case that fails, and a fix for it.
Comment 13 Jasper St. Pierre (not reading bugmail) 2012-07-02 20:56:20 UTC
Really, we should stop sticking in prefix stripping code at random, and just strip prefixes from every name we get back from the parser / gtype before it goes anywhere else.
Comment 14 Colin Walters 2012-07-02 21:09:30 UTC
Comment on attachment 173977 [details] [review]
remove namespace symbol prefix before looking up enum type

Earlier patch here is now obsolete
Comment 15 Giovanni Campagna 2012-07-02 21:17:49 UTC
Created attachment 217873 [details] [review]
Maintransfomer: fix again paring error domains with unregistered enums

Previous fix was wrong, as it called to_underscores_noprefix on a
prefixed type name. The actual fix is to call the transformer to
do the prefix / type_name split, and turn the latter to underscores.
Test case included.

Prefix / name splitting the way GGod intended it.
Comment 16 Colin Walters 2012-07-02 21:23:06 UTC
Review of attachment 217873 [details] [review]:

Ok, the test case greatly increases my confidence in this.
Comment 17 Giovanni Campagna 2012-07-02 21:32:29 UTC
Attachment 217873 [details] pushed as b87a149 - Maintransfomer: fix again paring error domains with unregistered enums
Comment 18 André Klapper 2015-02-07 17:03:57 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]