From 08959d8351cde4173745e21cf3ca4a365764fd71 Mon Sep 17 00:00:00 2001 From: Milan Crha Date: Thu, 14 Apr 2022 14:29:30 +0200 Subject: [PATCH 1/5] flatpak: Invalidate caches when app data rescan fails Otherwise the gnome-software caches may not reflect actual state of the flatpak data, pretending everything is up-to-date even it's not. This could happen for example when enabling a flatpak remote in the Repositories dialog and closing the dialog early, which cancels the operation, maybe in the middle of downloading appstream data from the server. --- plugins/flatpak/gs-flatpak.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c index 91d6540c2..50ffa5583 100644 --- a/plugins/flatpak/gs-flatpak.c +++ b/plugins/flatpak/gs-flatpak.c @@ -447,11 +447,9 @@ gs_flatpak_create_source (GsFlatpak *self, FlatpakRemote *xremote) return g_steal_pointer (&app); } -static gboolean -gs_flatpak_claim_changed_idle_cb (gpointer user_data) +static void +gs_flatpak_internal_data_changed (GsFlatpak *self) { - GsFlatpak *self = user_data; - g_autoptr(GError) error = NULL; g_autoptr(GMutexLocker) locker = NULL; g_autoptr(GRWLockWriterLocker) writer_locker = NULL; @@ -476,7 +474,14 @@ gs_flatpak_claim_changed_idle_cb (gpointer user_data) g_clear_pointer (&writer_locker, g_rw_lock_writer_locker_free); self->requires_full_rescan = TRUE; +} +static gboolean +gs_flatpak_claim_changed_idle_cb (gpointer user_data) +{ + GsFlatpak *self = user_data; + + gs_flatpak_internal_data_changed (self); gs_plugin_cache_invalidate (self->plugin); gs_plugin_reload (self->plugin); @@ -1086,10 +1091,17 @@ gs_flatpak_rescan_app_data (GsFlatpak *self, gboolean res = gs_flatpak_refresh (self, 0, interactive, cancellable, error); if (res) self->requires_full_rescan = FALSE; + else + gs_flatpak_internal_data_changed (self); return res; } - return gs_flatpak_rescan_appstream_store (self, interactive, cancellable, error); + if (!gs_flatpak_rescan_appstream_store (self, interactive, cancellable, error)) { + gs_flatpak_internal_data_changed (self); + return FALSE; + } + + return TRUE; } gboolean -- GitLab From dc1ee700da25a18f74e3902372228ee8136f75eb Mon Sep 17 00:00:00 2001 From: Milan Crha Date: Thu, 14 Apr 2022 14:25:19 +0200 Subject: [PATCH 2/5] flatpak: Always drop internal caches for both installation instances When a refresh is requested, always drop internal flatpak caches for both FlatpakInstallation instances, to ensure they both agree on the local cache content. Otherwise the other FlatpakInstallation uses stale data and can error out on calls when being used. An example: the refresh can be called with interactive, but search with non-interactive, then the search can fail to refine metainfo data with error "No such ref", because its internal flatpak cache was not updated. --- plugins/flatpak/gs-flatpak.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c index 50ffa5583..df81ea90d 100644 --- a/plugins/flatpak/gs-flatpak.c +++ b/plugins/flatpak/gs-flatpak.c @@ -1968,8 +1968,16 @@ gs_flatpak_refresh (GsFlatpak *self, g_hash_table_remove_all (self->broken_remotes); g_mutex_unlock (&self->broken_remotes_mutex); - /* manually drop the cache */ - if (!flatpak_installation_drop_caches (gs_flatpak_get_installation (self, interactive), + /* manually drop the cache in both installation instances; + * it's needed to have them both agree on the content. */ + if (!flatpak_installation_drop_caches (gs_flatpak_get_installation (self, FALSE), + cancellable, + error)) { + gs_flatpak_error_convert (error); + return FALSE; + } + + if (!flatpak_installation_drop_caches (gs_flatpak_get_installation (self, TRUE), cancellable, error)) { gs_flatpak_error_convert (error); -- GitLab From 3961f569a2a07e830f5f90ea616c99c4b227527c Mon Sep 17 00:00:00 2001 From: Milan Crha Date: Wed, 20 Apr 2022 15:57:29 +0200 Subject: [PATCH 3/5] flatpak: Only mark caches as obsolete on source install When adding/enabling a remote, just mark caches as obsolete, thus anything later will refresh internal cache once needed. This helps to speed up the operation. --- plugins/flatpak/gs-flatpak.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c index df81ea90d..a77230ab2 100644 --- a/plugins/flatpak/gs-flatpak.c +++ b/plugins/flatpak/gs-flatpak.c @@ -1752,22 +1752,16 @@ gs_flatpak_app_install_source (GsFlatpak *self, gs_flatpak_error_convert (error); g_prefix_error (error, "cannot modify remote: "); gs_app_set_state_recover (app); + gs_flatpak_internal_data_changed (self); return FALSE; } - /* invalidate cache */ - g_rw_lock_reader_lock (&self->silo_lock); - if (self->silo != NULL) - xb_silo_invalidate (self->silo); - g_rw_lock_reader_unlock (&self->silo_lock); + /* Mark the internal cache as obsolete. */ + gs_flatpak_internal_data_changed (self); /* success */ gs_app_set_state (app, GS_APP_STATE_INSTALLED); - /* This can fail silently, it's only to update necessary caches, to provide - * up-to-date information after the successful remote enable/install. */ - gs_flatpak_refresh (self, 1, interactive, cancellable, NULL); - gs_plugin_repository_changed (self->plugin, app); return TRUE; -- GitLab From ce41b9efcec2dd6c6bf224b470c8d6c7166318f5 Mon Sep 17 00:00:00 2001 From: Milan Crha Date: Wed, 20 Apr 2022 15:59:56 +0200 Subject: [PATCH 4/5] flatpak: Properly react to cancellation of the rescan of appstream store While a load of an appstream store can fail and be ignored, the cancellation is a different kind of failure, as it can mean the appstream data not being read at all, thus rather return failure when the read is cancelled, to have the internal caches marked as invalid by the caller. Closes https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1712 --- plugins/flatpak/gs-flatpak.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c index a77230ab2..09d28add1 100644 --- a/plugins/flatpak/gs-flatpak.c +++ b/plugins/flatpak/gs-flatpak.c @@ -1042,6 +1042,10 @@ gs_flatpak_rescan_appstream_store (GsFlatpak *self, if (!gs_flatpak_add_apps_from_xremote (self, builder, xremote, interactive, cancellable, &error_local)) { g_debug ("Failed to add apps from remote ‘%s’; skipping: %s", flatpak_remote_get_name (xremote), error_local->message); + if (g_cancellable_set_error_if_cancelled (cancellable, error)) { + gs_flatpak_error_convert (error); + return FALSE; + } } } @@ -1069,7 +1073,7 @@ gs_flatpak_rescan_appstream_store (GsFlatpak *self, self->silo = xb_builder_ensure (builder, file, XB_BUILDER_COMPILE_FLAG_IGNORE_INVALID | XB_BUILDER_COMPILE_FLAG_SINGLE_LANG, - NULL, error); + cancellable, error); if (old_thread_default != NULL) g_main_context_push_thread_default (old_thread_default); -- GitLab From f5906fd9daf6a42c19b304f8188147d5d613cc63 Mon Sep 17 00:00:00 2001 From: Milan Crha Date: Fri, 22 Apr 2022 08:00:01 +0200 Subject: [PATCH 5/5] flatpak: Rescan all app data when getting app categories This was the only place where only appstream data had been updated, while it should update whole app data, in case it's needed. Caught by Phaedrus Leeds. --- plugins/flatpak/gs-flatpak.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c index 09d28add1..94f4237c4 100644 --- a/plugins/flatpak/gs-flatpak.c +++ b/plugins/flatpak/gs-flatpak.c @@ -4003,7 +4003,7 @@ gs_flatpak_add_category_apps (GsFlatpak *self, { g_autoptr(GRWLockReaderLocker) locker = NULL; - if (!gs_flatpak_rescan_appstream_store (self, interactive, cancellable, error)) + if (!gs_flatpak_rescan_app_data (self, interactive, cancellable, error)) return FALSE; locker = g_rw_lock_reader_locker_new (&self->silo_lock); -- GitLab