From 6c50142ee524fc8c82e565b09f1e9223a41fb599 Mon Sep 17 00:00:00 2001 From: Simon McVittie 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 (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 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 (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 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 (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 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 (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 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 (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 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 (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 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 (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