Created attachment 202913 [details] [review]
Add g-ir-dep-scanner tool

Patch version 1 - Currently fail to build git master, but even without this patch. The .c source is tested, just the integration in the build system is not sure to work (yet)

Review of attachment 202913 [details] [review]:

Use /* */ comments.

Scanner suffix is confusing, better to call it tool.

You need to add it to noinst_PROGRAMS.

Created attachment 202916 [details] [review]
Patch version 2

Reworked patch, taken your comments (and some other I received in the meantime) into account.

Even have made test builds and integration into the build system is fine with this patch.

Created attachment 203121 [details] [review]
Patch version 3

Sorry for the noise. But I just came across the fact that shared libraries can actually be a comma separated list, so I now split the returned string in pieces in an array and return one library per line (thus giving the same output as with the typelib based dependencies).

Example with Gkbd-3.0:
> g-ir-dep-tool Gkbd
typelib: xlib-2.0
typelib: cairo-1.0
typelib: Pango-1.0
typelib: Gtk-3.0
typelib: Gio-2.0
typelib: GdkPixbuf-2.0
typelib: Gdk-3.0
typelib: GObject-2.0
typelib: GModule-2.0
typelib: GLib-2.0
typelib: Atk-1.0
shlib: libgnomekbd.so.7
shlib: libgnomekbdui.so.7

FYI: openSUSE has had this patch in for since the patch existed and served a great deal already in not having too many broken deps for this...

Created attachment 226537 [details]
for reference: this is the rpm script we use in openSUSE to create the dependencies.

Can I add a "me too" to this patch.

We (Mageia) use a different mechanism to extract deps automatically, but it would certainly help reduce pointless distribution variations if this was available in an upstream package.

Sorry for not replying earlier, didn't see this patch go by.  So I see the generic desire for a tool to output data from typelibs is an easily readable form.

Typelibs are a bit like ELF files, so maybe what we want is something a bit more generic like "eu-readelf".  What about calling the tool g-ir-readtypelib, and adding options like:

--print-shlibs
--print-typelibs

Then it'd be a bit more future proof, we could add:

--print-symbols

etc.

Note of course that unlike ELF, girepository also ships with a public library to consume the typelibs, so it's also an option to maintain such a tool inside RPM or whatever.

As the original author of that, and seeing that there is demand by distributions (which is great motivation of course to do more out of this), I am certainly willing to work towards a solution that can be integrated into the upstream code base...

Makes everybodys life easier and we can (better) profit from bugs found by each other (we realized that multiple dists already use this scanner / rpm dep but they obviously drifted apart).

