girepository: Make gi_repository_find_by_gtype() deterministic

When faced with a `GType` which is present in multiple typelibs, the old
implementation was not deterministic, as it iterated over a hash table
of typelibs. The iteration order of a hash table is not deterministic.

Use the new `ordered_typelibs` and `ordered_lazy_typelibs` arrays to
iterate instead, making the order deterministic.

Add a unit test to check this. In particular, to check that symbols
which are present in both `Gio` and `GioUnix` are correctly resolved as
being from `GioUnix`.

Signed-off-by: Philip Withnall <pwithnall@gnome.org>

Helps: #3303

girepository/girepository.c

@@ -931,32 +931,29 @@ gi_repository_get_info (GIRepository *repository,
                            NULL, typelib, entry->offset);
 }
 
-typedef struct {
-  const char *gtype_name;
-  GITypelib *result_typelib;
-} FindByGTypeData;
-
 static DirEntry *
-find_by_gtype (GHashTable *table, FindByGTypeData *data, gboolean check_prefix)
+find_by_gtype (GPtrArray   *ordered_table,
+               const char  *gtype_name,
+               gboolean     check_prefix,
+               GITypelib  **out_result_typelib)
 {
-  GHashTableIter iter;
+  /* Search in reverse order as the longest namespaces will be listed last, and
-  gpointer key, value;
+   * those are the ones we want to search first. */
-  DirEntry *ret;
+  for (guint i = ordered_table->len; i > 0; i--)
-
-  g_hash_table_iter_init (&iter, table);
-  while (g_hash_table_iter_next (&iter, &key, &value))
     {
-      GITypelib *typelib = (GITypelib*)value;
+      GITypelib *typelib = g_ptr_array_index (ordered_table, i - 1);
+      DirEntry *ret;
+
       if (check_prefix)
         {
-          if (!gi_typelib_matches_gtype_name_prefix (typelib, data->gtype_name))
+          if (!gi_typelib_matches_gtype_name_prefix (typelib, gtype_name))
             continue;
         }
 
-      ret = gi_typelib_get_dir_entry_by_gtype_name (typelib, data->gtype_name);
+      ret = gi_typelib_get_dir_entry_by_gtype_name (typelib, gtype_name);
       if (ret)
         {
-          data->result_typelib = typelib;
+          *out_result_typelib = typelib;
           return ret;
         }
     }
@@ -985,7 +982,8 @@ GIBaseInfo *
 gi_repository_find_by_gtype (GIRepository *repository,
                              GType         gtype)
 {
-  FindByGTypeData data;
+  const char *gtype_name;
+  GITypelib *result_typelib = NULL;
   GIBaseInfo *cached;
   DirEntry *entry;
 
@@ -1001,8 +999,7 @@ gi_repository_find_by_gtype (GIRepository *repository,
   if (g_hash_table_contains (repository->unknown_gtypes, (gpointer)gtype))
     return NULL;
 
-  data.gtype_name = g_type_name (gtype);
+  gtype_name = g_type_name (gtype);
-  data.result_typelib = NULL;
 
   /* Inside each typelib, we include the "C prefix" which acts as
    * a namespace mechanism.  For GtkTreeView, the C prefix is Gtk.
@@ -1011,9 +1008,9 @@ gi_repository_find_by_gtype (GIRepository *repository,
    * target type does not have this typelib's C prefix. Use this
    * assumption as our first attempt at locating the DirEntry.
    */
-  entry = find_by_gtype (repository->typelibs, &data, TRUE);
+  entry = find_by_gtype (repository->ordered_typelibs, gtype_name, TRUE, &result_typelib);
   if (entry == NULL)
-    entry = find_by_gtype (repository->lazy_typelibs, &data, TRUE);
+    entry = find_by_gtype (repository->ordered_lazy_typelibs, gtype_name, TRUE, &result_typelib);
 
   /* Not every class library necessarily specifies a correct c_prefix,
    * so take a second pass. This time we will try a global lookup,
@@ -1021,15 +1018,15 @@ gi_repository_find_by_gtype (GIRepository *repository,
    * See http://bugzilla.gnome.org/show_bug.cgi?id=564016
    */
   if (entry == NULL)
-    entry = find_by_gtype (repository->typelibs, &data, FALSE);
+    entry = find_by_gtype (repository->ordered_typelibs, gtype_name, FALSE, &result_typelib);
   if (entry == NULL)
-    entry = find_by_gtype (repository->lazy_typelibs, &data, FALSE);
+    entry = find_by_gtype (repository->ordered_lazy_typelibs, gtype_name, FALSE, &result_typelib);
 
   if (entry != NULL)
     {
       cached = gi_info_new_full (gi_typelib_blob_type_to_info_type (entry->blob_type),
                                  repository,
-                                 NULL, data.result_typelib, entry->offset);
+                                 NULL, result_typelib, entry->offset);
 
       g_hash_table_insert (repository->info_by_gtype,
                            (gpointer) gtype,

girepository/introspection/meson.build

@@ -268,6 +268,7 @@ if host_system == 'windows'
       '--identifier-prefix=GWin32'
     ],
   )
+  gio_platform_gir = gio_win32_gir
 else
   gio_unix_gir_c_includes = []
   foreach h: gio_unix_include_headers
@@ -297,6 +298,7 @@ else
       '--identifier-prefix=GUnix'
     ],
   )
+  gio_platform_gir = gio_unix_gir
 endif
 
 # GIRepository

girepository/tests/meson.build

@@ -29,6 +29,11 @@ if enable_gir
     gio_gir,
   ]
 
