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 630342 - scanner: Remove dependency on libgirepository for dumper
scanner: Remove dependency on libgirepository for dumper
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)
Depends on:
Blocks:
 
 
Reported: 2010-09-22 15:20 UTC by Colin Walters
Modified: 2015-02-07 16:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
debug log of Gtk-3.0.gir build without -L/path/to/builroot and no la files (even in buildroot) (420.57 KB, text/plain)
2010-09-24 00:12 UTC, Alban Browaeys
  Details
scanner: Don't link to -lgirepository when dumping (10.40 KB, patch)
2010-09-30 16:32 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2010-09-22 15:20:55 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/.
Comment 1 Johan (not receiving bugmail) Dahlin 2010-09-23 11:56:57 UTC
libc is pushing it a bit, but only using glib should be fine.
Comment 2 Colin Walters 2010-09-23 17:19:07 UTC
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.
Comment 3 Alban Browaeys 2010-09-24 00:12:06 UTC
Created attachment 170994 [details]
debug log of Gtk-3.0.gir build without -L/path/to/builroot and no la files (even in buildroot)
Comment 4 Alban Browaeys 2010-09-24 00:13:05 UTC
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
Comment 5 Colin Walters 2010-09-30 16:32:29 UTC
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.
Comment 6 Johan (not receiving bugmail) Dahlin 2010-09-30 16:46:23 UTC
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
Comment 7 Colin Walters 2010-09-30 17:35:20 UTC
(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 ?
Comment 8 Johan (not receiving bugmail) Dahlin 2010-09-30 17:39:55 UTC
(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):
  • File "<stdin>", line 1 in <module>
  • File "<stdin>", line 1 in foo
    AssertionError
  • File "<stdin>", line 1 in <module>
TypeError: bar() takes exactly 3 arguments (2 given)

Comment 9 Colin Walters 2010-09-30 17:58:05 UTC
Attachment 171442 [details] pushed as 82e08ec - scanner: Don't link to -lgirepository when dumping
Comment 10 André Klapper 2015-02-07 16:45:57 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]