Your suggestion of g-ir-readtype is acceptable to me (what's in the name :) )

the option parsing is also an option (so far we did not care for it, as we need everything... but it's surely an option).

Would you say that stating the tool without --print* gives the output as it does now? Or only the 'help'? 

Of course, 'combination' of --print-typelibs --print-shlib should be possible (which would be what the tool does now).

I'll try to work on something like this in the next few days.

[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]

Any progress on this?

Created attachment 319247 [details] [review]
rework as per earlier comments

The tool is now named g-ir-readtypelib and uses parameters

--print-shlibs
--print-typelibs

 to print as requested

Review of attachment 319247 [details] [review]:

Looks generally good to me; I'm not sure on the rules for coding style in gobject-introspection tools, but the code is so small that I don't think it makes much of a difference.

Created attachment 326990 [details] [review]
g-ir-dep-scanner: Scan dependencies of a typelib and give information

This allows distributions to create automatic dependency tracking coming
from .typelib files.

The dependencies identified at this time are:
typelib - based dependencies
shared library - dependencies

Reference: https://bugzilla.gnome.org/show_bug.cgi?id=665672
Signed-off-by: Igor Gnatenko <ignatenko@redhat.com>

Created attachment 326994 [details] [review]
g-ir-readtypelib: new tool

Various distributions (mainly RPM based so far) make use of automatic
dependencies extracted from typelib files (they can require other typelibs
and also shared libraries)

g-ir-readtypelib inspects a .typelib file and prints the dependencies.

Reference: https://bugzilla.gnome.org/show_bug.cgi?id=665672
Signed-off-by: Dominique Leuenberger <dimstar@opensuse.org>
Signed-off-by: Igor Gnatenko <ignatenko@redhat.com>

Created attachment 326995 [details] [review]
g-ir-readtypelib: new tool

Various distributions (mainly RPM based so far) make use of automatic
dependencies extracted from typelib files (they can require other typelibs
and also shared libraries)

g-ir-readtypelib inspects a typelib and prints the dependencies.

Reference: https://bugzilla.gnome.org/show_bug.cgi?id=665672
Signed-off-by: Dominique Leuenberger <dimstar@opensuse.org>
Signed-off-by: Igor Gnatenko <ignatenko@redhat.com>

Created attachment 326996 [details] [review]
g-ir-inspect: Inspect GI typelibs

Various distributions (mainly RPM based so far) make use of automatic
dependencies extracted from typelib files (they can require other typelibs
and also shared libraries)

Current features
* Print used shared libraries
* Print used typelib dependencies

Reference: https://bugzilla.gnome.org/show_bug.cgi?id=665672
Signed-off-by: Dominique Leuenberger <dimstar@opensuse.org>
Signed-off-by: Igor Gnatenko <ignatenko@redhat.com>

Dominique, you're okay with Igor taking over this series?

I'm confused, why do we have two patches for the tool with different names?

(In reply to Colin Walters from comment #18)
> Dominique, you're okay with Igor taking over this series?

I'm fine with Igor doing the work - his patch is based on top of mine, apparently with cleanups people seem to care for

Review of attachment 326996 [details] [review]:

::: tools/g-ir-inspect.c
@@ +31,3 @@
+    /* Finding the shared library we depend on (if any) */
+    const gchar *shlibs = g_irepository_get_shared_library (NULL, namespace);
+    if (shlibs && shlibs[0] != '\0') {

Brace on new line.

@@ +44,3 @@
+    /* Finding all the typelib-based Requires */
+    g_auto(GStrv) deps = g_irepository_get_dependencies (NULL, namespace);
+    if (deps) {

g-i is GNU style, brace on new line.

@@ +54,3 @@
+      gchar *argv[])
+{
+    g_autoptr(GError) error = NULL;

Sadly, we need to support MSVC builds of g-i.  So we can't use g_auto here.

@@ +83,3 @@
+        return EXIT_FAILURE;
+    }
+    const gchar *namespace = namespaces[0];

I'd also prefer being conservative with declaration-after-statement here.  Most of the rest of the code doesn't do that yet.

(In reply to Dominique Leuenberger from comment #20)
> (In reply to Colin Walters from comment #18)
> > Dominique, you're okay with Igor taking over this series?
> 
> I'm fine with Igor doing the work - his patch is based on top of mine,
> apparently with cleanups people seem to care for

One thing I'm quite sensitive to personally is attribution.  I'd have at least added a:

"Based on a patch by: "

Created attachment 327025 [details] [review]
g-ir-inspect: Inspect GI typelibs

Various distributions (mainly RPM based so far) make use of automatic
dependencies extracted from typelib files (they can require other typelibs
and also shared libraries)

Current features
* Print used shared libraries
* Print used typelib dependencies

Based-on-patch-by: Dominique Leuenberger <dimstar@opensuse.org>
Reference: https://bugzilla.gnome.org/show_bug.cgi?id=665672
Signed-off-by: Dominique Leuenberger <dimstar@opensuse.org>
Signed-off-by: Igor Gnatenko <ignatenko@redhat.com>

Review of attachment 327025 [details] [review]:

Ok, LGTM.

(In reply to Colin Walters from comment #24)
> Review of attachment 327025 [details] [review] [review]:
> 
> Ok, LGTM.

pushed as 43597c9