[gnome-software: 1/29] gs-plugin: Bind initialize/destroy to the GsPlugin lifetime
- From: Philip Withnall <pwithnall src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software: 1/29] gs-plugin: Bind initialize/destroy to the GsPlugin lifetime
- Date: Wed, 13 Oct 2021 12:39:53 +0000 (UTC)
commit 6d60933fbc5054e637f6b2af490b81784b0aa7c6
Author: Philip Withnall <pwithnall endlessos org>
Date: Thu May 20 19:58:12 2021 +0100
gs-plugin: Bind initialize/destroy to the GsPlugin lifetime
This is a step towards having a `GsPlugin` subclass per plugin, which
will allow the vfuncs to be class methods, and avoid a load of `dlsym()`
calls on every plugin job. It will also allow the C type system to be
used, rather than a load of dynamic function pointer casting in
`gs-plugin-loader.c`. Finally, it will prevent the perennial issue of
identically-named symbols from multiple modules being conflicting in the
process’ function name space, and being difficult to grep in the source
code.
Signed-off-by: Philip Withnall <pwithnall endlessos org>
lib/gs-plugin-loader.c | 69 ++++++++++++++------------------------------------
lib/gs-plugin-types.h | 7 ++---
lib/gs-plugin-vfuncs.h | 31 ++++++++++-------------
lib/gs-plugin.c | 34 ++++++++++++-------------
4 files changed, 51 insertions(+), 90 deletions(-)
---
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index 7f5859a05..8278bc2e9 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -617,13 +617,6 @@ gs_plugin_loader_call_vfunc (GsPluginLoaderHelper *helper,
if (gs_plugin_job_get_interactive (helper->plugin_job))
gs_plugin_interactive_inc (plugin);
switch (action) {
- case GS_PLUGIN_ACTION_INITIALIZE:
- case GS_PLUGIN_ACTION_DESTROY:
- {
- GsPluginFunc plugin_func = func;
- plugin_func (plugin);
- }
- break;
case GS_PLUGIN_ACTION_SETUP:
{
GsPluginSetupFunc plugin_func = func;
@@ -843,8 +836,6 @@ gs_plugin_loader_call_vfunc (GsPluginLoaderHelper *helper,
GLogLevelFlags log_level;
switch (action) {
- case GS_PLUGIN_ACTION_INITIALIZE:
- case GS_PLUGIN_ACTION_DESTROY:
case GS_PLUGIN_ACTION_SETUP:
if (g_getenv ("GS_SELF_TEST_PLUGIN_ERROR_FAIL_HARD") == NULL)
log_level = G_LOG_LEVEL_WARNING;
@@ -2406,11 +2397,6 @@ gs_plugin_loader_clear_caches (GsPluginLoader *plugin_loader)
void
gs_plugin_loader_setup_again (GsPluginLoader *plugin_loader)
{
- GsPluginAction actions[] = {
- GS_PLUGIN_ACTION_DESTROY,
- GS_PLUGIN_ACTION_INITIALIZE,
- GS_PLUGIN_ACTION_SETUP,
- GS_PLUGIN_ACTION_UNKNOWN };
#ifdef HAVE_SYSPROF
gint64 begin_time_nsec G_GNUC_UNUSED = SYSPROF_CAPTURE_CURRENT_TIME;
#endif
@@ -2421,28 +2407,23 @@ gs_plugin_loader_setup_again (GsPluginLoader *plugin_loader)
/* remove any events */
gs_plugin_loader_remove_events (plugin_loader);
- /* call in order */
- for (guint j = 0; actions[j] != GS_PLUGIN_ACTION_UNKNOWN; j++) {
- for (guint i = 0; i < plugin_loader->plugins->len; i++) {
- g_autoptr(GError) error_local = NULL;
- g_autoptr(GsPluginLoaderHelper) helper = NULL;
- g_autoptr(GsPluginJob) plugin_job = NULL;
- GsPlugin *plugin = g_ptr_array_index (plugin_loader->plugins, i);
- if (!gs_plugin_get_enabled (plugin))
- continue;
+ for (guint i = 0; i < plugin_loader->plugins->len; i++) {
+ g_autoptr(GError) error_local = NULL;
+ g_autoptr(GsPluginLoaderHelper) helper = NULL;
+ g_autoptr(GsPluginJob) plugin_job = NULL;
+ GsPlugin *plugin = g_ptr_array_index (plugin_loader->plugins, i);
+ if (!gs_plugin_get_enabled (plugin))
+ continue;
- plugin_job = gs_plugin_job_newv (actions[j], NULL);
- helper = gs_plugin_loader_helper_new (plugin_loader, plugin_job);
- if (!gs_plugin_loader_call_vfunc (helper, plugin, NULL, NULL,
- GS_PLUGIN_REFINE_FLAGS_DEFAULT,
- NULL, &error_local)) {
- g_warning ("resetup of %s failed: %s",
- gs_plugin_get_name (plugin),
- error_local->message);
- break;
- }
- if (actions[j] == GS_PLUGIN_ACTION_DESTROY)
- gs_plugin_clear_data (plugin);
+ plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_SETUP, NULL);
+ helper = gs_plugin_loader_helper_new (plugin_loader, plugin_job);
+ if (!gs_plugin_loader_call_vfunc (helper, plugin, NULL, NULL,
+ GS_PLUGIN_REFINE_FLAGS_DEFAULT,
+ NULL, &error_local)) {
+ g_warning ("resetup of %s failed: %s",
+ gs_plugin_get_name (plugin),
+ error_local->message);
+ break;
}
}
@@ -2588,12 +2569,6 @@ gs_plugin_loader_setup (GsPluginLoader *plugin_loader,
}
}
- /* run the plugins */
- plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_INITIALIZE, NULL);
- helper = gs_plugin_loader_helper_new (plugin_loader, plugin_job);
- if (!gs_plugin_loader_run_results (helper, cancellable, error))
- return FALSE;
-
/* order by deps */
do {
changes = FALSE;
@@ -2714,8 +2689,8 @@ gs_plugin_loader_setup (GsPluginLoader *plugin_loader,
} while (changes);
/* run setup */
- gs_plugin_job_set_action (helper->plugin_job, GS_PLUGIN_ACTION_SETUP);
- helper->function_name = "gs_plugin_setup";
+ plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_SETUP, NULL);
+ helper = gs_plugin_loader_helper_new (plugin_loader, plugin_job);
for (i = 0; i < plugin_loader->plugins->len; i++) {
g_autoptr(GError) error_local = NULL;
plugin = g_ptr_array_index (plugin_loader->plugins, i);
@@ -2815,11 +2790,7 @@ gs_plugin_loader_dispose (GObject *object)
GsPluginLoader *plugin_loader = GS_PLUGIN_LOADER (object);
if (plugin_loader->plugins != NULL) {
- g_autoptr(GsPluginLoaderHelper) helper = NULL;
- g_autoptr(GsPluginJob) plugin_job = NULL;
- plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_DESTROY, NULL);
- helper = gs_plugin_loader_helper_new (plugin_loader, plugin_job);
- gs_plugin_loader_run_results (helper, NULL, NULL);
+ /* FIXME: call a ->stop() (or similar) method on every plugin */
g_clear_pointer (&plugin_loader->plugins, g_ptr_array_unref);
}
if (plugin_loader->updates_changed_id != 0) {
@@ -3396,10 +3367,8 @@ gs_plugin_loader_process_thread_cb (GTask *task,
/* some functions are really required for proper operation */
switch (action) {
- case GS_PLUGIN_ACTION_DESTROY:
case GS_PLUGIN_ACTION_GET_INSTALLED:
case GS_PLUGIN_ACTION_GET_UPDATES:
- case GS_PLUGIN_ACTION_INITIALIZE:
case GS_PLUGIN_ACTION_INSTALL:
case GS_PLUGIN_ACTION_DOWNLOAD:
case GS_PLUGIN_ACTION_LAUNCH:
diff --git a/lib/gs-plugin-types.h b/lib/gs-plugin-types.h
index 4674ad228..db6c5b8e0 100644
--- a/lib/gs-plugin-types.h
+++ b/lib/gs-plugin-types.h
@@ -175,7 +175,8 @@ typedef enum {
* @GS_PLUGIN_RULE_BETTER_THAN: Results are better than another
*
* The rules used for ordering plugins.
- * Plugins are expected to add rules in gs_plugin_initialize().
+ * Plugins are expected to add rules in the init function for their #GsPlugin
+ * subclass.
**/
typedef enum {
GS_PLUGIN_RULE_CONFLICTS,
@@ -216,8 +217,6 @@ typedef enum {
* @GS_PLUGIN_ACTION_URL_TO_APP: Convert the URI to an application
* @GS_PLUGIN_ACTION_GET_RECENT: Get the apps recently released
* @GS_PLUGIN_ACTION_GET_UPDATES_HISTORICAL: Get the list of historical updates
- * @GS_PLUGIN_ACTION_INITIALIZE: Initialize the plugin
- * @GS_PLUGIN_ACTION_DESTROY: Destroy the plugin
* @GS_PLUGIN_ACTION_DOWNLOAD: Download an application
* @GS_PLUGIN_ACTION_GET_ALTERNATES: Get the alternates for a specific application
* @GS_PLUGIN_ACTION_GET_LANGPACKS: Get appropriate language pack
@@ -258,8 +257,6 @@ typedef enum {
GS_PLUGIN_ACTION_URL_TO_APP,
GS_PLUGIN_ACTION_GET_RECENT,
GS_PLUGIN_ACTION_GET_UPDATES_HISTORICAL,
- GS_PLUGIN_ACTION_INITIALIZE,
- GS_PLUGIN_ACTION_DESTROY,
GS_PLUGIN_ACTION_DOWNLOAD,
GS_PLUGIN_ACTION_GET_ALTERNATES,
GS_PLUGIN_ACTION_GET_LANGPACKS,
diff --git a/lib/gs-plugin-vfuncs.h b/lib/gs-plugin-vfuncs.h
index 23b7829db..f4073312c 100644
--- a/lib/gs-plugin-vfuncs.h
+++ b/lib/gs-plugin-vfuncs.h
@@ -29,26 +29,21 @@
G_BEGIN_DECLS
/**
- * gs_plugin_initialize:
- * @plugin: a #GsPlugin
+ * gs_plugin_query_type:
*
- * Checks if the plugin should run, and if initializes it. If the plugin should
- * not be run then gs_plugin_set_enabled() should be called.
- * This is also the place to call gs_plugin_alloc_data() if private data is
- * required for the plugin.
+ * Returns the #GType for a subclass of #GsPlugin provided by this plugin
+ * module. It should not do any other computation.
*
- * NOTE: Do not do any failable actions in this function; use gs_plugin_setup()
- * instead.
- **/
-void gs_plugin_initialize (GsPlugin *plugin);
-
-/**
- * gs_plugin_destroy:
- * @plugin: a #GsPlugin
+ * The init function for that type should initialize the plugin. If the plugin
+ * should not be run then gs_plugin_set_enabled() should be called from the
+ * init function.
*
- * Called when the plugin should destroy any private data.
- **/
-void gs_plugin_destroy (GsPlugin *plugin);
+ * NOTE: Do not do any failable actions in the plugin class’ init function; use
+ * gs_plugin_setup() instead.
+ *
+ * Since: 42
+ */
+GType gs_plugin_query_type (void);
/**
* gs_plugin_adopt_app:
@@ -164,7 +159,7 @@ gboolean gs_plugin_add_alternates (GsPlugin *plugin,
* gs_app_set_progress() if they will take more than tens of milliseconds
* to complete.
*
- * This function will also not be called if gs_plugin_initialize() self-disabled.
+ * This function will also not be called if the plugin is disabled.
*
* Returns: %TRUE for success
**/
diff --git a/lib/gs-plugin.c b/lib/gs-plugin.c
index 186bf3159..ee4844f15 100644
--- a/lib/gs-plugin.c
+++ b/lib/gs-plugin.c
@@ -165,6 +165,9 @@ gs_plugin_create (const gchar *filename, GError **error)
GsPlugin *plugin = NULL;
GsPluginPrivate *priv;
g_autofree gchar *basename = NULL;
+ GModule *module = NULL;
+ GType (*query_type_function) (void) = NULL;
+ GType plugin_type;
/* get the plugin name from the basename */
basename = g_path_get_basename (filename);
@@ -179,17 +182,26 @@ gs_plugin_create (const gchar *filename, GError **error)
g_strdelimit (basename, ".", '\0');
/* create new plugin */
- plugin = gs_plugin_new ();
- priv = gs_plugin_get_instance_private (plugin);
- priv->module = g_module_open (filename, 0);
- if (priv->module == NULL) {
+ module = g_module_open (filename, 0);
+ if (module == NULL ||
+ !g_module_symbol (module, "gs_plugin_query_type", (gpointer *) &query_type_function)) {
g_set_error (error,
GS_PLUGIN_ERROR,
GS_PLUGIN_ERROR_FAILED,
"failed to open plugin %s: %s",
filename, g_module_error ());
+ if (module != NULL)
+ g_module_close (module);
return NULL;
}
+
+ plugin_type = query_type_function ();
+ g_assert (g_type_is_a (plugin_type, GS_TYPE_PLUGIN));
+
+ plugin = g_object_new (plugin_type, NULL);
+ priv = gs_plugin_get_instance_private (plugin);
+ priv->module = g_steal_pointer (&module);
+
gs_plugin_set_name (plugin, basename + 13);
return plugin;
}
@@ -344,7 +356,7 @@ gs_plugin_get_enabled (GsPlugin *plugin)
* @enabled: the enabled state
*
* Enables or disables a plugin.
- * This is normally only called from gs_plugin_initialize().
+ * This is normally only called from the init function for a #GsPlugin instance.
*
* Since: 3.22
**/
@@ -1638,10 +1650,6 @@ gs_plugin_action_to_function_name (GsPluginAction action)
return "gs_plugin_add_categories";
if (action == GS_PLUGIN_ACTION_SETUP)
return "gs_plugin_setup";
- if (action == GS_PLUGIN_ACTION_INITIALIZE)
- return "gs_plugin_initialize";
- if (action == GS_PLUGIN_ACTION_DESTROY)
- return "gs_plugin_destroy";
if (action == GS_PLUGIN_ACTION_GET_ALTERNATES)
return "gs_plugin_add_alternates";
if (action == GS_PLUGIN_ACTION_GET_LANGPACKS)
@@ -1728,10 +1736,6 @@ gs_plugin_action_to_string (GsPluginAction action)
return "get-recent";
if (action == GS_PLUGIN_ACTION_GET_UPDATES_HISTORICAL)
return "get-updates-historical";
- if (action == GS_PLUGIN_ACTION_INITIALIZE)
- return "initialize";
- if (action == GS_PLUGIN_ACTION_DESTROY)
- return "destroy";
if (action == GS_PLUGIN_ACTION_GET_ALTERNATES)
return "get-alternates";
if (action == GS_PLUGIN_ACTION_GET_LANGPACKS)
@@ -1818,10 +1822,6 @@ gs_plugin_action_from_string (const gchar *action)
return GS_PLUGIN_ACTION_GET_RECENT;
if (g_strcmp0 (action, "get-updates-historical") == 0)
return GS_PLUGIN_ACTION_GET_UPDATES_HISTORICAL;
- if (g_strcmp0 (action, "initialize") == 0)
- return GS_PLUGIN_ACTION_INITIALIZE;
- if (g_strcmp0 (action, "destroy") == 0)
- return GS_PLUGIN_ACTION_DESTROY;
if (g_strcmp0 (action, "get-alternates") == 0)
return GS_PLUGIN_ACTION_GET_ALTERNATES;
if (g_strcmp0 (action, "get-langpacks") == 0)
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
