GNOME Bugzilla – Bug 703091
GObject Introspection (and Vala) support
Last modified: 2013-07-01 20:57:21 UTC
For Geary, we've developed a VAPI for GMime 2.6. After using them for a while, they've stabilized and work well for us. We're hoping they can be included and maintained in the Vala distribution. Is there any interest in this?
What about distributing vapi and deps by yourself? The reason why we maintain vapis is due to upstream not distributing them.
It has a somewhat limited user base (in terms of number of projects, not number of users of those projects). If upstream is not interested in distributing it with gmime, perhaps it would make sense to include it in the vala-extra-vapis repository (see https://wiki.gnome.org/Vala/ExternalBindings#Git_Repository)?
We've asked upstream to distribute them, but GMime's maintainer isn't interested: https://bugzilla.gnome.org/show_bug.cgi?id=695319#c6 He did say he'd maintain a patch that generated .gir files, but (a) I'm not guru enough with GIR to offer such a patch, and (b) if you look at our bindings, you'll see a lot of .metadata patching and custom code to get names and namespaces right, and I don't know if such changes are easy or doable in GIR: http://git.yorba.org/cgit.cgi/geary/tree/bindings/vapi/gmime-2.6 I considered Evan's extra VAPIs repo, but since GMime is a GNOME project, I thought it was best if we tried to get it into Vala first. We would prefer it if we were using distributed bindings rather than ones from the extras repo, because that means we'd have to copy the bindings into our project and risk running behind as the master in vala-extras-vapis gets updated.
(In reply to comment #3) > We've asked upstream to distribute them, but GMime's maintainer isn't > interested: https://bugzilla.gnome.org/show_bug.cgi?id=695319#c6 > > He did say he'd maintain a patch that generated .gir files, but (a) I'm not > guru enough with GIR to offer such a patch, and (b) if you look at our > bindings, you'll see a lot of .metadata patching and custom code to get names > and namespaces right, and I don't know if such changes are easy or doable in > GIR: http://git.yorba.org/cgit.cgi/geary/tree/bindings/vapi/gmime-2.6 I didn't realize your bindings were based on GIDL. GIDL is deprecated and I don't plan on accepting any new bindings based on it into Vala. AFAIK everything which you can do (and is useful) with GIDL metadata should be possible with GIR metadata as well. And there is generally a lot less metadata so it is a lot easier. I've thrown together a few quick patches against GMime which add GIR (and Vala bindings based on it), I'll attach them and move this bug over to gmime. > I considered Evan's extra VAPIs repo, but since GMime is a GNOME project, I > thought it was best if we tried to get it into Vala first. We would prefer it > if we were using distributed bindings rather than ones from the extras repo, > because that means we'd have to copy the bindings into our project and risk > running behind as the master in vala-extras-vapis gets updated. That's not really true. It's designed to be used as a git submodule, so you don't have to copy them into your project (unless you're using a different VCS, but I don't think that's the case here). If you're afraid of falling behind you can always add some stuff to your autogen.sh to update the submodule whenever you run autogen.sh.
Created attachment 247856 [details] [review] Add GObject Introspection support
Created attachment 247857 [details] [review] Add Vala bindings (based on GObject Introspection) This is all the Vala-specific stuff. Jeffrey, would you be willing to carry this in GMime? As you can see, there's not really much too it and it shouldn't be a significant maintenance burden (which I'm happy to help with--you can just assign bug reports to me or CC me on them, ping me on IRC, etc.), we just prefer to get bindings distributed upstream (see https://wiki.gnome.org/Vala/UpstreamGuide#Why_Distribute_Bindings_Upstream for some of the reasons). Hopefully, over time, G-I will improve and some of the stuff in the metadata file can be moved into annotations.
Created attachment 247858 [details] [review] introspection: add many missing annotations This should take care of the low-hanging fruit for annotations. Most of it is the stuff which g-ir-scanner warns about, though there are some remaining warnings which can't be fixed with annotations alone (need to register some types with GObject and add GDestroyNotify and user_data args for some callbacks).
Created attachment 247862 [details] [review] introspection: add many missing annotations Forgot a few things from the old metadata file.
Evan -- thanks for picking up the ball on this. I didn't think about git submodules, so that's good to keep in mind going forward. But landing gir in GMime would be the best of all possible worlds.
Created attachment 247868 [details] VAPI Jim, here is the resulting VAPI. Can you try compiling Geary with it to make sure everything works as expected? You might want to diff the generated C and check any changes carefully, too.
There's a couple of naming differences I worked around, but still hitting some that are more fundamental. I'll attach a text file of the compilation errors. They kind of break down to two issues: The name `content_encoding_from_string' does not exist in the context of `GMime' Is this gone in GMime master? I hope not, but I don't see anything close to that symbol in your VAPI. And then various errors related to GMime.Filter's outbuf and filter virtual method. The gist of the problems (I think) are that GMime's filters deal with character (i.e. byte) arrays and they're being translated into the VAPI as strings. I created a branch working through these errors with your VAPI integrated into the build. If you like: git clone git://yorba.org/geary/ It's on branch bug/6413-gmime-bindings.
Created attachment 247939 [details] Compile errors with Evan's new VAPI
Created attachment 248004 [details] [review] introspection: add many missing annotations (In reply to comment #11) > There's a couple of naming differences I worked around, but still hitting some > that are more fundamental. I'll attach a text file of the compilation errors. > > They kind of break down to two issues: > > The name `content_encoding_from_string' does not exist in the context of > `GMime' > > Is this gone in GMime master? I hope not, but I don't see anything close to > that symbol in your VAPI. GMime.ContentEncoding.from_string > And then various errors related to GMime.Filter's outbuf and filter virtual > method. The gist of the problems (I think) are that GMime's filters deal with > character (i.e. byte) arrays and they're being translated into the VAPI as > strings. There isn't really a lot I can do in annotations about GMime.Filter.outbuf + friends. G-I's support for fields is basically non-existant. I've added the annotations, but G-I doesn't currently parse them so until bug #561619 is resolved this will be broken in other languages, and require metadata to allow Vala to work. Are they character arrays or binary data? I've assumed binary data, but it would be nice if someone who actually knows the API could go through the VAPI. Even if you don't care about Vala, it will tell you what other languages will expect (since it's derived from the GIR, and the GIR is much less pleasant to read than the VAPI). (In reply to comment #12) > Created an attachment (id=247939) [details] > Compile errors with Evan's new VAPI I think I've fixed all of these, although I'm using uint8[] instead of char[] (see above question about binary data or character arrays).
(In reply to comment #13) > > Is this gone in GMime master? I hope not, but I don't see anything close to > > that symbol in your VAPI. > > GMime.ContentEncoding.from_string Ok, I fixed that in our branch. > Are they character arrays or binary data? Binary data, although most of the time it will be 7-bit. (Maybe that's why they're using chars?) > I think I've fixed all of these, although I'm using uint8[] instead of char[] > (see above question about binary data or character arrays). I think that's a better choice.
(In reply to comment #14) > (In reply to comment #13) > > Are they character arrays or binary data? > > Binary data, although most of the time it will be 7-bit. (Maybe that's why > they're using chars?) Maybe, but "most of the time" isn't enough to make the argument a char array in GObject Introspection. Unfortunately, in C char* doesn't really say all that much since people use it for UTF-8 and ASCII strings, binary data, a pointer to a single char, strings with encodings other than UTF-8 (like ISO-8859), etc. In Vala (and GObject Introspection) we're trying to standardize on guint8 arrays for binary data. If the input and output are guaranteed to be UTF-8, char[] (or possibly even string) could make sense, otherwise I think uint8[] is probably the best way to go.
I'll try to review these patches this weekend. From a quick look, the patches look good to me.
patches have been committed
Unfortunately, when I run ./autogen.sh, I now get the following errors: gmime/Makefile.am:179: HAVE_INTROSPECTION does not appear in AM_CONDITIONAL gmime/Makefile.am:202: ENABLE_VAPIGEN does not appear in AM_CONDITIONAL How do I fix those?
(In reply to comment #18) > Unfortunately, when I run ./autogen.sh, I now get the following errors: > > gmime/Makefile.am:179: HAVE_INTROSPECTION does not appear in AM_CONDITIONAL > gmime/Makefile.am:202: ENABLE_VAPIGEN does not appear in AM_CONDITIONAL > > How do I fix those? That shouldn't happen if you include the m4 macros... m4/introspection.m4 for the former, m4/vapigen.m4 for the latter.
Just tried with a fresh checkout and I can't reproduce that error. Perhaps a git clean -Xdf would help? Also, what version of automake are you using? I'm using 1.13 (with the patch in bug #703156). AFAIK those macros should be pretty tolerant of older versions, but if you're using something older I can install that to test with.
I've got automake-1.11 I'll try doing a fresh checkout - btw, git master has those disabled, so make sure you revert the most recent commit
This is the output from a fresh checkout: fejj@linux:/cvs/tmp/gmime$ ./autogen.sh libtoolize: putting auxiliary files in `.'. libtoolize: linking file `./ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'. libtoolize: linking file `m4/libtool.m4' libtoolize: linking file `m4/ltoptions.m4' libtoolize: linking file `m4/ltsugar.m4' libtoolize: linking file `m4/ltversion.m4' libtoolize: linking file `m4/lt~obsolete.m4' configure.ac:47: installing `./config.guess' configure.ac:47: installing `./config.sub' configure.ac:53: installing `./install-sh' configure.ac:53: installing `./missing' examples/Makefile.am: installing `./depcomp' gmime/Makefile.am:179: HAVE_INTROSPECTION does not appear in AM_CONDITIONAL gmime/Makefile.am:202: ENABLE_VAPIGEN does not appear in AM_CONDITIONAL
I don't have vapigen.m4 - what package does that come from? introspection.m4 should be getting picked up from the m4/ directory, shouldn't it?
Created attachment 248073 [details] [review] Fix build of introspection and vala bindings, teach aclocal about m4/ (In reply to comment #23) > I don't have vapigen.m4 - what package does that come from? introspection.m4 > should be getting picked up from the m4/ directory, shouldn't it? It is distributed with Vala but I should have copied it to m4/ so Vala doesn't have to be installed. The file is at https://git.gnome.org/browse/vala/tree/vapigen/vapigen.m4. Yes, introspection.m4 should be getting picked up from the m4/ directory... After a bit of work, I think I found the issue--autogen.sh isn't passing -I m4 to aclocal. This patch fixes both issues and re-enables introspection + vala.
thanks Evan! this seems to fix it I should have thought of the -I m4 fix, that seems so obvious now.
Thanks to both of you -- very glad to see this work completed.