[gnome-software/wip/jrocha/install-updated-apps-runtimes: 2/2] flatpak: Install new runtimes when an app update needs them

[Joaquim Rocha <jrocha src gnome org>]

commit 0c1efd7519049f15c603a792d52ee577d0b26381
Author: Joaquim Rocha <jrocha endlessm com>
Date:   Thu Oct 26 22:59:54 2017 +0200

    flatpak: Install new runtimes when an app update needs them
    
    App updates can declare different runtimes than their previous version
    but the Flatpak plugin was not taking those into account. This meant
    that updating such apps would break them because they would be updated
    and left without their needed runtime installed.
    
    These changes make sure that when an app is updated, the information
    about the needed runtime is updated from the remote metadata, so it
    ends up installing the needed runtime.
    
    Unit tests for the update cases have been also changed accordingly to
    cover this one.
    
    https://phabricator.endlessm.com/T19711

 plugins/flatpak/gs-flatpak.c                       |  58 +++++++++++----------
 plugins/flatpak/gs-self-test.c                     |  28 ++++++++--
 .../tests/app-update/org.test.Chiron/metadata      |   2 +-
 plugins/flatpak/tests/build.py                     |   9 +++-
 plugins/flatpak/tests/flatpakrepos.tar.gz          | Bin 37465 -> 34594 bytes
 5 files changed, 64 insertions(+), 33 deletions(-)
