Created attachment 350909 [details] [review]
WIP modification to Autotools build system to combine the namespaces

I tried to do a quick fix using a Python script that can merge two .gir files into a single one that we can then pass to g-ir-compiler.

This should work, but there's a problem: the Vala compiler seems to base the GIR namespace on the filename of the .gir file. So if you pass `--gir Tracker_Vala-1.0.gir` to the Vala compiler, it creates a .gir with a namespace `Tracker_Vala`, which makes no sense. However, if the final GIR file is called Tracker-1.0.gir then we can't also have `valac` generate a file with that name.

Carlos pointed out that we may be able to get valac to generate all of the introspection data by writing an appropriate .vapi file. That's worth investigating too.

Possibly one could use vapigen to generate the vapi from the first C-based intermediate gir, then use that as an input for valac to generate a final GIR?

Unless simple, manually writing vapi is rather annyoing.

I had a look at writing a .vapi file that describes the C API, but any source file with a .vapi extension is treated as an "external package" and thus its symbols don't get included in the generated .gir file.

Renaming the file to .vala and making the symbols `extern` is more promising, but still doesn't seem to work.

I included this in the Vala build as tracker-sparql-ccode.vala:

[CCode (cprefix = "Tracker", gir_namespace = "Tracker", gir_version = "1.0", lower_case_cprefix = "tracker_")]
namespace Tracker {
    [CCode (cheader_filename = "libtracker-sparql/tracker-sparql.h")]
    public class Resource : GLib.Object {
        public extern Resource (string? identifier);
        public extern void set_gvalue (string property_uri, GLib.Value value);
    }
}

This causes a build failure because now the header file generated by Vala contains a duplicate definition of the TrackerResource class.

I think the only option is going to be modifying g-ir-compiler to merge multiple .gir files into a single .typelib

(In reply to Sam Thursfield from comment #4)
> 
> I think the only option is going to be modifying g-ir-compiler to merge
> multiple .gir files into a single .typelib

IIUC the mapping between gir and typelib files is 1:1 (a typelib being just a binary representation of a gir), we also have to install both for development purposes.

I think we have to join at the .gir level, I still can think of a couple of options to achieve this that just involve changes on our side:

1) Rely on g-ir-scanner fully to generate the gir file out of the *.[ch] files, we already have gtk-doc style comments in the .vala files that get copied as-is to the generated C files, so we can as well add any missing introspection annotations to them.

2) Resort to sed/xslt surgery to merge the gir generated by valac with the missing annotations from g-ir-scanner.

I'm attaching a POC patch doing the first that seems to work, it also includes a hack that I needed for g-ir-scanner to work with TrackerNotifier, because it uses the tracker_sparql_connection_get() symbol that's only defined later at src/libtracker-sparql-backend/, and need one way or another :(. For master, I think it'd make sense to move this folder to src/libtracker-sparql/backends/ or somesuch.

Created attachment 352310 [details] [review]
POC: libtracker-sparql: Generate gir from C sources.

(In reply to Carlos Garnacho from comment #5)
> (In reply to Sam Thursfield from comment #4)
> > 
> > I think the only option is going to be modifying g-ir-compiler to merge
> > multiple .gir files into a single .typelib
> 
> IIUC the mapping between gir and typelib files is 1:1 (a typelib being just
> a binary representation of a gir), we also have to install both for
> development purposes.

And I missed the bug/ML links you provided. Anyway just throwing ideas here, maybe some sticks :).

> 
> I think we have to join at the .gir level, I still can think of a couple of
> options to achieve this that just involve changes on our side:
> 
> 1) Rely on g-ir-scanner fully to generate the gir file out of the *.[ch]
> files, we already have gtk-doc style comments in the .vala files that get
> copied as-is to the generated C files, so we can as well add any missing
> introspection annotations to them.
> 
> 2) Resort to sed/xslt surgery to merge the gir generated by valac with the
> missing annotations from g-ir-scanner.
> 
> I'm attaching a POC patch doing the first that seems to work, it also
> includes a hack that I needed for g-ir-scanner to work with TrackerNotifier,
> because it uses the tracker_sparql_connection_get() symbol that's only
> defined later at src/libtracker-sparql-backend/, and need one way or another
> :(. For master, I think it'd make sense to move this folder to
> src/libtracker-sparql/backends/ or somesuch.

May GXml can help here.

GIR can be modeled in GXml's Serialization GOM framework. Is really easy using Vala.

Gom allows to read strings and add nodes to other. Each node(s) can be imported, to produce a unique GIR to be used by typelib compiler.

All these using DOM4 GObject API.

I was thinking to do so any way. May could be useful for Vala API autocomplation or some thing else, wanting to access gir files using a GObject API.

I've developed gresg to produce gresources from a list of files, using GXml's GOM.

For your problem about Gir namespace, you should use Vala annotations at namespace level like:

https://git.gnome.org/browse/gxml/tree/gxml/namespace-info.vala.in

Thanks Daniel. The problem is that if we run `valac --gir=Tracker_Vala-1.0.gir` for example then the namespace in the filename overrides the `[CCode (gir_namespace = "Tracker", gir_version = "1.0")]` annotations that we have in `src/libtracker-sparql/tracker-namespace.vala`.

If we did use a custom GIR XML munger than we'd be able to fix that up on the way, so I guess I should indeed look at doing things that way. Would using GXml be easier than Python's various XML modules ? I certainly want to avoid the insanity of xsltproc if possible :-)

Firs all, Vala GIR for Tracker should be the same name as its namespace, so, Tracker-1.0.gir, this is required for all other libraries out there.

