[libpeas] Order the PeasEngine:plugin-list by dependencies

[Garrett Regier <gregier src gnome org>]

commit ddd65c849420fa390977971208c7a8ea2d8ea6a3
Author: Garrett Regier <garrettregier gmail com>
Date:   Tue Nov 17 23:23:03 2015 -0800

    Order the PeasEngine:plugin-list by dependencies
    
    Otherwise PeasExtensionSet could add an extension
    for a plugin before that of a plugin it depends on.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=758446

 libpeas/peas-engine.c             |   49 +++++++++++++++++++++++++++++++------
 tests/libpeas/engine.c            |   34 +++++++++++++++++++++++++
 tests/plugins/Makefile.am         |    1 +
 tests/plugins/two-deps.plugin     |    7 +++++
 tests/testing-util/testing-util.c |   15 ++++++++++-
 5 files changed, 96 insertions(+), 10 deletions(-)
---
diff --git a/libpeas/peas-engine.c b/libpeas/peas-engine.c
index ccee8a6..65efd0d 100644
--- a/libpeas/peas-engine.c
+++ b/libpeas/peas-engine.c
@@ -124,6 +124,42 @@ static void peas_engine_load_plugin_real   (PeasEngine     *engine,
 static void peas_engine_unload_plugin_real (PeasEngine     *engine,
                                             PeasPluginInfo *info);
 
+static GList *
+plugin_info_prepend_sorted (GList          *plugin_list,
+                            PeasPluginInfo *info)
+{
+  guint i;
+  GList *furthest_dep = NULL;
+  const gchar **dependencies;
+
+  dependencies = peas_plugin_info_get_dependencies (info);
+
+  for (i = 0; dependencies[i] != NULL; ++i)
+    {
+      GList *pos = furthest_dep != NULL ? furthest_dep : plugin_list;
+
+      for (; pos != NULL; pos = pos->next)
+        {
+          if (strcmp (dependencies[i],
+                      peas_plugin_info_get_module_name (pos->data)) == 0)
+            {
+              furthest_dep = pos;
+              break;
+            }
+        }
+    }
+
+  if (furthest_dep == NULL)
+    return g_list_prepend (plugin_list, info);
+
+  g_debug ("Adding '%s' after '%s' due to dependencies",
+           peas_plugin_info_get_module_name (info),
+           peas_plugin_info_get_module_name (furthest_dep->data));
+
+  g_list_insert (furthest_dep, info, 1);
+  return plugin_list;
+}
+
 static void
 load_plugin_info (PeasEngine  *engine,
                   const gchar *filename,
@@ -144,20 +180,17 @@ load_plugin_info (PeasEngine  *engine,
       return;
     }
 
-  /* If a plugin with this name has already been loaded
-   * drop this one (user plugins override system plugins) */
   module_name = peas_plugin_info_get_module_name (info);
   if (peas_engine_get_plugin_info (engine, module_name) != NULL)
     {
       _peas_plugin_info_unref (info);
+      return;
     }
-  else
-    {
-      priv->plugin_list = g_list_prepend (priv->plugin_list, info);
 
-      g_object_notify_by_pspec (G_OBJECT (engine),
-                                properties[PROP_PLUGIN_LIST]);
-    }
+  priv->plugin_list = plugin_info_prepend_sorted (priv->plugin_list, info);
+
+  g_object_notify_by_pspec (G_OBJECT (engine),
+                            properties[PROP_PLUGIN_LIST]);
 }
 
 static void
diff --git a/tests/libpeas/engine.c b/tests/libpeas/engine.c
index bcfa991..8b297b9 100644
--- a/tests/libpeas/engine.c
+++ b/tests/libpeas/engine.c
@@ -263,6 +263,39 @@ test_engine_not_loadable_plugin (PeasEngine *engine)
 }
 
 static void