---
diff --git a/plugins/flatpak/gs-flatpak.c b/plugins/flatpak/gs-flatpak.c
index 9a870283..a34eaf27 100644
--- a/plugins/flatpak/gs-flatpak.c
+++ b/plugins/flatpak/gs-flatpak.c
@@ -2390,33 +2390,30 @@ gs_flatpak_refine_runtime_for_install (GsFlatpak *self,
                                       GError **error)
 {
        GsApp *runtime;
+       gsize len;
+       const gchar *str;
+       g_autoptr(GBytes) data = NULL;
 
-       /* only required for ostree-based flatpak apps */
-       if (gs_flatpak_app_get_file_kind (app) == GS_FLATPAK_APP_FILE_KIND_BUNDLE) {
-               gsize len;
-               const gchar *str;
-               g_autoptr(GBytes) data = NULL;
-               g_autoptr(GsApp) runtime_new = NULL;
-
-               data = gs_flatpak_fetch_remote_metadata (self, app, cancellable, error);
-               if (data == NULL) {
-                       gs_utils_error_add_unique_id (error, app);
-                       return FALSE;
-               }
+       if (gs_app_get_kind (app) == AS_APP_KIND_RUNTIME)
+               return TRUE;
 
-               str = g_bytes_get_data (data, &len);
-               runtime_new = gs_flatpak_create_runtime_from_metadata (self, app,
-                                                                      str, len,
-                                                                      error);
-               if (runtime_new == NULL)
-                       return FALSE;
-               gs_app_set_update_runtime (app, runtime_new);
+       /* ensure that we get the right runtime that will need to be installed */
+       data = gs_flatpak_fetch_remote_metadata (self, app, cancellable, error);
+       if (data == NULL) {
+               gs_utils_error_add_unique_id (error, app);
+               return FALSE;
        }
 
-       /* no runtime required */
-       runtime = gs_app_get_update_runtime (app);
+       str = g_bytes_get_data (data, &len);
+       runtime = gs_flatpak_create_runtime_from_metadata (self, app,
+                                                          str, len,
+                                                          error);
+
+       /* apps need to have a runtime */
        if (runtime == NULL)
-               return TRUE;
+               return FALSE;
+
+       gs_app_set_update_runtime (app, runtime);
 
        /* the runtime could come from a different remote to the app */
        if (!gs_refine_item_metadata (self, runtime, cancellable, error)) {
@@ -2441,8 +2438,6 @@ gs_flatpak_refine_runtime_for_install (GsFlatpak *self,
                return FALSE;
        }
 
-       gs_app_set_runtime (app, runtime);
-
        return TRUE;
 }
 
@@ -2473,11 +2468,11 @@ gs_flatpak_get_list_for_install (GsFlatpak *self, GsApp *app,
        }
 
        /* add runtime */
-       runtime = gs_app_get_runtime (app);
+       if (!gs_flatpak_refine_runtime_for_install (self, app, cancellable, error))
+               return FALSE;
+       runtime = gs_app_get_update_runtime (app);
        if (runtime != NULL) {
                g_autofree gchar *ref_display = NULL;
-               if (!gs_flatpak_refine_runtime_for_install (self, app, cancellable, error))
-                       return FALSE;
                ref_display = gs_flatpak_app_get_ref_display (runtime);
                if (g_hash_table_contains (hash_installed, ref_display)) {
                        g_debug ("%s is already installed, so skipping",
@@ -2899,6 +2894,8 @@ gs_flatpak_update_app (GsFlatpak *self,
        g_autoptr(GPtrArray) xrefs_installed = NULL;
        g_autoptr(GsAppList) list = NULL;
        g_autoptr(GsFlatpakProgressHelper) phelper = NULL;
+       GsApp *runtime = NULL;
+       GsApp *update_runtime = NULL;
 
        /* install */
        gs_app_set_state (app, AS_APP_STATE_INSTALLING);
@@ -2933,6 +2930,7 @@ gs_flatpak_update_app (GsFlatpak *self,
                GsApp *app_tmp = gs_app_list_index (list, phelper->job_now);
                gs_app_set_state (app_tmp, AS_APP_STATE_INSTALLING);
        }
+
        for (phelper->job_now = 0; phelper->job_now < phelper->job_max; phelper->job_now++) {
                GsApp *app_tmp = gs_app_list_index (list, phelper->job_now);
                g_autofree gchar *ref_display = NULL;
@@ -2975,6 +2973,12 @@ gs_flatpak_update_app (GsFlatpak *self,
        gs_app_set_update_details (app, NULL);
        gs_app_set_update_urgency (app, AS_URGENCY_KIND_UNKNOWN);
 
+       /* setup the new runtime if needed */
+       runtime = gs_app_get_runtime (app);
+       update_runtime = gs_app_get_update_runtime (app);
+       if (runtime != update_runtime && gs_app_is_installed (update_runtime))
+               gs_app_set_runtime (app, update_runtime);
+
        /* set new version */
        if (!gs_flatpak_refine_appstream (self, app, error))
                return FALSE;
diff --git a/plugins/flatpak/gs-self-test.c b/plugins/flatpak/gs-self-test.c
index d2110fc3..b2288c6e 100644
--- a/plugins/flatpak/gs-self-test.c
+++ b/plugins/flatpak/gs-self-test.c
@@ -1240,6 +1240,7 @@ gs_plugins_flatpak_app_update_func (GsPluginLoader *plugin_loader)
        GsApp *app;
        GsApp *app_tmp;
        GsApp *runtime;
+       GsApp *old_runtime;
        gboolean got_progress_installing = FALSE;
        gboolean ret;
        guint notify_progress_id;
@@ -1398,6 +1399,11 @@ gs_plugins_flatpak_app_update_func (GsPluginLoader *plugin_loader)
                                  G_CALLBACK (update_app_progress_notify_cb),
                                  &progress_cnt);
 
+       /* check that the runtime is not the update's one */
+       old_runtime = gs_app_get_runtime (app);
+       g_assert (old_runtime != NULL);
+       g_assert_cmpstr (gs_app_get_branch (old_runtime), !=, "new_master");
+
        /* use a mainloop so we get the events in the default context */
        g_object_unref (plugin_job);
        plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_UPDATE,
@@ -1421,6 +1427,13 @@ gs_plugins_flatpak_app_update_func (GsPluginLoader *plugin_loader)
        g_assert_cmpint (pending_app_changed_cnt, ==, 0);
        g_assert_cmpint (updates_changed_cnt, ==, 0);
 
+       /* check that the app's runtime has changed */
+       runtime = gs_app_get_runtime (app);
+       g_assert (runtime != NULL);
+       g_assert (old_runtime != runtime);
+       g_assert_cmpstr (gs_app_get_branch (runtime), ==, "new_master");
+       g_assert (gs_app_get_state (runtime) == AS_APP_STATE_INSTALLED);
+
        /* no longer care */
        g_signal_handler_disconnect (plugin_loader, pending_apps_changed_id);
        g_signal_handler_disconnect (plugin_loader, updates_changed_id);
@@ -1436,10 +1449,19 @@ gs_plugins_flatpak_app_update_func (GsPluginLoader *plugin_loader)
        g_assert_no_error (error);
        g_assert (ret);
 
+       /* remove the old_runtime */
+       g_assert_cmpstr (gs_app_get_unique_id (old_runtime), ==, 
"user/flatpak/test/runtime/org.test.Runtime/master");
+       g_object_unref (plugin_job);
+       plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_REMOVE,
+                                        "app", old_runtime,
+                                        NULL);
+       ret = gs_plugin_loader_job_action (plugin_loader, plugin_job, NULL, &error);
+       gs_test_flush_main_context ();
+       g_assert_no_error (error);
+       g_assert (ret);
+
        /* remove the runtime */
-       runtime = gs_app_get_runtime (app);
-       g_assert (runtime != NULL);
-       g_assert_cmpstr (gs_app_get_unique_id (runtime), ==, 
"user/flatpak/test/runtime/org.test.Runtime/master");
+       g_assert_cmpstr (gs_app_get_unique_id (runtime), ==, 
"user/flatpak/test/runtime/org.test.Runtime/new_master");
        g_object_unref (plugin_job);
        plugin_job = gs_plugin_job_newv (GS_PLUGIN_ACTION_REMOVE,
                                         "app", runtime,
diff --git a/plugins/flatpak/tests/app-update/org.test.Chiron/metadata 
b/plugins/flatpak/tests/app-update/org.test.Chiron/metadata
index ce573570..1de0ab8a 100644
--- a/plugins/flatpak/tests/app-update/org.test.Chiron/metadata
+++ b/plugins/flatpak/tests/app-update/org.test.Chiron/metadata
@@ -1,4 +1,4 @@
 [Application]
 name=org.test.Chiron
-runtime=org.test.Runtime/x86_64/master
+runtime=org.test.Runtime/x86_64/new_master
 command=chiron.sh
diff --git a/plugins/flatpak/tests/build.py b/plugins/flatpak/tests/build.py
index ae9f3f11..ae0b04ac 100755
--- a/plugins/flatpak/tests/build.py
+++ b/plugins/flatpak/tests/build.py
@@ -4,7 +4,7 @@ import subprocess
 import os
 import shutil
 
-def build_flatpak(appid, srcdir, repodir, cleanrepodir=True):
+def build_flatpak(appid, srcdir, repodir, branch='master', cleanrepodir=True):
     print('Building %s from %s into %s' % (appid, srcdir, repodir))
 
     # delete repodir
@@ -51,6 +51,7 @@ def build_flatpak(appid, srcdir, repodir, cleanrepodir=True):
     argv = [flatpak_cmd, 'build-export']
     argv.append(repodir)
     argv.append(os.path.join(srcdir, appid))
+    argv.append(branch)
     argv.append('--update-appstream')
     argv.append('--timestamp=2016-09-15T01:02:03')
     if is_runtime:
@@ -80,7 +81,11 @@ build_flatpak('org.test.Chiron',
               'app-missing-runtime/repo')
 
 # app with an update
-copy_repo('app-with-runtime', 'app-update')
+build_flatpak('org.test.Runtime',
+              'app-with-runtime',
+              'app-update/repo',
+              branch='new_master',
+              cleanrepodir=True)
 build_flatpak('org.test.Chiron',
               'app-update',
               'app-update/repo',
diff --git a/plugins/flatpak/tests/flatpakrepos.tar.gz b/plugins/flatpak/tests/flatpakrepos.tar.gz
index 0a63de42..38294d93 100644
Binary files a/plugins/flatpak/tests/flatpakrepos.tar.gz and b/plugins/flatpak/tests/flatpakrepos.tar.gz 
differ