transaction: Utilize runtime repo info for origin remoteflatpakref-utilize-flatpakrepo-metadata

Many of the fields allowed in flatpakrepo files are not allowed in
flatpakref files, e.g. Comment, Description, Homepage, etc. So there is
no straightforward way to ensure those are set correctly when the user
installs something with a flatpakref file. However in most cases the
repo specified by the RuntimeRepo key is also the repo providing the
main ref, so in those cases it makes sense to utilize all the metadata
provided in the flatpakrepo file when creating a remote to provide
updates for the app being installed. We were already doing this when the
SuggestRemoteName key was unset; this commit makes it so that we utilize
that metadata also when SuggestRemoteName is present.

5 files changed, 120 insertions, 65 deletions

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index 5cba0f5d..9145e06f 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -13858,34 +13858,6 @@ flatpak_dir_create_remote_for_ref_file (FlatpakDir         *self,
   return TRUE;
 }
 
-static gboolean
-_flatpak_uri_equal (const char *uri1,
-                    const char *uri2)
-{
-  g_autofree char *uri1_norm = NULL;
-  g_autofree char *uri2_norm = NULL;
-  gsize uri1_len = strlen (uri1);
-  gsize uri2_len = strlen (uri2);
-
-  /* URIs handled by libostree are equivalent with or without a trailing slash,
-   * but this isn't otherwise guaranteed to be the case.
-   */
-  if (g_str_has_prefix (uri1, "oci+") || g_str_has_prefix (uri2, "oci+"))
-    return g_strcmp0 (uri1, uri2) == 0;
-
-  if (g_str_has_suffix (uri1, "/"))
-    uri1_norm = g_strndup (uri1, uri1_len - 1);
-  else
-    uri1_norm = g_strdup (uri1);
-
-  if (g_str_has_suffix (uri2, "/"))
-    uri2_norm = g_strndup (uri2, uri2_len - 1);
-  else
-    uri2_norm = g_strdup (uri2);
-
-  return g_strcmp0 (uri1_norm, uri2_norm) == 0;
-}
-
 /* This tries to find a pre-configured remote for the specified uri.
  *
  *  We consider non-OCI URLs equal even if one lacks a trailing slash.
@@ -13918,7 +13890,7 @@ flatpak_dir_find_remote_by_uri (FlatpakDir *self,
                                            NULL))
             continue;
 
-          if (_flatpak_uri_equal (uri, remote_uri))
+          if (flatpak_uri_equal (uri, remote_uri))
             return g_strdup (remote);
         }
     }
diff --git a/common/flatpak-transaction.c b/common/flatpak-transaction.c
index 7f20681f..84c20e5c 100644
--- a/common/flatpak-transaction.c
+++ b/common/flatpak-transaction.c
@@ -3924,7 +3924,10 @@ remote_is_already_configured (FlatpakTransaction *self,
 }
 
 static gboolean
-handle_suggested_remote_name (FlatpakTransaction *self, GKeyFile *keyfile, GError **error)
+handle_suggested_remote_name (FlatpakTransaction *self,
+                              GKeyFile *keyfile,
+                              GKeyFile *runtime_repo_keyfile, /* nullable */
+                              GError **error)
 {
   FlatpakTransactionPrivate *priv = flatpak_transaction_get_instance_private (self);
   g_autofree char *suggested_name = NULL;
@@ -3959,7 +3962,16 @@ handle_suggested_remote_name (FlatpakTransaction *self, GKeyFile *keyfile, GErro
                  name, suggested_name, url, &res);
   if (res)
     {
-      config = flatpak_parse_repofile (suggested_name, TRUE, keyfile, &gpg_key, NULL, error);
+      g_autofree char *runtime_repo_url = NULL;
+
+      /* In case the runtime repo is the same repo, use its title, comment,
+       * description, etc. since flatpakref files don't have those fields. */
+      runtime_repo_url = g_key_file_get_string (runtime_repo_keyfile, FLATPAK_REPO_GROUP, FLATPAK_REPO_URL_KEY, NULL);
+      if (runtime_repo_url != NULL && flatpak_uri_equal (runtime_repo_url, url))
+        config = flatpak_parse_repofile (suggested_name, FALSE, runtime_repo_keyfile, &gpg_key, NULL, error);
+      else
+        config = flatpak_parse_repofile (suggested_name, TRUE, keyfile, &gpg_key, NULL, error);
+
       if (config == NULL)
         return FALSE;
 
@@ -3976,28 +3988,17 @@ handle_suggested_remote_name (FlatpakTransaction *self, GKeyFile *keyfile, GErro
 }
 
 static gboolean
-handle_runtime_repo_deps (FlatpakTransaction *self,
-                          const char         *id,
-                          const char         *dep_url,
-                          GCancellable       *cancellable,
-                          GError            **error)
+load_flatpakrepo_file (FlatpakTransaction *self,
+                       const char         *dep_url,
+                       GKeyFile          **out_keyfile,
+                       GCancellable       *cancellable,
+                       GError            **error)
 {
   FlatpakTransactionPrivate *priv = flatpak_transaction_get_instance_private (self);
   g_autoptr(GBytes) dep_data = NULL;
-  g_autofree char *runtime_url = NULL;
-  g_autofree char *new_remote = NULL;
-  g_autofree char *basename = NULL;
-  g_autoptr(SoupURI) uri = NULL;
-  g_auto(GStrv) remotes = NULL;
-  g_autoptr(GKeyFile) config = NULL;
   g_autoptr(GKeyFile) dep_keyfile = g_key_file_new ();
-  g_autoptr(GBytes) gpg_key = NULL;
-  g_autofree char *group = NULL;
   g_autoptr(GError) local_error = NULL;
   g_autoptr(SoupSession) soup_session = NULL;
-  char *t;
-  int i;
-  gboolean res;
 
   if (priv->disable_deps)
     return TRUE;
@@ -4021,6 +4022,38 @@ handle_runtime_repo_deps (FlatpakTransaction *self,
                                   0, &local_error))
     return flatpak_fail_error (error, FLATPAK_ERROR_INVALID_DATA, _("Invalid .flatpakrepo: %s"), local_error->message);
 
+  if (out_keyfile)
+    *out_keyfile = g_steal_pointer (&dep_keyfile);
+
+  return TRUE;
+}
+
+static gboolean
+handle_runtime_repo_deps (FlatpakTransaction *self,
+                          const char         *id,
+                          const char         *dep_url,
+                          GKeyFile           *dep_keyfile,
+                          GCancellable       *cancellable,
+                          GError            **error)
+{
+  FlatpakTransactionPrivate *priv = flatpak_transaction_get_instance_private (self);
+  g_autofree char *runtime_url = NULL;
+  g_autofree char *new_remote = NULL;
+  g_autofree char *basename = NULL;
+  g_autoptr(SoupURI) uri = NULL;
+  g_auto(GStrv) remotes = NULL;
+  g_autoptr(GKeyFile) config = NULL;
+  g_autoptr(GBytes) gpg_key = NULL;
+  g_autofree char *group = NULL;
+  char *t;
+  int i;
+  gboolean res;
+
+  if (priv->disable_deps)
+    return TRUE;
+
+  g_assert (dep_keyfile != NULL);
+
   uri = soup_uri_new (dep_url);
   basename = g_path_get_basename (soup_uri_get_path (uri));
   /* Strip suffix */
@@ -4077,30 +4110,23 @@ handle_runtime_repo_deps (FlatpakTransaction *self,
 
 static gboolean
 handle_runtime_repo_deps_from_keyfile (FlatpakTransaction *self,
-                                       GKeyFile           *keyfile,
+                                       GKeyFile           *flatpakref_keyfile,
+                                       const char         *runtime_repo_url,
+                                       GKeyFile           *runtime_repo_keyfile,
                                        GCancellable       *cancellable,
                                        GError            **error)
 {
   FlatpakTransactionPrivate *priv = flatpak_transaction_get_instance_private (self);
-  g_autofree char *dep_url = NULL;
   g_autofree char *name = NULL;
 
   if (priv->disable_deps)
     return TRUE;
 
-  dep_url = g_key_file_get_string (keyfile, FLATPAK_REF_GROUP,
-                                   FLATPAK_REF_RUNTIME_REPO_KEY, NULL);
-  if (dep_url == NULL)
-    {
-      g_warning ("Flatpakref file does not contain a %s", FLATPAK_REF_RUNTIME_REPO_KEY);
-      return TRUE;
-    }
-
-  name = g_key_file_get_string (keyfile, FLATPAK_REF_GROUP, FLATPAK_REF_NAME_KEY, NULL);
+  name = g_key_file_get_string (flatpakref_keyfile, FLATPAK_REF_GROUP, FLATPAK_REF_NAME_KEY, NULL);
   if (name == NULL)
     return TRUE;
 
-  return handle_runtime_repo_deps (self, name, dep_url, cancellable, error);
+  return handle_runtime_repo_deps (self, name, runtime_repo_url, runtime_repo_keyfile, cancellable, error);
 }
 
 static gboolean
@@ -4115,13 +4141,30 @@ flatpak_transaction_resolve_flatpakrefs (FlatpakTransaction *self,
     {
       GKeyFile *flatpakref = l->data;
       g_autofree char *remote = NULL;
+      g_autofree char *runtime_repo_url = NULL;
       g_autoptr(FlatpakDecomposed) ref = NULL;
+      g_autoptr(GKeyFile) runtime_repo_keyfile = NULL;
 
-      /* Handle this before the runtime deps, because they might be the same */
-      if (!handle_suggested_remote_name (self, flatpakref, error))
+      if (!priv->disable_deps)
+        {
+          runtime_repo_url = g_key_file_get_string (flatpakref, FLATPAK_REF_GROUP,
+                                                    FLATPAK_REF_RUNTIME_REPO_KEY, NULL);
+          if (runtime_repo_url == NULL)
+            g_warning ("Flatpakref file does not contain a %s", FLATPAK_REF_RUNTIME_REPO_KEY);
+          else if (!load_flatpakrepo_file (self, runtime_repo_url, &runtime_repo_keyfile, cancellable, error))
+            return FALSE;
+        }
+
+      /* Handle SuggestRemoteName before the runtime deps, because they might
+       * be the same. Pass in the RuntimeRepo keyfile so its metadata can be
+       * used in that case. */
+      if (!handle_suggested_remote_name (self, flatpakref, runtime_repo_keyfile, error))
         return FALSE;
 
-      if (!handle_runtime_repo_deps_from_keyfile (self, flatpakref, cancellable, error))
+      if (runtime_repo_keyfile != NULL &&
+          !handle_runtime_repo_deps_from_keyfile (self, flatpakref,
+                                                  runtime_repo_url, runtime_repo_keyfile,
+                                                  cancellable, error))
         return FALSE;
 
       if (!flatpak_dir_create_remote_for_ref_file (priv->dir, flatpakref, priv->default_arch,
@@ -4151,6 +4194,7 @@ handle_runtime_repo_deps_from_bundle (FlatpakTransaction *self,
   g_autofree char *dep_url = NULL;
   g_autoptr(FlatpakDecomposed) ref = NULL;
   g_autoptr(GVariant) metadata = NULL;
+  g_autoptr(GKeyFile) runtime_repo_keyfile = NULL;
   g_autofree char *id = NULL;
 
   if (priv->disable_deps)
@@ -4172,7 +4216,10 @@ handle_runtime_repo_deps_from_bundle (FlatpakTransaction *self,
 
   id = flatpak_decomposed_dup_id (ref);
 
-  return handle_runtime_repo_deps (self, id, dep_url, cancellable, error);
+  if (!load_flatpakrepo_file (self, dep_url, &runtime_repo_keyfile, cancellable, error))
+    return FALSE;
+
+  return handle_runtime_repo_deps (self, id, dep_url, runtime_repo_keyfile, cancellable, error);
 }
 
 static gboolean
diff --git a/common/flatpak-utils-private.h b/common/flatpak-utils-private.h
index 905706b8..aa013847 100644
--- a/common/flatpak-utils-private.h
+++ b/common/flatpak-utils-private.h
@@ -913,6 +913,9 @@ int flatpak_envp_cmp (const void *p1,
 
 gboolean flatpak_str_is_integer (const char *s);
 
+gboolean flatpak_uri_equal (const char *uri1,
+                            const char *uri2);
+
 #define FLATPAK_MESSAGE_ID "c7b39b1e006b464599465e105b361485"
 
 #endif /* __FLATPAK_UTILS_H__ */
diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c
index 7674e0be..8afa23fc 100644
--- a/common/flatpak-utils.c
+++ b/common/flatpak-utils.c
@@ -9042,3 +9042,31 @@ flatpak_str_is_integer (const char *s)
 
   return TRUE;
 }
+
+gboolean
+flatpak_uri_equal (const char *uri1,
+                   const char *uri2)
+{
+  g_autofree char *uri1_norm = NULL;
+  g_autofree char *uri2_norm = NULL;
+  gsize uri1_len = strlen (uri1);
+  gsize uri2_len = strlen (uri2);
+
+  /* URIs handled by libostree are equivalent with or without a trailing slash,
+   * but this isn't otherwise guaranteed to be the case.
+   */
+  if (g_str_has_prefix (uri1, "oci+") || g_str_has_prefix (uri2, "oci+"))
+    return g_strcmp0 (uri1, uri2) == 0;
+
+  if (g_str_has_suffix (uri1, "/"))
+    uri1_norm = g_strndup (uri1, uri1_len - 1);
+  else
+    uri1_norm = g_strdup (uri1);
+
+  if (g_str_has_suffix (uri2, "/"))
+    uri2_norm = g_strndup (uri2, uri2_len - 1);
+  else
+    uri2_norm = g_strdup (uri2);
+
+  return g_strcmp0 (uri1_norm, uri2_norm) == 0;
+}
diff --git a/tests/test-repo.sh b/tests/test-repo.sh
index 58867420..63d49048 100644
--- a/tests/test-repo.sh
+++ b/tests/test-repo.sh
@@ -24,7 +24,7 @@ set -euo pipefail
 skip_without_bwrap
 skip_revokefs_without_fuse
 
-echo "1..41"
+echo "1..42"
 
 #Regular repo
 setup_repo
@@ -185,7 +185,7 @@ cat << EOF > repos/flatpakref/flatpakref-repo.flatpakrepo
 [Flatpak Repo]
 Version=1
 Url=http://127.0.0.1:$(cat httpd-port)/flatpakref/
-Title=The Title
+Title=The Remote Title
 GPGKey=${FL_GPG_BASE64}
 EOF
 
@@ -198,6 +198,7 @@ cat << EOF > org.test.Hello.flatpakref
 Name=org.test.Hello
 Branch=master
 Url=http://127.0.0.1:$(cat httpd-port)/flatpakref
+SuggestRemoteName=allthegoodstuff
 GPGKey=${FL_GPG_BASE64}
 RuntimeRepo=http://127.0.0.1:$(cat httpd-port)/flatpakref/flatpakref-repo.flatpakrepo
 EOF
@@ -216,6 +217,10 @@ fi
 
 ok "install flatpakref normalizes remote URL trailing slash"
 
+assert_remote_has_config allthegoodstuff xa.title "The Remote Title"
+
+ok "install flatpakref uses RuntimeRepo metadata for remote"
+
 ${FLATPAK} ${U} uninstall -y org.test.Platform org.test.Hello
 
 if ${FLATPAK} ${U} install -y test-missing-gpg-repo org.test.Platform 2> install-error-log; then