GNOME Bugzilla – Bug 678440
Add Vala bindings
Last modified: 2019-02-22 11:45:51 UTC
Title says it all.
Created attachment 216779 [details] [review] gnome-keyring-1: switch to GIR
Created attachment 217013 [details] [review] Add Vala bindings
Review of attachment 217013 [details] [review]: Looking over this. make distcheck fails with: CCLD libgnome-keyring.la CCLD libgnome-keyring-testable.la GISCAN GnomeKeyring-1.0.gir GICOMP GnomeKeyring-1.0.gir VAPIGEN gnome-keyring-1.vapi error: GnomeKeyring-1.0-custom.vala not found make[4]: *** [gnome-keyring-1.vapi] Error 1 make[4]: Leaving directory `/data/projects/libgnome-keyring/libgnome-keyring-3.5.3/_build/library' make[3]: *** [all-recursive] Error 1 make[3]: Leaving directory `/data/projects/libgnome-keyring/libgnome-keyring-3.5.3/_build/library' make[2]: *** [all-recursive] Error 1 make[2]: Leaving directory `/data/projects/libgnome-keyring/libgnome-keyring-3.5.3/_build' make[1]: *** [all] Error 2 make[1]: Leaving directory `/data/projects/libgnome-keyring/libgnome-keyring-3.5.3/_build' make: *** [distcheck] Error 1 ::: library/GnomeKeyring-1.0.metadata @@ +1,1 @@ +Attribute struct=false Let's add a comment here about what this file is for.
Note this required http://git.gnome.org/browse/libsecret/commit/?id=541ba2095a9f1b44861d247622348f51034ca17a
Created attachment 217556 [details] [review] Add Vala bindings. (In reply to comment #3) > make distcheck fails with: Fixed. Tested both with and without vala installed, both in VPATH regular builds. > Let's add a comment here about what this file is for. Done (copied from libsecret) (In reply to comment #4) > Note this required > http://git.gnome.org/browse/libsecret/commit/?id=541ba2095a9f1b44861d247622348f51034ca17a Did you mean to post that to bug #678846? It seems like gnome-autogen.sh is smart enough to pick up the m4 directory--like I said, I tested this both with and without vala installed.
(In reply to comment #5) > > Fixed. Tested both with and without vala installed, both in VPATH regular > builds. I don't know how you're doing your VPATH builds, but it seems wrong: http://git.gnome.org/browse/libsecret/commit/?id=e72c4c5409e72e5f68ae8842d0655ba1c2f6efff > Did you mean to post that to bug #678846? It seems like gnome-autogen.sh is > smart enough to pick up the m4 directory--like I said, I tested this both with > and without vala installed. Yes. Tested...how? I can say that in my gnome-ostree build system, your code didn't work. My guess is that you didn't actually uninstall vala completely. The gnome-ostree build system constructs complete chroots for every build, so I know vala wasn't in there.
(In reply to comment #6) > (In reply to comment #5) > > > > Fixed. Tested both with and without vala installed, both in VPATH regular > > builds. > > I don't know how you're doing your VPATH builds, but it seems wrong: > http://git.gnome.org/browse/libsecret/commit/?id=e72c4c5409e72e5f68ae8842d0655ba1c2f6efff I agree that was wrong. I have no idea why that was working--the only thing I can think of is that I was working with a checkout older than http://git.gnome.org/browse/libsecret/commit/?id=26326a7c8b6700bd36404691a2901030a3005a51 when I made it. I'm normally very good about making sure I'm using master, but that's the only explanation I can think of... > > Did you mean to post that to bug #678846? It seems like gnome-autogen.sh is > > smart enough to pick up the m4 directory--like I said, I tested this both with > > and without vala installed. > > Yes. Tested...how? I can say that in my gnome-ostree build system, your code > didn't work. My guess is that you didn't actually uninstall vala completely. > The gnome-ostree build system constructs complete chroots for every build, so I > know vala wasn't in there. Tested basically how http://www.gnu.org/software/automake/manual/html_node/VPATH-Builds.html says to do it. Create a directory in $(top_srcdir), cd into it, then ../autogen.sh ... (using autogen.sh instead of configure being the difference between what I did and the example in the automake manual). I tested the patch I just posted in #c5 (attachment #217556 [details]) with and without Vala installed (using `make uninstall` in my vala source tree, and I manually verified that /opt/gnome/share/aclocal/vapigen.m4 was removed). I did not test the libsecret patch without vala installed. Are you saying that libgnome-keyring, with attachment #217556 [details] applied, fails?
<nemequ> walters, are you sure you also had that problem with libgnome-keyring, not just libsecret? you would have had to manually apply the patch from bugzilla... <walters> nemequ, nope, only libsecret; i did comment on the wrong bug So afaik this patch is good to go.
Review of attachment 217556 [details] [review]: ::: library/GnomeKeyring-1.0.metadata @@ +50,3 @@ +@lock skip=false +lock_all skip=false +memory_* skip=false Are you sure you want to expose these memory functions? The memory returned cannot be freed via g_free/free.
(In reply to comment #9) > Review of attachment 217556 [details] [review]: > > ::: library/GnomeKeyring-1.0.metadata > @@ +50,3 @@ > +@lock skip=false > +lock_all skip=false > +memory_* skip=false > > Are you sure you want to expose these memory functions? The memory returned > cannot be freed via g_free/free. It shouldn't be a problem... they return void*, so Vala will not try to manage the memory automatically. Basically, they work just like in C. That said, the real reason for exposing them is to avoid a regression, since the bindings currently distributed with Vala expose them.
commit ea1476c1a941d71d6d7be90aa6fc5b8ba8da7c83 Author: Evan Nemerson <evan@coeus-group.com> Date: Thu Jun 28 11:20:04 2012 -0700 Add Vala bindings. https://bugzilla.gnome.org/show_bug.cgi?id=678440