+test_engine_plugin_list (PeasEngine *engine)
+{
+  GList *plugin_list;
+  const gchar **dependencies;
+  gint builtin_index, loadable_index, two_deps_index;
+  PeasPluginInfo *builtin_info, *loadable_info, *two_deps_info;
+
+  plugin_list = (GList *) peas_engine_get_plugin_list (engine);
+
+  builtin_info = peas_engine_get_plugin_info (engine, "builtin");
+  loadable_info = peas_engine_get_plugin_info (engine, "loadable");
+  two_deps_info = peas_engine_get_plugin_info (engine, "two-deps");
+
+  builtin_index = g_list_index (plugin_list, builtin_info);
+  loadable_index = g_list_index (plugin_list, loadable_info);
+  two_deps_index = g_list_index (plugin_list, two_deps_info);
+
+  g_assert_cmpint (builtin_index, !=, -1);
+  g_assert_cmpint (loadable_index, !=, -1);
+  g_assert_cmpint (two_deps_index, !=, -1);
+
+  /* Verify that we are finding the furthest dependency in the list */
+  dependencies = peas_plugin_info_get_dependencies (two_deps_info);
+  g_assert_cmpint (loadable_index, <, builtin_index);
+  g_assert_cmpstr (dependencies[0], ==, "loadable");
+  g_assert_cmpstr (dependencies[1], ==, "builtin");
+
+  /* The two-deps plugin should be ordered after builtin and loadable */
+  g_assert_cmpint (builtin_index, <, two_deps_index);
+  g_assert_cmpint (loadable_index, <, two_deps_index);
+}
+
+static void
 load_plugin_cb (PeasEngine     *engine,
                 PeasPluginInfo *info,
                 gint           *loaded)
@@ -454,6 +487,7 @@ main (int    argc,
   TEST ("unavailable-plugin", unavailable_plugin);
   TEST ("not-loadable-plugin", not_loadable_plugin);
 
+  TEST ("plugin-list", plugin_list);
   TEST ("loaded-plugins", loaded_plugins);
 
   TEST ("enable-unkown-loader", enable_unkown_loader);
diff --git a/tests/plugins/Makefile.am b/tests/plugins/Makefile.am
index eb07efe..cd5b8cd 100644
--- a/tests/plugins/Makefile.am
+++ b/tests/plugins/Makefile.am
@@ -9,6 +9,7 @@ SUBDIRS = \
 noinst_PLUGIN = \
        full-info.plugin                \
        min-info.plugin                 \
+       two-deps.plugin                 \
        unavailable.plugin
 
 EXTRA_DIST = $(noinst_PLUGIN)
diff --git a/tests/plugins/two-deps.plugin b/tests/plugins/two-deps.plugin
new file mode 100644
index 0000000..b463a82
--- /dev/null
+++ b/tests/plugins/two-deps.plugin
@@ -0,0 +1,7 @@
+[Plugin]
+Module=two-deps
+Depends=loadable;builtin
+Name=Two Deps
+Description=This plugin cannot be loaded and has two deps.
+Authors=Garrett Regier
+Copyright=Copyright © 2015 Garrett Regier
diff --git a/tests/testing-util/testing-util.c b/tests/testing-util/testing-util.c
index 8c6c28d..5229945 100644
--- a/tests/testing-util/testing-util.c
+++ b/tests/testing-util/testing-util.c
@@ -249,8 +249,19 @@ testing_util_engine_new_full (gboolean nonglobal_loaders)
                      (GWeakNotify) engine_weak_notify,
                      NULL);
 
-  peas_engine_add_search_path (engine, BUILDDIR "/tests/plugins",
-                                       SRCDIR   "/tests/plugins");
+  /* The plugins that two-deps depends on must be added
+   * to the engine before it. This is used to verify that
+   * the engine will order the plugin list correctly.
+   */
+  peas_engine_add_search_path (engine,
+                               BUILDDIR "/tests/plugins/builtin",
+                               SRCDIR   "/tests/plugins/builtin");
+  peas_engine_add_search_path (engine,
+                               BUILDDIR "/tests/plugins/loadable",
+                               SRCDIR   "/tests/plugins/loadable");
+  peas_engine_add_search_path (engine,
+                               BUILDDIR "/tests/plugins",
+                               SRCDIR   "/tests/plugins");
 
   return engine;
 }