Blob Blame History Raw
From 6c50142ee524fc8c82e565b09f1e9223a41fb599 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 8 Feb 2022 14:04:50 +0000
Subject: [PATCH 1/7] dir: Pass cancellable through to remote EnsureRepo call

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 15c1d4f8cb8bf8266b454a9789fe589c645a5480)
---
 common/flatpak-dir.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index 965e1ca5376b..bc348b51a7e6 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -3943,7 +3943,7 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
           if (!flatpak_dir_system_helper_call_ensure_repo (self,
                                                            FLATPAK_HELPER_ENSURE_REPO_FLAGS_NONE,
                                                            installation ? installation : "",
-                                                           NULL, &local_error))
+                                                           cancellable, &local_error))
             {
               if (allow_empty)
                 return TRUE;
@@ -4087,7 +4087,7 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
           if (!flatpak_dir_system_helper_call_ensure_repo (self,
                                                            FLATPAK_HELPER_ENSURE_REPO_FLAGS_NONE,
                                                            installation ? installation : "",
-                                                           NULL, &my_error))
+                                                           cancellable, &my_error))
             {
               if (allow_empty)
                 return TRUE;
-- 
2.34.1


From 03f16b300e662afd324d3afab04a95186bf31e07 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 8 Feb 2022 14:09:05 +0000
Subject: [PATCH 2/7] dir: Factor out common code to call EnsureRepo on system
 helper

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 951b111d2627c6f62bfb5e20d86974beadcf2587)
---
 common/flatpak-dir.c | 54 +++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index bc348b51a7e6..8f7b7f011048 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -3918,6 +3918,30 @@ apply_new_flatpakrepo (const char *remote_name,
   return TRUE;
 }
 
