GNOME Bugzilla – Bug 617736
Add gobject introspection support using the introspection.m4 macro
Last modified: 2012-06-27 12:43:07 UTC
Created attachment 160329 [details] [review] Initial patch. More work needs to be done annotating the source code Gobject introspection support was added in bug #569083 but: - It doesn't use the official introspection.m4 macro as described here: http://live.gnome.org/GObjectIntrospection/AutotoolsIntegration - It doesn't work right now since .gir files and .typelib files are not installed.
Have you tested that an out-of-srcdir build still works? Thinks like +EvinceDocument_1_0_gir_FILES = $(addprefix $(srcdir)/,$(introspection_sources)) are wrong.
(In reply to comment #1) > Have you tested that an out-of-srcdir build still works? Yes I have Thinks like > > +EvinceDocument_1_0_gir_FILES = $(addprefix > $(srcdir)/,$(introspection_sources)) > > are wrong. What do you mean that it is wrong? It's adapted directly from: http://live.gnome.org/GObjectIntrospection/AutotoolsIntegration Maybe you mean using 1_0 instead of $(EV_API_VERSION) ?
(In reply to comment #2) > (In reply to comment #1) > Thinks like > > > > +EvinceDocument_1_0_gir_FILES = $(addprefix > > $(srcdir)/,$(introspection_sources)) > > > > are wrong. > > What do you mean that it is wrong? It's adapted directly from: > > http://live.gnome.org/GObjectIntrospection/AutotoolsIntegration That page is wrong too, then. For example, the sources contain the ev-document-type-builtins.[ch] files which exist in builddir, not srcdir. Also, I just noticed the patch is even more wrong. You do +introspection_sources = $(libevdocument_la_SOURCES) but this contains many files which must NOT end up in the GIR since they're internal. The current code only scans the $(INST_H_FILES). > Maybe you mean using 1_0 instead of $(EV_API_VERSION) ? Yes, I missed that. You need to define a EV_API_VERSION_U in configure that is EV_API_VERSION but with '_' substituted for '.' (AC_SUBST([EV_API_VERSION_U],[AS_TR_SH([ev_api_version])]) or something like that should do the trick), and then use EvinceDocument_@EV_API_VERSION_U@_gir_FILES etc. BTW, comment 0 says the .gir and .typelib aren't installed, but I just verified that in fact they are installed by "make install" (to $(datadir)/gir-1.0/ and $(libdir)/girepository-1.0/, resp.) when configuring with --enable-introspection. Not using the introspection.m4 file isn't a problem IMHO, esp. since it seems the docs and/or the macro itself is wrong wrt. builddir issues.
(In reply to comment #3) > (In reply to comment #2) > > (In reply to comment #1) > > Thinks like > > > > > > +EvinceDocument_1_0_gir_FILES = $(addprefix > > > $(srcdir)/,$(introspection_sources)) > > > > > > are wrong. > > > > What do you mean that it is wrong? It's adapted directly from: > > > > http://live.gnome.org/GObjectIntrospection/AutotoolsIntegration > > That page is wrong too, then. For example, the sources contain the > ev-document-type-builtins.[ch] files which exist in builddir, not srcdir. > > Also, I just noticed the patch is even more wrong. You do > > +introspection_sources = $(libevdocument_la_SOURCES) > > but this contains many files which must NOT end up in the GIR since they're > internal. The current code only scans the $(INST_H_FILES). IMHO that's one of the reasons the current code is wrong: the gir-scanner needs to read the *.c files too since that's where the gtk-doc comments *and* annotations are for every function. You can check that in GTK+ itself: http://git.gnome.org/browse/gtk+/tree/gtk/Makefile.am#n992 > > > Maybe you mean using 1_0 instead of $(EV_API_VERSION) ? > > Yes, I missed that. You need to define a EV_API_VERSION_U in configure that is > EV_API_VERSION but with '_' substituted for '.' > (AC_SUBST([EV_API_VERSION_U],[AS_TR_SH([ev_api_version])]) or something like > that should do the trick), and then use > EvinceDocument_@EV_API_VERSION_U@_gir_FILES etc. I'm not sure the gir files version need to have so much detail as given in with EV_API_VERSION. In my system, most of the stuff found /usr/share/gir-1.0/ is either 1.0 or 2.0 but not 2.28 or 2.30. Anyway, this may be a non writter convention and it doesn't matter too much. I can change my patch to do what you suggest without problems. > > > BTW, comment 0 says the .gir and .typelib aren't installed, but I just verified > that in fact they are installed by "make install" (to $(datadir)/gir-1.0/ and > $(libdir)/girepository-1.0/, resp.) when configuring with > --enable-introspection. You are right. I thought the gobject introspection support was enabled by default and that's why I didn't got them generated. Is there any reason to not enable it by default?
In reply to comment #4) > > but this contains many files which must NOT end up in the GIR since they're > > internal. The current code only scans the $(INST_H_FILES). > > IMHO that's one of the reasons the current code is wrong: the gir-scanner needs > to read the *.c files too since that's where the gtk-doc comments *and* > annotations are for every function. Yes, I know, some .c files need to be added. But only in a way that doesn't expose stuff to the gir that isn't in the installed headers. So probably adding $(filter %.c,$(libevdocument_la_SOURCES)) to the dependencies on EvinceDocument-$(EV_API_VERSION).gir: libevdocument.la Makefile $(INST_H_FILES) and adding $(filter %.c,$^) after the filter for %.h, should be enough. Need to verify though that the gir scanner only adds things to the gir that are in the .h files even if the .c files have more stuff. > > > Maybe you mean using 1_0 instead of $(EV_API_VERSION) ? > > > > Yes, I missed that. You need to define a EV_API_VERSION_U in configure that is > > EV_API_VERSION but with '_' substituted for '.' > > (AC_SUBST([EV_API_VERSION_U],[AS_TR_SH([ev_api_version])]) or something like > > that should do the trick), and then use > > EvinceDocument_@EV_API_VERSION_U@_gir_FILES etc. > > I'm not sure the gir files version need to have so much detail as given in > with EV_API_VERSION. In my system, most of the stuff found /usr/share/gir-1.0/ > is either 1.0 or 2.0 but not 2.28 or 2.30. Anyway, this may be a non writter > convention and it doesn't matter too much. I can change my patch to do what you > suggest without problems. Well, the current files have this version, so I think it should be kept. Plus, in contrast to those other libs that are at version 1.0 or 2.0, evince isn't API stable, and headers themselves are versioned too, so this should all be consistent. > You are right. I thought the gobject introspection support was enabled by > default and that's why I didn't got them generated. Is there any reason to not > enable it by default? I didn't enable it by default because it's experimental. And also because there's no users of the evince girs anywhere in gnome, so enableing it by default just adds an unnecessary build dependency.
(In reply to comment #5) > Yes, I know, some .c files need to be added. But only in a way that doesn't > expose stuff to the gir that isn't in the installed headers. So probably adding > $(filter %.c,$(libevdocument_la_SOURCES)) to the dependencies on > > EvinceDocument-$(EV_API_VERSION).gir: libevdocument.la Makefile $(INST_H_FILES) > > and adding $(filter %.c,$^) after the filter for %.h, should be enough. > > Need to verify though that the gir scanner only adds things to the gir that are > in the .h files even if the .c files have more stuff. I did that on master, and it seems to work, picking up an annotation I had temporarility added to a .c file.
*** This bug has been marked as a duplicate of bug 678971 ***