[gnome-software: 3/5] gs-external-appstream-utils: Make refresh asynchronous
- From: Milan Crha <mcrha src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gnome-software: 3/5] gs-external-appstream-utils: Make refresh asynchronous
- Date: Wed, 2 Mar 2022 17:58:30 +0000 (UTC)
commit f615620bd034f4584a28cdaee47fb625bbe2ca07
Author: Philip Withnall <pwithnall endlessos org>
Date: Mon Feb 28 12:12:19 2022 +0000
gs-external-appstream-utils: Make refresh asynchronous
This reworks the external-appstream utils to be asynchronous, allowing
all external appstream files to be downloaded in parallel, and
downloaded without blocking the calling thread.
Signed-off-by: Philip Withnall <pwithnall endlessos org>
Helps: #1472
lib/gs-external-appstream-utils.c | 414 ++++++++++++++++++++++++++++++++------
lib/gs-external-appstream-utils.h | 12 +-
lib/gs-plugin-loader.c | 44 ++--
3 files changed, 394 insertions(+), 76 deletions(-)
---
diff --git a/lib/gs-external-appstream-utils.c b/lib/gs-external-appstream-utils.c
index c3f0a83d4..3914618d2 100644
--- a/lib/gs-external-appstream-utils.c
+++ b/lib/gs-external-appstream-utils.c
@@ -8,6 +8,46 @@
* SPDX-License-Identifier: GPL-2.0+
*/
+/*
+ * SECTION:gs-external-appstream-urls
+ * @short_description: Provides support for downloading external AppStream files.
+ *
+ * This downloads the set of configured external AppStream files, and caches
+ * them locally.
+ *
+ * According to the `external-appstream-system-wide` GSetting, the files will
+ * either be downloaded to a per-user cache, or to a system-wide cache. In the
+ * case of a system-wide cache, they are downloaded to a temporary file writable
+ * by the user, and then the suexec binary `gnome-software-install-appstream` is
+ * run to copy them to the system location.
+ *
+ * All the downloads are done in the default #GMainContext for the thread which
+ * calls gs_external_appstream_refresh_async(). They are done in parallel and
+ * the async refresh function will only complete once the last download is
+ * complete.
+ *
+ * Progress data is reported via a callback, and gives the total progress of all
+ * parallel downloads. Internally this is done by updating #ProgressTuple
+ * structs as each download progresses. A periodic timeout callback sums these
+ * and reports the total progress to the caller. That means that progress
+ * reports from gs_external_appstream_refresh_async() are done at a constant
+ * frequency.
+ *
+ * To test this code locally you will probably want to change your GSettings
+ * configuration to add some external AppStream URIs:
+ * ```
+ * gsettings set org.gnome.software external-appstream-urls '["https://example.com/appdata.xml.gz";]'
+ * ```
+ *
+ * When you are done with development, run the following command to use the real
+ * external AppStream list again:
+ * ```
+ * gsettings reset org.gnome.software external-appstream-urls
+ * ```
+ *
+ * Since: 42
+ */
+
#include <glib.h>
#include <glib/gi18n.h>
#include <libsoup/soup.h>
@@ -57,15 +97,48 @@ gs_external_appstream_install (const gchar *appstream_file,
return g_subprocess_wait_check (subprocess, cancellable, error);
}
-static gboolean
-gs_external_appstream_refresh_url (GsPlugin *plugin,
- GSettings *settings,
- const gchar *url,
- SoupSession *soup_session,
- guint64 cache_age_secs,
- GCancellable *cancellable,
- GError **error)
+static void download_system_cb (GObject *source_object,
+ GAsyncResult *result,
+ gpointer user_data);
+static void download_user_cb (GObject *source_object,
+ GAsyncResult *result,
+ gpointer user_data);
+
+/* A tuple to store the last-received progress data for a single download.
+ * Each download (refresh_url_async()) has a pointer to the relevant
+ * #ProgressTuple for its download. These are stored in an array in #RefreshData
+ * and a timeout callback periodically sums them all and reports progress to the
+ * caller. */
+typedef struct {
+ gsize bytes_downloaded;
+ gsize total_download_size;
+} ProgressTuple;
+
+static void
+refresh_url_progress_cb (gsize bytes_downloaded,
+ gsize total_download_size,
+ gpointer user_data)
{
+ ProgressTuple *tuple = user_data;
+
+ tuple->bytes_downloaded = bytes_downloaded;
+ tuple->total_download_size = total_download_size;
+
+ /* The timeout callback in progress_cb() periodically sums these. No
+ * need to notify of progress from here. */
+}
+
+static void
+refresh_url_async (GSettings *settings,
+ const gchar *url,
+ SoupSession *soup_session,
+ guint64 cache_age_secs,
+ ProgressTuple *progress_tuple,
+ GCancellable *cancellable,
+ GAsyncReadyCallback callback,
+ gpointer user_data)
+{
+ g_autoptr(GTask) task = NULL;
g_autofree gchar *basename = NULL;
g_autofree gchar *basename_url = g_path_get_basename (url);
/* make sure different uris with same basenames differ */
@@ -73,17 +146,21 @@ gs_external_appstream_refresh_url (GsPlugin *plugin,
g_autofree gchar *target_file_path = NULL;
g_autoptr(GFile) target_file = NULL;
g_autoptr(GFile) tmp_file = NULL;
- g_autoptr(GsApp) app_dl = gs_app_new (gs_plugin_get_name (plugin));
+ g_autoptr(GsApp) app_dl = gs_app_new ("external-appstream");
g_autoptr(GError) local_error = NULL;
-
gboolean system_wide;
+ task = g_task_new (NULL, cancellable, callback, user_data);
+ g_task_set_source_tag (task, refresh_url_async);
+
/* Calculate the basename of the target file. */
hash = g_compute_checksum_for_string (G_CHECKSUM_SHA1, url, -1);
if (hash == NULL) {
- g_set_error (error, GS_PLUGIN_ERROR, GS_PLUGIN_ERROR_FAILED,
- "Failed to hash url %s", url);
- return FALSE;
+ g_task_return_new_error (task,
+ GS_PLUGIN_ERROR,
+ GS_PLUGIN_ERROR_FAILED,
+ "Failed to hash url %s", url);
+ return;
}
basename = g_strdup_printf ("%s-%s", hash, basename_url);
@@ -106,7 +183,8 @@ gs_external_appstream_refresh_url (GsPlugin *plugin,
g_debug ("skipping updating external appstream file %s: "
"cache age is older than file",
target_file_path);
- return TRUE;
+ g_task_return_boolean (task, TRUE);
+ return;
}
/* If downloading system wide, write the download contents into a
@@ -118,92 +196,308 @@ gs_external_appstream_refresh_url (GsPlugin *plugin,
basename,
GS_UTILS_CACHE_FLAG_WRITEABLE |
GS_UTILS_CACHE_FLAG_CREATE_DIRECTORY,
- error);
- if (tmp_file_path == NULL)
- return FALSE;
+ &local_error);
+ if (tmp_file_path == NULL) {
+ g_task_return_error (task, g_steal_pointer (&local_error));
+ return;
+ }
tmp_file = g_file_new_for_path (tmp_file_path);
} else {
tmp_file = g_object_ref (target_file);
}
+ g_task_set_task_data (task, g_object_ref (tmp_file), g_object_unref);
+
gs_app_set_summary_missing (app_dl,
/* TRANSLATORS: status text when downloading */
_("Downloading extra metadata files…"));
/* Do the download. */
- if (!gs_plugin_download_file (plugin, app_dl, url, g_file_peek_path (tmp_file),
- cancellable, error)) {
- g_set_error_literal (error,
- GS_PLUGIN_ERROR,
- GS_PLUGIN_ERROR_DOWNLOAD_FAILED,
- local_error->message);
- return FALSE;
+ gs_download_file_async (soup_session, url, tmp_file, G_PRIORITY_LOW,
+ refresh_url_progress_cb,
+ progress_tuple,
+ cancellable,
+ system_wide ? download_system_cb : download_user_cb,
+ g_steal_pointer (&task));
+}
+
+static void
+download_system_cb (GObject *source_object,
+ GAsyncResult *result,
+ gpointer user_data)
+{
+ SoupSession *soup_session = SOUP_SESSION (source_object);
+ g_autoptr(GTask) task = g_steal_pointer (&user_data);
+ GCancellable *cancellable = g_task_get_cancellable (task);
+ GFile *tmp_file = g_task_get_task_data (task);
+ g_autoptr(GError) local_error = NULL;
+
+ if (!gs_download_file_finish (soup_session, result, &local_error)) {
+ g_task_return_new_error (task,
+ GS_PLUGIN_ERROR,
+ GS_PLUGIN_ERROR_DOWNLOAD_FAILED,
+ "%s", local_error->message);
+ return;
}
g_debug ("Downloaded appstream file %s", g_file_peek_path (tmp_file));
- if (system_wide) {
- /* install file systemwide */
- if (gs_external_appstream_install (g_file_peek_path (tmp_file),
- cancellable,
- error)) {
- g_debug ("Installed appstream file %s", g_file_peek_path (tmp_file));
- return TRUE;
- } else {
- return FALSE;
- }
+ /* install file systemwide */
+ if (gs_external_appstream_install (g_file_peek_path (tmp_file),
+ cancellable,
+ &local_error)) {
+ g_debug ("Installed appstream file %s", g_file_peek_path (tmp_file));
+ g_task_return_boolean (task, TRUE);
+ } else {
+ g_task_return_error (task, g_steal_pointer (&local_error));
}
+}
- return TRUE;
+static void
+download_user_cb (GObject *source_object,
+ GAsyncResult *result,
+ gpointer user_data)
+{
+ SoupSession *soup_session = SOUP_SESSION (source_object);
+ g_autoptr(GTask) task = g_steal_pointer (&user_data);
+ GFile *tmp_file = g_task_get_task_data (task);
+ g_autoptr(GError) local_error = NULL;
+
+ if (!gs_download_file_finish (soup_session, result, &local_error)) {
+ g_task_return_new_error (task,
+ GS_PLUGIN_ERROR,
+ GS_PLUGIN_ERROR_DOWNLOAD_FAILED,
+ "%s", local_error->message);
+ return;
+ }
+
+ g_debug ("Downloaded appstream file %s", g_file_peek_path (tmp_file));
+
+ g_task_return_boolean (task, TRUE);
+}
+
+static gboolean
+refresh_url_finish (GAsyncResult *result,
+ GError **error)
+{
+ return g_task_propagate_boolean (G_TASK (result), error);
}
+static void refresh_cb (GObject *source_object,
+ GAsyncResult *result,
+ gpointer user_data);
+static gboolean progress_cb (gpointer user_data);
+static void finish_refresh_op (GTask *task,
+ GError *error);
+
+typedef struct {
+ /* Input data. */
+ guint64 cache_age_secs;
+
+ /* In-progress data. */
+ guint n_pending_ops;
+ GError *error; /* (nullable) (owned) */
+ gsize n_appstream_urls;
+ GsDownloadProgressCallback progress_callback; /* (nullable) */
+ gpointer progress_user_data; /* (closure progress_callback) */
+ ProgressTuple *progress_tuples; /* (array length=n_appstream_urls) (owned) */
+ GSource *progress_source; /* (owned) */
+} RefreshData;
+
+static void
+refresh_data_free (RefreshData *data)
+{
+ g_assert (data->n_pending_ops == 0);
+
+ /* If this was set it should have been stolen for g_task_return_error()
+ * by now. */
+ g_assert (data->error == NULL);
+
+ /* Similarly, progress reporting should have been stopped by now. */
+ g_assert (g_source_is_destroyed (data->progress_source));
+ g_source_unref (data->progress_source);
+
+ g_free (data->progress_tuples);
+
+ g_free (data);
+}
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (RefreshData, refresh_data_free)
+
/**
- * gs_external_appstream_refresh:
- * @plugin: the #GsPlugin calling this refresh operation
- * @cache_age_secs: as passed to gs_plugin_refresh()
+ * gs_external_appstream_refresh_async:
+ * @cache_age_secs: cache age, in seconds, as passed to gs_plugin_refresh()
+ * @progress_callback: (nullable): callback to call with progress information
+ * @progress_user_data: (nullable) (closure progress_callback): data to pass
+ * to @progress_callback
* @cancellable: (nullable): a #GCancellable, or %NULL
- * @error: return location for a #GError
+ * @callback: function call when the asynchronous operation is complete
+ * @user_data: data to pass to @callback
*
* Refresh any configured external appstream files, if the cache is too old.
- * This is intended to be called from a gs_plugin_refresh() function.
*
- * Returns: %TRUE on success, %FALSE otherwise
- * Since: 41
+ * Since: 42
*/
-gboolean
-gs_external_appstream_refresh (GsPlugin *plugin,
- guint64 cache_age_secs,
- GCancellable *cancellable,
- GError **error)
+void
+gs_external_appstream_refresh_async (guint64 cache_age_secs,
+ GsDownloadProgressCallback progress_callback,
+ gpointer progress_user_data,
+ GCancellable *cancellable,
+ GAsyncReadyCallback callback,
+ gpointer user_data)
{
g_autoptr(GSettings) settings = NULL;
g_auto(GStrv) appstream_urls = NULL;
+ gsize n_appstream_urls;
g_autoptr(SoupSession) soup_session = NULL;
+ g_autoptr(GTask) task = NULL;
+ RefreshData *data;
+ g_autoptr(RefreshData) data_owned = NULL;
+
+ /* Chosen to allow a few UI updates per second without updating the
+ * progress label so often it’s unreadable. */
+ const guint progress_update_period_ms = 300;
+
+ task = g_task_new (NULL, cancellable, callback, user_data);
+ g_task_set_source_tag (task, gs_external_appstream_refresh_async);
settings = g_settings_new ("org.gnome.software");
soup_session = gs_build_soup_session ();
appstream_urls = g_settings_get_strv (settings,
"external-appstream-urls");
- for (guint i = 0; appstream_urls[i] != NULL; ++i) {
- g_autoptr(GError) error_local = NULL;
+ n_appstream_urls = g_strv_length (appstream_urls);
+
+ data = data_owned = g_new0 (RefreshData, 1);
+ data->progress_callback = progress_callback;
+ data->progress_user_data = progress_user_data;
+ data->n_appstream_urls = n_appstream_urls;
+ data->progress_tuples = g_new0 (ProgressTuple, n_appstream_urls);
+ data->progress_source = g_timeout_source_new (progress_update_period_ms);
+ g_task_set_task_data (task, g_steal_pointer (&data_owned), (GDestroyNotify) refresh_data_free);
+
+ /* Set up the progress timeout. This periodically sums up the progress
+ * tuples in `data->progress_tuples` and reports them to the calling
+ * function via @progress_callback, giving an overall progress for all
+ * the parallel operations. */
+ g_source_set_callback (data->progress_source, progress_cb, g_object_ref (task), g_object_unref);
+ g_source_attach (data->progress_source, g_main_context_get_thread_default ());
+
+ /* Refresh all the URIs in parallel. */
+ data->n_pending_ops = 1;
+
+ for (gsize i = 0; i < n_appstream_urls; i++) {
if (!g_str_has_prefix (appstream_urls[i], "https")) {
g_warning ("Not considering %s as an external "
"appstream source: please use an https URL",
appstream_urls[i]);
continue;
}
- if (!gs_external_appstream_refresh_url (plugin,
- settings,
- appstream_urls[i],
- soup_session,
- cache_age_secs,
- cancellable,
- &error_local)) {
- g_warning ("Failed to update external appstream file: %s",
- error_local->message);
- }
+
+ data->n_pending_ops++;
+ refresh_url_async (settings,
+ appstream_urls[i],
+ soup_session,
+ cache_age_secs,
+ &data->progress_tuples[i],
+ cancellable,
+ refresh_cb,
+ g_object_ref (task));
}
- return TRUE;
+ finish_refresh_op (task, NULL);
+}
+
+static void
+refresh_cb (GObject *source_object,
+ GAsyncResult *result,
+ gpointer user_data)
+{
+ g_autoptr(GTask) task = g_steal_pointer (&user_data);
+ g_autoptr(GError) local_error = NULL;
+
+ refresh_url_finish (result, &local_error);
+ finish_refresh_op (task, g_steal_pointer (&local_error));
+}
+
+static gboolean
+progress_cb (gpointer user_data)
+{
+ GTask *task = G_TASK (user_data);
+ RefreshData *data = g_task_get_task_data (task);
+ gsize parallel_bytes_downloaded = 0, parallel_total_download_size = 0;
+
+ /* Sum up the progress numerator and denominator for all parallel
+ * downloads. */
+ for (gsize i = 0; i < data->n_appstream_urls; i++) {
+ const ProgressTuple *progress_tuple = &data->progress_tuples[i];
+
+ if (!g_size_checked_add (¶llel_bytes_downloaded,
+ parallel_bytes_downloaded,
+ progress_tuple->bytes_downloaded))
+ parallel_bytes_downloaded = G_MAXSIZE;
+ if (!g_size_checked_add (¶llel_total_download_size,
+ parallel_total_download_size,
+ progress_tuple->total_download_size))
+ parallel_total_download_size = G_MAXSIZE;
+ }
+
+ /* Report progress to the calling function. */
+ if (data->progress_callback != NULL)
+ data->progress_callback (parallel_bytes_downloaded,
+ parallel_total_download_size,
+ data->progress_user_data);
+
+ return G_SOURCE_CONTINUE;
+}
+
+/* @error is (transfer full) if non-%NULL */
+static void
+finish_refresh_op (GTask *task,
+ GError *error)
+{
+ RefreshData *data = g_task_get_task_data (task);
+ g_autoptr(GError) error_owned = g_steal_pointer (&error);
+
+ if (data->error == NULL && error_owned != NULL)
+ data->error = g_steal_pointer (&error_owned);
+ else if (error_owned != NULL)
+ g_debug ("Additional error while refreshing external appstream: %s", error_owned->message);
+
+ g_assert (data->n_pending_ops > 0);
+ data->n_pending_ops--;
+
+ if (data->n_pending_ops > 0)
+ return;
+
+ /* Emit one final progress update, then stop any further ones. */
+ progress_cb (task);
+ g_source_destroy (data->progress_source);
+
+ /* All complete. */
+ if (data->error != NULL)
+ g_task_return_error (task, g_steal_pointer (&data->error));
+ else
+ g_task_return_boolean (task, TRUE);
+}
+
+/**
+ * gs_external_appstream_refresh_finish:
+ * @result: a #GAsyncResult
+ * @error: return location for a #GError, or %NULL
+ *
+ * Finish an asynchronous refresh operation started with
+ * gs_external_appstream_refresh_async().
+ *
+ * Returns: %TRUE on success, %FALSE otherwise
+ * Since: 42
+ */
+gboolean
+gs_external_appstream_refresh_finish (GAsyncResult *result,
+ GError **error)
+{
+ g_return_val_if_fail (g_task_is_valid (result, NULL), FALSE);
+ g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
+
+ return g_task_propagate_boolean (G_TASK (result), error);
}
diff --git a/lib/gs-external-appstream-utils.h b/lib/gs-external-appstream-utils.h
index ecc6d4937..2759ca61e 100644
--- a/lib/gs-external-appstream-utils.h
+++ b/lib/gs-external-appstream-utils.h
@@ -19,7 +19,11 @@
const gchar *gs_external_appstream_utils_get_system_dir (void);
gchar *gs_external_appstream_utils_get_file_cache_path (const gchar *file_name);
-gboolean gs_external_appstream_refresh (GsPlugin *plugin,
- guint64 cache_age_secs,
- GCancellable *cancellable,
- GError **error);
+void gs_external_appstream_refresh_async (guint64 cache_age_secs,
+ GsDownloadProgressCallback progress_callback,
+ gpointer progress_user_data,
+ GCancellable *cancellable,
+ GAsyncReadyCallback callback,
+ gpointer user_data);
+gboolean gs_external_appstream_refresh_finish (GAsyncResult *result,
+ GError **error);
diff --git a/lib/gs-plugin-loader.c b/lib/gs-plugin-loader.c
index 4081a6910..abccfc2a1 100644
--- a/lib/gs-plugin-loader.c
+++ b/lib/gs-plugin-loader.c
@@ -873,14 +873,14 @@ gs_plugin_loader_job_sorted_truncation (GsPluginLoaderHelper *helper)
typedef struct {
GsPluginLoader *plugin_loader; /* (not nullable) (unowned) */
GsApp *app; /* (not nullable) (unowned) */
-} OdrsRefreshProgressData;
+} RefreshProgressData;
static void
-odrs_refresh_progress_cb (gsize bytes_downloaded,
- gsize total_download_size,
- gpointer user_data)
+refresh_progress_cb (gsize bytes_downloaded,
+ gsize total_download_size,
+ gpointer user_data)
{
- OdrsRefreshProgressData *data = user_data;
+ RefreshProgressData *data = user_data;
guint percentage;
if (total_download_size > 0)
@@ -913,11 +913,31 @@ gs_plugin_loader_run_results (GsPluginLoaderHelper *helper,
/* Download updated external appstream before anything else */
#ifdef ENABLE_EXTERNAL_APPSTREAM
if (action == GS_PLUGIN_ACTION_REFRESH) {
- /* FIXME: Using plugin_loader->plugins->pdata[0] is a hack; see
- * comment below for details. */
- if (!gs_external_appstream_refresh (plugin_loader->plugins->pdata[0],
- gs_plugin_job_get_age (helper->plugin_job),
- cancellable, error))
+ g_autoptr(GsApp) app_dl = NULL;
+ RefreshProgressData progress_data;
+ g_autoptr(GAsyncResult) external_appstream_result = NULL;
+
+ app_dl = gs_app_new ("external-appstream");
+ gs_app_set_summary_missing (app_dl,
+ /* TRANSLATORS: status text when downloading */
+ _("Downloading extra metadata files…"));
+
+ progress_data.plugin_loader = plugin_loader;
+ progress_data.app = app_dl;
+
+ gs_external_appstream_refresh_async (gs_plugin_job_get_age (helper->plugin_job),
+ refresh_progress_cb,
+ &progress_data,
+ cancellable,
+ async_result_cb,
+ &external_appstream_result);
+
+ /* FIXME: Make this sync until the enclosing function is
+ * refactored to be async. */
+ while (external_appstream_result == NULL)
+ g_main_context_iteration (g_main_context_get_thread_default (), TRUE);
+
+ if (!gs_external_appstream_refresh_finish (external_appstream_result, error))
return FALSE;
}
#endif
@@ -940,7 +960,7 @@ gs_plugin_loader_run_results (GsPluginLoaderHelper *helper,
if (action == GS_PLUGIN_ACTION_REFRESH &&
plugin_loader->odrs_provider != NULL) {
g_autoptr(GsApp) app_dl = NULL;
- OdrsRefreshProgressData progress_data;
+ RefreshProgressData progress_data;
g_autoptr(GAsyncResult) odrs_result = NULL;
g_autoptr(GError) local_error = NULL;
@@ -954,7 +974,7 @@ gs_plugin_loader_run_results (GsPluginLoaderHelper *helper,
gs_odrs_provider_refresh_ratings_async (plugin_loader->odrs_provider,
gs_plugin_job_get_age (helper->plugin_job),
- odrs_refresh_progress_cb,
+ refresh_progress_cb,
&progress_data,
cancellable,
async_result_cb,
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