+static gboolean
+system_helper_maybe_ensure_repo (FlatpakDir *self,
+                                 gboolean allow_empty,
+                                 GCancellable *cancellable,
+                                 GError **error)
+{
+  g_autoptr(GError) local_error = NULL;
+  const char *installation = flatpak_dir_get_id (self);
+
+  if (!flatpak_dir_system_helper_call_ensure_repo (self,
+                                                   FLATPAK_HELPER_ENSURE_REPO_FLAGS_NONE,
+                                                   installation ? installation : "",
+                                                   cancellable, &local_error))
+    {
+      if (allow_empty)
+        return TRUE;
+
+      g_propagate_error (error, g_steal_pointer (&local_error));
+      return FALSE;
+    }
+
+  return TRUE;
+}
+
 static gboolean
 _flatpak_dir_ensure_repo (FlatpakDir   *self,
                           gboolean      allow_empty,
@@ -3937,20 +3961,8 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
     {
       if (flatpak_dir_use_system_helper (self, NULL))
         {
-          g_autoptr(GError) local_error = NULL;
-          const char *installation = flatpak_dir_get_id (self);
-
-          if (!flatpak_dir_system_helper_call_ensure_repo (self,
-                                                           FLATPAK_HELPER_ENSURE_REPO_FLAGS_NONE,
-                                                           installation ? installation : "",
-                                                           cancellable, &local_error))
-            {
-              if (allow_empty)
-                return TRUE;
-
-              g_propagate_error (error, g_steal_pointer (&local_error));
-              return FALSE;
-            }
+          if (!system_helper_maybe_ensure_repo (self, allow_empty, cancellable, error))
+            return FALSE;
         }
       else
         {
@@ -4083,18 +4095,8 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
     {
       if (flatpak_dir_use_system_helper (self, NULL))
         {
-          const char *installation = flatpak_dir_get_id (self);
-          if (!flatpak_dir_system_helper_call_ensure_repo (self,
-                                                           FLATPAK_HELPER_ENSURE_REPO_FLAGS_NONE,
-                                                           installation ? installation : "",
-                                                           cancellable, &my_error))
-            {
-              if (allow_empty)
-                return TRUE;
-
-              g_propagate_error (error, g_steal_pointer (&my_error));
-              return FALSE;
-            }
+          if (!system_helper_maybe_ensure_repo (self, allow_empty, cancellable, error))
+            return FALSE;
 
           if (!ostree_repo_reload_config (repo, cancellable, error))
             return FALSE;
-- 
2.34.1


From 46d2abd78561c9a1686896292a19280d09deaf77 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 8 Feb 2022 14:12:01 +0000
Subject: [PATCH 3/7] dir: Factor out function to open the libostree repository

I'm about to add another caller for this.

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 8537b3412ad70479b16ad2c880c73ba37daf80d0)
---
 common/flatpak-dir.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index 8f7b7f011048..f44241c40290 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -3942,6 +3942,23 @@ system_helper_maybe_ensure_repo (FlatpakDir *self,
   return TRUE;
 }
 
+static gboolean
+ensure_repo_opened (OstreeRepo *repo,
+                    GCancellable *cancellable,
+                    GError **error)
+{
+  if (!ostree_repo_open (repo, cancellable, error))
+    {
+      g_autofree char *repopath = NULL;
+
+      repopath = g_file_get_path (ostree_repo_get_path (repo));
+      g_prefix_error (error, _("While opening repository %s: "), repopath);
+      return FALSE;
+    }
+
+  return TRUE;
+}
+
 static gboolean
 _flatpak_dir_ensure_repo (FlatpakDir   *self,
                           gboolean      allow_empty,
@@ -4008,14 +4025,8 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
     }
   else
     {
-      if (!ostree_repo_open (repo, cancellable, error))
-        {
-          g_autofree char *repopath = NULL;
-
-          repopath = g_file_get_path (repodir);
-          g_prefix_error (error, _("While opening repository %s: "), repopath);
-          return FALSE;
-        }
+      if (!ensure_repo_opened (repo, cancellable, error))
+        return FALSE;
     }
 
   /* In the system-helper case we're directly using the global repo, and we can't write any
-- 
2.34.1


From 4c831cad673e3271aafcb03da717515aec197636 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 8 Feb 2022 16:39:06 +0000
Subject: [PATCH 4/7] dir: Use system helper to create system repo if necessary

Previously, if /var/lib/flatpak didn't exist then we would use the
system helper to create and populate it, but if it existed and was empty,
we could only populate it if we had privileges. This led to errors from
libostree:

    Creating repo: mkdirat: Permission denied

The EnsureRepo method call is allowed by default for active local users,
so do this even if allow_empty is true: this will incorporate
/etc/flatpak/remotes.d into the repository, whether it is newly-created
or not. This makes a `flatpak search` work immediately, without having
to fetch metadata explicitly.

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 2489b915efae3a78ca7040c39161f8c304e4c7a5)
---
 common/flatpak-dir.c | 35 +++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index f44241c40290..aef3a75392dc 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -4005,22 +4005,33 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
          that still user bare-user */
       OstreeRepoMode mode = OSTREE_REPO_MODE_BARE_USER_ONLY;
 
-      if (!ostree_repo_create (repo, mode, cancellable, &my_error))
+      if (flatpak_dir_use_system_helper (self, NULL))
         {
-          flatpak_rm_rf (repodir, cancellable, NULL);
-
-          if (allow_empty)
-            return TRUE;
+          if (!system_helper_maybe_ensure_repo (self, allow_empty, cancellable, error))
+            return FALSE;
 
-          g_propagate_error (error, g_steal_pointer (&my_error));
-          return FALSE;
+          if (!ensure_repo_opened (repo, cancellable, error))
+            return FALSE;
         }
-
-      /* Create .changed file early to avoid polling non-existing file in monitor */
-      if (!flatpak_dir_mark_changed (self, &my_error))
+      else
         {
-          g_warning ("Error marking directory as changed: %s", my_error->message);
-          g_clear_error (&my_error);
+          if (!ostree_repo_create (repo, mode, cancellable, &my_error))
+            {
+              flatpak_rm_rf (repodir, cancellable, NULL);
+
+              if (allow_empty)
+                return TRUE;
+
+              g_propagate_error (error, g_steal_pointer (&my_error));
+              return FALSE;
+            }
+
+          /* Create .changed file early to avoid polling non-existing file in monitor */
+          if (!flatpak_dir_mark_changed (self, &my_error))
+            {
+              g_warning ("Error marking directory as changed: %s", my_error->message);
+              g_clear_error (&my_error);
+            }
         }
     }
   else
-- 
2.34.1


From 30b7778dc412143a8f06052298f63bdeb7bd5186 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 8 Feb 2022 16:11:31 +0000
Subject: [PATCH 5/7] dir: Avoid polkit prompts for EnsureRepo in most CLI
 commands

If we are running a CLI command in the background, then EnsureRepo
might require authorization. Silently skip it if allow_empty was true,
as it is for commands that iterate through all repositories.

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 48f40d45045071c0e073061b72a1c32fb0562c3f)
---
 common/flatpak-dir.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index aef3a75392dc..597f8906b1ce 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -3920,6 +3920,7 @@ apply_new_flatpakrepo (const char *remote_name,
 
 static gboolean
 system_helper_maybe_ensure_repo (FlatpakDir *self,
+                                 FlatpakHelperEnsureRepoFlags flags,
                                  gboolean allow_empty,
                                  GCancellable *cancellable,
                                  GError **error)
@@ -3928,7 +3929,7 @@ system_helper_maybe_ensure_repo (FlatpakDir *self,
   const char *installation = flatpak_dir_get_id (self);
 
   if (!flatpak_dir_system_helper_call_ensure_repo (self,
-                                                   FLATPAK_HELPER_ENSURE_REPO_FLAGS_NONE,
+                                                   flags,
                                                    installation ? installation : "",
                                                    cancellable, &local_error))
     {
@@ -3970,15 +3971,20 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
   g_autoptr(GError) my_error = NULL;
   g_autoptr(GFile) cache_dir = NULL;
   g_autoptr(GHashTable) flatpakrepos = NULL;
+  FlatpakHelperEnsureRepoFlags ensure_flags = FLATPAK_HELPER_ENSURE_REPO_FLAGS_NONE;
 
   if (self->repo != NULL)
     return TRUE;
 
+  /* Don't trigger polkit prompts if we are just doing this opportunistically */
+  if (allow_empty)
+    ensure_flags |= FLATPAK_HELPER_ENSURE_REPO_FLAGS_NO_INTERACTION;
+
   if (!g_file_query_exists (self->basedir, cancellable))
     {
       if (flatpak_dir_use_system_helper (self, NULL))
         {
-          if (!system_helper_maybe_ensure_repo (self, allow_empty, cancellable, error))
+          if (!system_helper_maybe_ensure_repo (self, ensure_flags, allow_empty, cancellable, error))
             return FALSE;
         }
       else
@@ -4007,7 +4013,7 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
 
       if (flatpak_dir_use_system_helper (self, NULL))
         {
-          if (!system_helper_maybe_ensure_repo (self, allow_empty, cancellable, error))
+          if (!system_helper_maybe_ensure_repo (self, ensure_flags, allow_empty, cancellable, error))
             return FALSE;
 
           if (!ensure_repo_opened (repo, cancellable, error))
@@ -4117,7 +4123,7 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
     {
       if (flatpak_dir_use_system_helper (self, NULL))
         {
-          if (!system_helper_maybe_ensure_repo (self, allow_empty, cancellable, error))
+          if (!system_helper_maybe_ensure_repo (self, ensure_flags, allow_empty, cancellable, error))
             return FALSE;
 
           if (!ostree_repo_reload_config (repo, cancellable, error))
-- 
2.34.1


From f2747123d890f836cc7f1c5c88edd67b8095f72b Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 8 Feb 2022 16:40:49 +0000
Subject: [PATCH 6/7] dir: Include repo path in error message if unable to
 create it

libostree makes heavy use of fd-based I/O, which has the disadvantage
that it is rarely obvious what path an error message is referring to.

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit d106384446ae57c78a76f4a0a0360797df62c31f)
---
 common/flatpak-dir.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/common/flatpak-dir.c b/common/flatpak-dir.c
index 597f8906b1ce..f6a43aafa612 100644
--- a/common/flatpak-dir.c
+++ b/common/flatpak-dir.c
@@ -4023,12 +4023,24 @@ _flatpak_dir_ensure_repo (FlatpakDir   *self,
         {
           if (!ostree_repo_create (repo, mode, cancellable, &my_error))
             {
+              const char *repo_path = flatpak_file_get_path_cached (repodir);
+
               flatpak_rm_rf (repodir, cancellable, NULL);
 
               if (allow_empty)
                 return TRUE;
 
-              g_propagate_error (error, g_steal_pointer (&my_error));
+              /* As of 2022, the error message from libostree is not the most helpful:
+               * Creating repo: mkdirat: Permission denied
+               * If the repository path is in the error message, assume this
+               * has been fixed. If not, add it. */
+              if (strstr (my_error->message, repo_path) != NULL)
+                g_propagate_error (error, g_steal_pointer (&my_error));
+              else
+                g_set_error (error, my_error->domain, my_error->code,
+                             "Unable to create repository at %s (%s)",
+                             repo_path, my_error->message);
+
               return FALSE;
             }
 
-- 
2.34.1


From 4772e9935f5a2b2178efa3005027b19e997ec8e5 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 8 Feb 2022 14:47:24 +0000
Subject: [PATCH 7/7] app: Make ALL_DIRS and STANDARD_DIRS imply OPTIONAL_REPO

It is already the case that when we are using ALL_DIRS, we always
combine it with OPTIONAL_REPO, meaning no need to populate empty
installations. ALL_DIRS is used for commands that iterate through all
known installations to enumerate apps/runtimes, such as `flatpak run`
and `flatpak list`; for these commands, it's reasonable to say that
if the installation does not have a libostree repository, then that's
equivalent to it having a libostree repository with no apps and no
runtimes. Make this happen automatically if forgotten.

For STANDARD_DIRS, we were inconsistent about this: `flatpak remote-list`
had OPTIONAL_REPO, but the other commands did not.
STANDARD_DIRS is used for `flatpak create-usb`, and for all the commands
that manipulate remotes.

For the commands that manipulate remotes, it seems reasonable to say
that if an installation has no libostree repository and we are unable
to create one, then that's equivalent to an installation with a
libostree repository but no remotes.

Similarly, for create-usb, an installation where we are unable to create
a libostree repository seems like it should be equivalent to an
installation whose libostree repository does not contain any of the
refs we are interested in.

Resolves: https://github.com/flatpak/flatpak/issues/4111
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 3f144d1e02fa060680eba18a9dd81969682a3170)
---
 app/flatpak-builtins.h | 6 ++++--
 app/flatpak-main.c     | 4 +++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/app/flatpak-builtins.h b/app/flatpak-builtins.h
index ec1ac4a4abdb..d925c8cd389e 100644
--- a/app/flatpak-builtins.h
+++ b/app/flatpak-builtins.h
@@ -39,10 +39,12 @@ G_BEGIN_DECLS
  * @FLATPAK_BUILTIN_FLAG_ONE_DIR: Allow a single --user/--system/--installation option
  *    and return a single dir. If no option is specified, default to --system
  * @FLATPAK_BUILTIN_FLAG_STANDARD_DIRS: Allow repeated use of --user/--system/--installation
- *    and return multiple dirs. If no option is specified return system(default)+user
+ *    and return multiple dirs. If no option is specified return system(default)+user.
+ *    Implies %FLATPAK_BUILTIN_FLAG_OPTIONAL_REPO.
  * @FLATPAK_BUILTIN_FLAG_ALL_DIRS: Allow repeated use of --user/--system/--installation
  *    and return multiple dirs. If no option is specified, return all installations,
- *    starting with system(default)+user
+ *    starting with system(default)+user.
+ *    Implies %FLATPAK_BUILTIN_FLAG_OPTIONAL_REPO.
  *
  * Flags affecting the behavior of flatpak_option_context_parse().
  *
diff --git a/app/flatpak-main.c b/app/flatpak-main.c
index e79df52eff25..4b4d65673645 100644
--- a/app/flatpak-main.c
+++ b/app/flatpak-main.c
@@ -460,7 +460,9 @@ flatpak_option_context_parse (GOptionContext     *context,
         {
           FlatpakDir *dir = g_ptr_array_index (dirs, i);
 
-          if (flags & FLATPAK_BUILTIN_FLAG_OPTIONAL_REPO)
+          if (flags & (FLATPAK_BUILTIN_FLAG_OPTIONAL_REPO |
+                       FLATPAK_BUILTIN_FLAG_ALL_DIRS |
+                       FLATPAK_BUILTIN_FLAG_STANDARD_DIRS))
             {
               if (!flatpak_dir_maybe_ensure_repo (dir, cancellable, error))
                 return FALSE;
-- 
2.34.1