GNOME Bugzilla – Bug 630342
scanner: Remove dependency on libgirepository for dumper
Last modified: 2015-02-07 16:45:57 UTC
The core problem is that by linking to libgirepository in the dumped C program, when we invoke libtool to build it, upon finding libgirepository.la from the build root, will add -L /path/to/buildroot, which forces us to find any *old* installed versions of any library, including the one we're trying to dump. Short of fixing libtool not to install .la files, we can make the dumped C program not depend on libgirepository, which will work around the problem. Implementation: Rewrite gdump.c to just use libc APIs. Install it into $(datadir)/gobject-introspection-1.0/.
libc is pushing it a bit, but only using glib should be fine.
Er, yeah obviously we'd need to use gobject to be able to call g_* to dump type info =) Which actually means that I think we'd work around some problems, but if glib gets built in the build root, we'd probably be screwed again. Anyways gtk-doc only uses glib, our use of libgirepository adds complexity here because it's much more common to build gobject-introspection than it is to build glib.
Created attachment 170994 [details] debug log of Gtk-3.0.gir build without -L/path/to/builroot and no la files (even in buildroot)
the issue I see currently is that if we remove la files from both system libraries and /path/to/buildroot, and remove this -L/path/to/buildroot (simple way is to edit install/lib/gobject-introspection/giscanner/dumper.py and comment line 208 to 214 ie: if not uninst_buildir: (to) args.extend(proc.communicate()[0].split()) this removes this libdir from the link path. But now libtool has no clue where to find its dependencies. It does the same as when la files where installed system wide with incompatible libraries. Attached is a run of Gtk-3.0.gir build with libtool --verbose . searchdirs are what it uses to look for la "or" so. Nevertheless there is an issue with this -L/path/to/buildroot as you explained . That is for gtk which depends on gdk from the same source module we use the /path/to/buildroot gdk in link introspection step to build Gtk-3.0.gir . And when a new symbol was added to gdk and used in gtk this fails to link. What seems to matter is the order in which the arguments to the link command are passed when it comes to la vs -L precedence (ie for Gtk-3.0-gir -L/path/to/buildroot is before the libgtk-x11-3.0.la ... though this is moot as even if libgtk-x11-3.0.la define properly the location of the same source module internal build of libgdk-x11-3.0.la its dependency_libs start with -L/path/to/buildroot) That is we have to explicitely add ../gdk/libgtk-x11-3.0.la first to the Gtk-3.0.gir link command. https://bugzilla.gnome.org/show_bug.cgi?id=630106
Created attachment 171442 [details] [review] scanner: Don't link to -lgirepository when dumping This helps us avoid a problematic case where in say jhbuild, using a system (/usr/lib) glib, adding in -l girepository-1.0 will inject -L /path/to/builddir, when we don't want that.
Review of attachment 171442 [details] [review]: I'd rather see gdump.c cleaned up a little bit before this goes in. ::: girepository/Makefile.am @@ +70,3 @@ libgirepository_parser_la_CFLAGS = $(GIREPO_CFLAGS) +gdumpdir = $(datadir)/gobject-introspection-1.0/ Is this the same directory that everything.c is installed to? ::: girepository/gdump.c @@ +27,3 @@ #include "config.h" +/* This file is both compiled into libgirepository.so, and installed Can't you just: 1) make g_irepository_dump private 2) create a simple public wrapper around g_irepository_dump() so we can avoid a little bit of ugliness? ::: giscanner/dumper.py @@ +94,3 @@ # Public API def run(self): Wow, this is actually /a lot/ nicer. ::: giscanner/scannermain.py @@ +293,3 @@ +def create_binary(transformer, options, args, pkg_datadir=None): + assert pkg_datadir is not None This is wrong, you should either allow None or don't put it as a default value ::: tools/Makefile.am @@ +6,1 @@ bin_SCRIPTS = g-ir-scanner g-ir-annotation-tool Nice, perhaps a separate commit
(In reply to comment #6) > Review of attachment 171442 [details] [review]: > > I'd rather see gdump.c cleaned up a little bit before this goes in. > > ::: girepository/Makefile.am > @@ +70,3 @@ > libgirepository_parser_la_CFLAGS = $(GIREPO_CFLAGS) > > +gdumpdir = $(datadir)/gobject-introspection-1.0/ > > Is this the same directory that everything.c is installed to? everything.c is in a tests/ subdirectory. > Can't you just: > 1) make g_irepository_dump private > 2) create a simple public wrapper around g_irepository_dump() > > so we can avoid a little bit of ugliness? g_irepository_dump is public libgirepository API, I don't understand why it makes sense to remove it, only to add a new API that would be identical? If you don't like the ugliess of the #ifdef static, I can simply remove it - the only cost is the dumped binary exporting a symbol, which totally doesn't matter. > Wow, this is actually /a lot/ nicer. Yeah. > +def create_binary(transformer, options, args, pkg_datadir=None): > + assert pkg_datadir is not None > > This is wrong, you should either allow None or don't put it as a default value So pkg_datadir=42 ?
(In reply to comment #7) > (In reply to comment #6) [..] > > Can't you just: > > 1) make g_irepository_dump private > > 2) create a simple public wrapper around g_irepository_dump() > > > > so we can avoid a little bit of ugliness? > > g_irepository_dump is public libgirepository API, I don't understand why it > makes sense to remove it, only to add a new API that would be identical? > > If you don't like the ugliess of the #ifdef static, I can simply remove it - > the only cost is the dumped binary exporting a symbol, which totally doesn't > matter. Yes, that's a bit better. > > Wow, this is actually /a lot/ nicer. > > Yeah. > > > +def create_binary(transformer, options, args, pkg_datadir=None): > > + assert pkg_datadir is not None > > > > This is wrong, you should either allow None or don't put it as a default value > > So pkg_datadir=42 ? The difference is that the error messages are nicer if you just remove the default value, eg: def foo(a, b, c=None): assert c is not None def bar(a, b, c): pass >>> foo(1, 2) Traceback (most recent call last):
+ Trace 223958
AssertionError
Attachment 171442 [details] pushed as 82e08ec - scanner: Don't link to -lgirepository when dumping
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]