Your Vala GIR should be generated in a different directory, may this is the real problem, I haven't tested if --gir=dir/Tracker-1.0.gir works.

Now considering we have two files with the same name Tracker-1.0.gir we want to merge symbols and lets think we have a GXml model for a GIR file to get datafrom (well this is not really necessary but useful for other applications), lets say we will write this code:

for (int i = 0; i < gir1.symbols.length; i++) {
  // Get one symbol from a source GIR file
  var s = gir1.symbols.get_item (i) as Symbol;
  if (s == null) continue;
  // Create a new symbol node for GIR file to merge symbols to
  var ns = gir2.symbols.create_item () as Symbol;
  // Serialize source symbol and deserialize to destiny symbol to merge
  ns.read_from_string (s.write_string ());
  gir2.symbols.append (ns); // merge deserialized symbol to destiny GIR file
}
// Once done write destiny GIR file
gir2.write_file (destiny_file);


Please note that Symbol may will have a different name, depending on modelled API.

Same process could be done but just using "node_name" property from DOM4 API, for each node, so, just interested nodes should be merged, using a very similar steps.

Is important to note that write_string() method is a GomElement one and it will write a string representation of all child nodes in actual element node, so just taking top level nodes (childs nodes of <namespace> node), you will get all its childs nodes added to new deserialized node.

Created attachment 354340 [details] [review]
Fix missing introspection data for libtracker-sparql (when using Meson)

Up til now only the Tracker.SparqlConnection and Tracker.SparqlBuilder
resources were introspectable. This is because we only used the
introspection output from `valac`, but other bits of libtracker-sparql
have since been added that are written in C.

There seems to be no way to generate a single .gir for a combined C and
Vala codebase, so instead I have written a simple `g-ir-merge` script
which can combine two different namespaces into a single .gir.

This is currently tested and working with the Meson build instructions.
It would be possible to implement this for the Autotools build
instructions as well.

Review of attachment 354340 [details] [review]:

If we have to stick to a build time dependency, I indeed prefer it to be python, as it's more ubiquitous than GXml (sorry Daniel, and thanks for helping on the brainstorming :). I think we should also do "the right thing" with autotools though, since I'm still generating tarballs from there.

Besides that I just have cosmetic comments.

::: utils/g-ir-merge/g-ir-merge
@@ +24,3 @@
+libtracker-sparql written in both C and Vala. It's not possible to generate
+a single .gir for all of this code as `g-ir-scanner` can't deal with Vala's
+generated .c code, and `valac` only handles .vala code.

FWIW, the "g-ir-scanner can't deal with Vala generated code" is not quite true, the proof-of-concept patch I attached to this bug went that way with some degree of success. We also have gtk-doc style comments in Vala code that get included as is in generated C code, and we should probably be adding any missing introspection annotations for documentation purposes anyway.

The only thing I saw breaking with that patch are the static symbols that get defined as virtual in src/libtracker-sparql/tracker-connection.vala, but are actually implemented in src/libtracker-sparql-backend/tracker-backend.vala, because we build the gir at an intermediate step before generating the full libtracker-sparql.so in src/libtracker-sparql-backend. That could be presumably fixed if we shuffled all of libtracker-bus,libtracker-direct,libtracker-remote and libtracker-sparql-backend into libtracker-sparql/[bus|direct|remote], which I would like it to eventually happen.

But your solution sounds acceptable IMHO, and doesn't require us to rush major code shuffling in to get introspection working, so take nothing of this comment as an argument against it.

@@ +41,3 @@
+def argument_parser():
+    parser = argparse.ArgumentParser(
+        description="Merge .gir repostories together.")

typo: repositories

@@ +60,3 @@
+
+    This does no post-processing of the data, it simply returns everything
+    including any duplicate or contractory info.

I think this is just fine. If there is such info things will fail quite earlier at link time.

BTW, typo: contradictory :)

@@ +127,3 @@
+    '''Create a new GI repository with a single namespace.'''
+    ElementTree.register_namespace(
+        '', 'http://www.gtk.org/introspection/core/1.0')

The namespace strings seem used in a couple of places, might be better to define those as variables so the string itself is in a single place?

Created attachment 354590 [details] [review]
libtracker-sparql: Drop needless typedef

We don't need defining TrackerSparqlBuilder here, and it causes it
to get picked up twice in C and Vala girs.

Created attachment 354591 [details] [review]
autotools: Build and merge C/Vala girs in libtracker-sparql

Following the meson changes, build 2 separate static libs, generate
introspection stuff for those, and put them together in
src/libtracker-sparql-backend.

Pushed the patches to master \o/. Thanks to everyone involved!

Attachment 354340 [details] pushed as 796e025 - Fix missing introspection data for libtracker-sparql (when using Meson)
Attachment 354590 [details] pushed as 0da7438 - libtracker-sparql: Drop needless typedef
Attachment 354591 [details] pushed as 0348492 - autotools: Build and merge C/Vala girs in libtracker-sparql

Comment on attachment 354340 [details] [review]
Fix missing introspection data for libtracker-sparql (when using Meson)

Can we use '#!/usr/bin/env python3' instead of '#!/usr/bin/python3' here to make it work on systems installing python to different prefixes? FreeBSD doesn't include python in its base, and all python packages use /usr/local prefix by default.

/bin/sh: ../../utils/g-ir-merge/g-ir-merge: not found

The following fix has been pushed:
4187ceb g-ir-merge: Use env to locate python3

Created attachment 356886 [details] [review]
g-ir-merge: Use env to locate python3

Pointed out to fix build on FreeBSD, since python is not
in /usr/bin.

Thanks for the heads up :)