+  gio_platform_gir_testing_dep = [
+    gio_gir_testing_dep,
+    gio_platform_gir,
+  ]
+
   girepository_gir_testing_dep = [
     gio_gir_testing_dep,
     girepository_gir,
@@ -46,7 +51,8 @@ if enable_gir
       'depends': gobject_gir_testing_dep,
     },
     'repository' : {
-      'depends': gio_gir_testing_dep,
+      'depends': [gio_gir_testing_dep, gio_platform_gir_testing_dep],
+      'dependencies': [libgio_dep],
     },
     'repository-search-paths' : {
       'c_args': '-DGOBJECT_INTROSPECTION_LIBDIR="@0@"'.format(glib_libdir),

girepository/tests/repository.c

@@ -35,6 +35,12 @@
 #include "glib.h"
 #include "test-common.h"
 
+#if defined(G_OS_UNIX)
+#include "gio/gdesktopappinfo.h"
+#elif defined(G_OS_WIN32)
+#include "gio/gwin32inputstream.h"
+#endif
+
 static void
 test_repository_basic (RepositoryFixture *fx,
                        const void *unused)
@@ -777,6 +783,67 @@ test_repository_vfunc_info_with_invoker_on_object (RepositoryFixture *fx,
   g_clear_pointer (&invoker_info, gi_base_info_unref);
 }
 
+static void
+test_repository_find_by_gtype (RepositoryFixture *fx,
+                               const void        *unused)
+{
+  GIObjectInfo *object_info = NULL;
+
+  g_test_summary ("Test finding a GType");
+
+  object_info = (GIObjectInfo *) gi_repository_find_by_gtype (fx->repository, G_TYPE_OBJECT);
+  g_assert_nonnull (object_info);
+  g_assert_true (GI_IS_OBJECT_INFO (object_info));
+  g_assert_cmpstr (gi_base_info_get_name (GI_BASE_INFO (object_info)), ==, "Object");
+
+  g_clear_pointer (&object_info, gi_base_info_unref);
+
+  /* Find it again; this time it should hit the cache. */
+  object_info = (GIObjectInfo *) gi_repository_find_by_gtype (fx->repository, G_TYPE_OBJECT);
+  g_assert_nonnull (object_info);
+  g_assert_true (GI_IS_OBJECT_INFO (object_info));
+  g_assert_cmpstr (gi_base_info_get_name (GI_BASE_INFO (object_info)), ==, "Object");
+
+  g_clear_pointer (&object_info, gi_base_info_unref);
+
+  /* Try and find an unknown GType. */
+  g_assert_null (gi_repository_find_by_gtype (fx->repository, GI_TYPE_BASE_INFO));
+
+  /* And check caching for unknown GTypes. */
+  g_assert_null (gi_repository_find_by_gtype (fx->repository, GI_TYPE_BASE_INFO));
+
+  /* Now try and find one which will resolve in both Gio and GioUnix/GioWin32.
+   * The longest-named typelib should be returned. */
+  {
+    GType platform_specific_type;
+    const char *expected_name, *expected_namespace;
+
+#if defined(G_OS_UNIX)
+    platform_specific_type = G_TYPE_DESKTOP_APP_INFO;
+    expected_name = "DesktopAppInfo";
+    expected_namespace = "GioUnix";
+#elif defined(G_OS_WIN32)
+    platform_specific_type = G_TYPE_WIN32_INPUT_STREAM;
+    expected_name = "InputStream";
+    expected_namespace = "GioWin32";
+#else
+    platform_specific_type = G_TYPE_INVALID;
+    expected_name = NULL;
+    expected_namespace = NULL;
+#endif
+
+    if (expected_name != NULL)
+      {
+        object_info = (GIObjectInfo *) gi_repository_find_by_gtype (fx->repository, platform_specific_type);
+        g_assert_nonnull (object_info);
+        g_assert_true (GI_IS_OBJECT_INFO (object_info));
+        g_assert_cmpstr (gi_base_info_get_name (GI_BASE_INFO (object_info)), ==, expected_name);
+        g_assert_cmpstr (gi_base_info_get_namespace (GI_BASE_INFO (object_info)), ==, expected_namespace);
+        g_clear_pointer (&object_info, gi_base_info_unref);
+      }
+  }
+}
+
 int
 main (int   argc,
       char *argv[])
@@ -804,6 +871,7 @@ main (int   argc,
   ADD_REPOSITORY_TEST ("/repository/vfunc-info-with-no-invoker", test_repository_vfunc_info_with_no_invoker, &typelib_load_spec_gobject);
   ADD_REPOSITORY_TEST ("/repository/vfunc-info-with-invoker-on-interface", test_repository_vfunc_info_with_invoker_on_interface, &typelib_load_spec_gio);
   ADD_REPOSITORY_TEST ("/repository/vfunc-info-with-invoker-on-object", test_repository_vfunc_info_with_invoker_on_object, &typelib_load_spec_gio);
+  ADD_REPOSITORY_TEST ("/repository/find-by-gtype", test_repository_find_by_gtype, &typelib_load_spec_gio_platform);
 
   return g_test_run ();
 }

girepository/tests/test-common.h

@@ -36,6 +36,11 @@ typedef struct
 static const TypelibLoadSpec typelib_load_spec_glib = { "GLib", "2.0" };
 static const TypelibLoadSpec typelib_load_spec_gobject = { "GObject", "2.0" };
 static const TypelibLoadSpec typelib_load_spec_gio = { "Gio", "2.0" };
+#if defined(G_OS_UNIX)
+static const TypelibLoadSpec typelib_load_spec_gio_platform = { "GioUnix", "2.0" };
+#elif defined(G_OS_WIN32)
+static const TypelibLoadSpec typelib_load_spec_gio_platform = { "GioWin32", "2.0" };
+#endif
 
 void repository_init (int *argc,
                       char **argv[]);