For now, gnome-ostree 3.8 builds are tagged to the previous e-d-s commit:
http://git.gnome.org/browse/gnome-ostree/commit/?id=fea3d6c6e5ecf09efbbc9e0decfa8afd204fc09d

Created attachment 231009 [details] [review]
vala: add type argument information for E.NamedParameters

I believe this is the right patch.  ENamedParameters is a GPtrArray of strings, right?

Review of attachment 231009 [details] [review]:

So this patch overrides the current gir file - it will fix the Vala bindings probably, but will not allow access to the API from "pure introspection" bindings like gjs/pygobject.

Regardless, this appears to fix the build, and so if there are no actual evolution "pure introspection" binding users, we should apply it.

I'm sorry, but I compile eds without any issue with --enable-vala-bindings. This is what configure says:
	Introspection:		yes
	Vala bindings:		yes

and there is no error printed after make && make install. This is what I see there:

make[2]: Entering directory `evolution-data-server/vala'
  VAPIG  ../libedataserver/EDataServer-1.2.gir
warning: empty metadata
Generation succeeded - 1 warning(s)
  GEN      libedataserver-1.2.vapi
  VAPIG  ../addressbook/libebook/EBook-1.2.gir libedataserver-1.2.vapi libebook-1.2-custom.vala
Generation succeeded - 0 warning(s)
  VAPIG  ../calendar/libecal/ECalendar-1.2.gir libedataserver-1.2.vapi
warning: empty metadata
Generation succeeded - 1 warning(s)
  GEN      .gitignore

Could it be that I'm on Fedora 18, with vala-0.18.1-1.fc18? Note, I'm at commit ff581ea.

Created attachment 231027 [details] [review]
proposed eds patch

for evolution-data-server;

I enabled --warn-all for introspection and after some fiddling managed to get rid of the warnings about ENamedParameters with this changes. Does it work for you too, please?

I get the broken build too.  Testing the patch.

Created attachment 231033 [details] [review]
the difference the previous patch makes

This is a before-vs-after of the .gir generated as a result of the previous patch.  Note that it doesn't touch the offending line, so the bug is not fixed...

Ok, this had come up in introspection before, but it took me a while to find the bug:

https://bugzilla.gnome.org/show_bug.cgi?id=629682

The current state of things is still fairly broken, because bindings won't know the (element-type).  We need to:

1) make the scanner understand that this won't work
2) allow adding (element-type) in the right place.  It would be something like this:

diff --git a/libedataserver/e-data-server-util.h b/libedataserver/e-data-server-util.h
index 4048c83..0a5c145 100644
--- a/libedataserver/e-data-server-util.h
+++ b/libedataserver/e-data-server-util.h
@@ -138,11 +138,9 @@ void               e_data_server_util_set_dbus_call_timeout
 
 typedef GPtrArray ENamedParameters;
 /**
- * ENamedParameters:
- * (element-type utf8):
+ * ENamedParameters: (element-type utf8)
  * Array of named parameters.
- **/
-
+ */

Milan, a few things:

* You can use "jhbuild buildone vala" to get an up-to-date compiler; vapigen appears to have gotten better recently (in detecting broken .gir files)

* Thanks for adding --warn-all!  One thing I recommend now is to use (skip) for APIs that aren't introspectable, to cut down on the warnings.

*** Bug 689918 has been marked as a duplicate of this bug. ***

Created attachment 231057 [details] [review]
proposed eds patch ][

for evolution-data-server;

Say something like this? It disables the ENamedParameters API from introspection, at least till the vapigen is taught how to read element-type from the type definition.

(In reply to comment #11)
> Created an attachment (id=231057) [details] [review]
> proposed eds patch ][
> 
> for evolution-data-server;
> 
> Say something like this? It disables the ENamedParameters API from
> introspection, at least till the vapigen is taught how to read element-type
> from the type definition.

Not vapigen, g-ir-scanner.  AFAICT the element-type annotation on the type is not included in the GIR, so there is nothing vapigen can do other than what I proposed in attachment #231009 [details].

Manually changing the GIR from

    <alias name="NamedParameters" c:type="ENamedParameters">
      <doc xml:whitespace="preserve">List of named parameters.</doc>
      <type name="GLib.PtrArray" c:type="GPtrArray"/>
    </alias>

to

    <alias name="NamedParameters" c:type="ENamedParameters">
      <doc xml:whitespace="preserve">List of named parameters.</doc>
      <type name="GLib.PtrArray" c:type="GPtrArray">
        <type name="utf8" c:type="gchar*"/>
      </type>
    </alias>

and feeding it to vapigen results in a correct VAPI.  Until g-ir-scanner picks up the element-type annotation on ENamedParameters I don't see why attachment #231057 [details] is preferable to attachment #231009 [details], and after it does neither patch should be necessary (though an element-type annotation should be added to ENamedParameters).

I spent an hour looking at implementing this in g-ir-scanner.  However, it is really quite ugly.  The core of the problem is that bindngs don't know about ENamedParameters - they only know generically about GPtrArray, GList, GSList, etc.  
Thus, we need to unwind the typedef.

However, if we do that, there's no place to put the e_named_parameters_*() methods.

I have a different idea - let's make ENamedParameters a boxed type, fully opaque even to C users.

(In reply to comment #13)
> I spent an hour looking at implementing this in g-ir-scanner.  However, it is
> really quite ugly.  The core of the problem is that bindngs don't know about
> ENamedParameters - they only know generically about GPtrArray, GList, GSList,
> etc.

Should I file a bug with a dependency on #639908?

> Thus, we need to unwind the typedef.
> 
> However, if we do that, there's no place to put the e_named_parameters_*()
> methods.
> 
> I have a different idea - let's make ENamedParameters a boxed type, fully
> opaque even to C users.

G-I doesn't support inheritance on non-GObject types, right?  So wouldn't all of the GPtrArray methods have to be wrapped (i.e., e_named_parameters_sort, e_named_parameters_index, e_named_parameters_remove, etc.) in order to access the relevant functionality?

Created attachment 231188 [details] [review]
Make ENamedParameters a fully boxed type

Introspection doesn't handle well typedefs as a general rule, but
typedefs for container types are particularly problematic.  This ends
up breaking the vala build, but the resulting .gir is also unusable by
bindings.

In this case, I think it's actually cleanest to make ENamedParameters
a fully opaque type even to C.  The parts of the Evolution code that
were peeking inside ENamedParameters really wanted a helper function
anyways to create a gchar **.

Therefore, this is a net code cleanup.

Review of attachment 231188 [details] [review]:

OK, works for me. Below are mostly only coding style issues. Please fix them before committing (or Matthew's script will fix it for you).

::: libebackend/e-user-prompter-server.c
@@ +475,1 @@
+			values = e_named_parameters_to_strv (extension_values);

this was to save memory allocations, if the passed-in array is NULL-terminated. I'm not sure if you care.

::: libebackend/e-user-prompter.c
@@ +203,3 @@
 	g_return_val_if_fail (dbus_prompter != NULL, FALSE);
 	g_return_val_if_fail (async_data != NULL, FALSE);
+	

no tab at the beginning of the line

::: libedataserver/e-data-server-util.c
@@ +1753,3 @@
 	parameters = e_named_parameters_new ();
 	for (ii = 0; strv[ii]; ii++) {
+		g_ptr_array_add ((GPtrArray*)parameters, g_strdup (strv[ii]));

coding style, should be:
   g_ptr_array_add ((GPtrArray *) parameters,...

@@ +1792,3 @@
 	g_return_if_fail (parameters != NULL);
 
+	array = (GPtrArray*) parameters;

(GPtrArray *)

@@ +1818,3 @@
 	if (from) {
 		gint ii;
+		GPtrArray *from_array = (GPtrArray*)from;

(GPtrArray *)

@@ +1838,3 @@
 	name_len = strlen (name);
 
+	array = (GPtrArray*)parameters;

(GPtrArray *) parameters

@@ +1879,3 @@
 	g_return_if_fail (*name != '\0');
 
+	array = (GPtrArray*)parameters;

(GPtrArray *) parameters

@@ +1923,3 @@
 		return NULL;
 
+	name_and_value = g_ptr_array_index ((GPtrArray*)parameters, index);

(GPtrArray *) parameters

@@ +1977,3 @@
+e_named_parameters_to_strv (const ENamedParameters *parameters)
+{
+	GPtrArray *array = (GPtrArray*)parameters;

(GPtrArray *) parameters

@@ +1986,3 @@
+		}
+	}
+	g_ptr_array_add (ret, NULL);

enter before, enter after :)

@@ +1987,3 @@
+	}
+	g_ptr_array_add (ret, NULL);
+	return (gchar**)g_ptr_array_free (ret, FALSE);

return (gchar **) g_ptr_array_free

@@ +1993,3 @@
+e_named_parameters_ref (ENamedParameters *params)
+{
+	return (ENamedParameters*)g_ptr_array_ref ((GPtrArray*)params);

(ENamedParameters *) g_ptr_array_ref ((GPtrArray *) params)

@@ +1999,3 @@
+e_named_parameters_unref (ENamedParameters *params)
+{
+	g_ptr_array_unref ((GPtrArray*)params);

((GPtrArray *) params)

::: libedataserver/e-data-server-util.h
@@ +148,1 @@
+GType           e_named_parameters_get_type     (void) G_GNUC_CONST;

I was told that G_GNUC_CONST should not be used, an old information on my side? I do not recall the exact reason, though, something with compiler optimizations, if I recall correctly.

Attachment 231188 [details] pushed as e8ab27d - Make ENamedParameters a fully boxed type

(In reply to comment #16)
> Review of attachment 231188 [details] [review]:
> 
> OK, works for me. Below are mostly only coding style issues. Please fix them
> before committing (or Matthew's script will fix it for you).
> 
> ::: libebackend/e-user-prompter-server.c
> @@ +475,1 @@
> +            values = e_named_parameters_to_strv (extension_values);
> 
> this was to save memory allocations, if the passed-in array is NULL-terminated.
> I'm not sure if you care.

Ah.  Hm...we could probably make ENamedParameters always NULL terminated, and
just hide it from users.

> 
> ::: libebackend/e-user-prompter.c
> @@ +203,3 @@
>      g_return_val_if_fail (dbus_prompter != NULL, FALSE);
>      g_return_val_if_fail (async_data != NULL, FALSE);
> +    
> 
> no tab at the beginning of the line

I couldn't find this tab...if you see it after committing, let me know where =)