From c81e6824219f6cc50a2d9f0c5f93e7dbddb22bc9 Mon Sep 17 00:00:00 2001 From: Ludovico de Nittis <ludovico.denittis@collabora.com> Date: Mon, 20 Jul 2020 17:50:42 +0200 Subject: [PATCH] Move locale generation to pressure-vessel-adverb pressure-vessel-with-lock has been renamed to pressure-vessel-adverb because now it has more capabilities than just taking a lock. This change should help the environments that are not able to normally run `bwrap`, like if we are in a Docker container or in a Flatpak app. Signed-off-by: Ludovico de Nittis <ludovico.denittis@collabora.com> --- build-relocatable-install.py | 2 +- src/{with-lock.c => adverb.c} | 129 ++++++++++++++++++++++++++++-- src/locking.md | 2 +- src/meson.build | 4 +- src/runtime.c | 145 ++++------------------------------ src/runtime.h | 4 +- src/utils.c | 31 ++++++++ src/utils.h | 2 + src/wrap.c | 2 +- tests/containers.py | 4 +- tests/relocatable-install.py | 2 +- 11 files changed, 183 insertions(+), 144 deletions(-) rename src/{with-lock.c => adverb.c} (71%) diff --git a/build-relocatable-install.py b/build-relocatable-install.py index d0bc4ab25..c87f02692 100755 --- a/build-relocatable-install.py +++ b/build-relocatable-install.py @@ -108,8 +108,8 @@ SCRIPTS = [ 'pressure-vessel-unruntime-test-ui', ] EXECUTABLES = [ + 'pressure-vessel-adverb', 'pressure-vessel-try-setlocale', - 'pressure-vessel-with-lock', 'pressure-vessel-wrap', ] LIBCAPSULE_TOOLS = [ diff --git a/src/with-lock.c b/src/adverb.c similarity index 71% rename from src/with-lock.c rename to src/adverb.c index 2ac1b4331..5fe9f098c 100644 --- a/src/with-lock.c +++ b/src/adverb.c @@ -1,8 +1,9 @@ -/* pressure-vessel-with-lock — run a command with a lock held. - * Basically flock(1), but using fcntl locks compatible with those used - * by bubblewrap and Flatpak. +/* pressure-vessel-adverb — run a command with an altered execution environment, + * e.g. holding a lock. + * The lock is basically flock(1), but using fcntl locks compatible with + * those used by bubblewrap and Flatpak. * - * Copyright © 2019 Collabora Ltd. + * Copyright © 2019-2020 Collabora Ltd. * * SPDX-License-Identifier: LGPL-2.1-or-later * @@ -46,6 +47,7 @@ static GPtrArray *global_locks = NULL; static gboolean opt_create = FALSE; +static gboolean opt_generate_locales = FALSE; static gboolean opt_subreaper = FALSE; static gboolean opt_verbose = FALSE; static gboolean opt_version = FALSE; @@ -126,6 +128,93 @@ opt_lock_file_cb (const char *name, return TRUE; } +static gboolean +generate_locales (gchar **locpath_out, + GError **error) +{ + g_autofree gchar *temp_dir = NULL; + g_autoptr(GDir) dir = NULL; + int wait_status; + g_autofree gchar *child_stdout = NULL; + g_autofree gchar *child_stderr = NULL; + g_autofree gchar *pvlg = NULL; + g_autofree gchar *this_path = NULL; + g_autofree gchar *this_dir = NULL; + + g_return_val_if_fail (locpath_out == NULL || *locpath_out == NULL, FALSE); + g_return_val_if_fail (error == NULL || *error == NULL, FALSE); + + temp_dir = g_dir_make_tmp ("pressure-vessel-locales-XXXXXX", error); + + if (temp_dir == NULL) + { + if (error != NULL) + glnx_prefix_error (error, + "Cannot create temporary directory for locales"); + return FALSE; + } + + this_path = g_file_read_link ("/proc/self/exe", NULL); + this_dir = g_dirname (this_path); + pvlg = g_build_filename (this_dir, "pressure-vessel-locale-gen", NULL); + + const char *locale_gen_argv[] = + { + pvlg, + "--output-dir", temp_dir, + "--verbose", + NULL + }; + + if (!g_spawn_sync (NULL, /* cwd */ + (gchar **) locale_gen_argv, + NULL, /* environ */ + G_SPAWN_DEFAULT, + NULL, NULL, /* child setup */ + &child_stdout, + &child_stderr, + &wait_status, + error)) + { + if (error != NULL) + glnx_prefix_error (error, "Cannot run pressure-vessel-locale-gen"); + return FALSE; + } + + if (child_stdout != NULL && child_stdout[0] != '\0') + g_debug ("Output:\n%s", child_stdout); + + if (child_stderr != NULL && child_stderr[0] != '\0') + g_debug ("Diagnostic output:\n%s", child_stderr); + + if (WIFEXITED (wait_status) && WEXITSTATUS (wait_status) == EX_OSFILE) + { + /* locale-gen exits 72 (EX_OSFILE) if it had to correct for + * missing locales at OS level. This is not an error. */ + g_debug ("pressure-vessel-locale-gen created missing locales"); + } + else if (!g_spawn_check_exit_status (wait_status, error)) + { + if (error != NULL) + glnx_prefix_error (error, "Unable to generate locales"); + return FALSE; + } + /* else all locales were already present (exit status 0) */ + + dir = g_dir_open (temp_dir, 0, error); + + if (dir == NULL || g_dir_read_name (dir) == NULL) + { + g_debug ("No locales have been generated"); + return TRUE; + } + + if (locpath_out != NULL) + *locpath_out = g_steal_pointer (&temp_dir); + + return TRUE; +} + static GOptionEntry options[] = { { "fd", '\0', @@ -143,6 +232,13 @@ static GOptionEntry options[] = "Don't create subsequent nonexistent lock files [default].", NULL }, + { "generate-locales", '\0', + G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, &opt_generate_locales, + "Attempt to generate any missing locales.", NULL }, + { "no-generate-locales", '\0', + G_OPTION_FLAG_REVERSE, G_OPTION_ARG_NONE, &opt_generate_locales, + "Don't generate any missing locales [default].", NULL }, + { "write", '\0', G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, &opt_write, "Lock each subsequent lock file for write access.", @@ -206,6 +302,8 @@ main (int argc, char **command_and_args; GPid child_pid; int wait_status = -1; + g_autofree gchar *locales_temp_dir = NULL; + g_auto(GStrv) my_environ = NULL; setlocale (LC_ALL, ""); pv_avoid_gvfs (); @@ -213,7 +311,7 @@ main (int argc, locks = g_ptr_array_new_with_free_func ((GDestroyNotify) pv_bwrap_lock_free); global_locks = locks; - g_set_prgname ("pressure-vessel-with-lock"); + g_set_prgname ("pressure-vessel-adverb"); g_log_set_handler (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING | G_LOG_LEVEL_MESSAGE, @@ -278,11 +376,27 @@ main (int argc, goto out; } + my_environ = g_get_environ (); + + if (opt_generate_locales) + { + /* If this fails, it is not fatal - carry on anyway */ + if (!generate_locales (&locales_temp_dir, error)) + { + g_warning ("%s", local_error->message); + g_clear_error (error); + } + else if (locales_temp_dir != NULL) + { + my_environ = g_environ_setenv (my_environ, "LOCPATH", locales_temp_dir, TRUE); + } + } + g_debug ("Launching child process..."); if (!g_spawn_async (NULL, /* working directory */ command_and_args, - NULL, /* environment */ + my_environ, G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, /* child setup + user_data */ &child_pid, @@ -343,6 +457,9 @@ main (int argc, out: global_locks = NULL; + if (locales_temp_dir != NULL) + pv_rm_rf (locales_temp_dir); + if (local_error != NULL) g_warning ("%s", local_error->message); diff --git a/src/locking.md b/src/locking.md index adbd4caa8..e00343341 100644 --- a/src/locking.md +++ b/src/locking.md @@ -7,7 +7,7 @@ process-oriented `F_SETLK` locks on older kernels. See `src/bwrap-lock.c` for full details. Do not use `flock(2)`, `flock(1)` or `lockf(3)` to interact with -pressure-vessel. `bwrap --lock-file` or `pressure-vessel-with-lock` +pressure-vessel. `bwrap --lock-file` or `pressure-vessel-adverb` can be used. Runtimes diff --git a/src/meson.build b/src/meson.build index 220d1d30b..4e3487246 100644 --- a/src/meson.build +++ b/src/meson.build @@ -60,9 +60,9 @@ pressure_vessel_utils = static_library( ) executable( - 'pressure-vessel-with-lock', + 'pressure-vessel-adverb', sources : [ - 'with-lock.c', + 'adverb.c', ], dependencies : [ gio_unix, diff --git a/src/runtime.c b/src/runtime.c index 07366f00f..00589c795 100644 --- a/src/runtime.c +++ b/src/runtime.c @@ -63,7 +63,7 @@ struct _PvRuntime FlatpakBwrap *container_access_adverb; const gchar *runtime_files; /* either source_files or mutable_sysroot */ gchar *runtime_usr; /* either runtime_files or that + "/usr" */ - const gchar *with_lock_in_container; + const gchar *adverb_in_container; PvRuntimeFlags flags; int mutable_parent_fd; @@ -880,32 +880,37 @@ pv_runtime_new (const char *source_files, NULL); } -/* If we are using a runtime, pass the lock fd to the executed process, +/* If we are using a runtime, ensure the locales to be generated, + * pass the lock fd to the executed process, * and make it act as a subreaper for the game itself. * * If we were using --unshare-pid then we could use bwrap --sync-fd * and rely on bubblewrap's init process for this, but we currently - * can't do that without breaking gameoverlayrender.so's assumptions. */ + * can't do that without breaking gameoverlayrender.so's assumptions, + * and we want -adverb for its locale functionality anyway. */ void -pv_runtime_append_lock_adverb (PvRuntime *self, - FlatpakBwrap *bwrap) +pv_runtime_append_adverbs (PvRuntime *self, + FlatpakBwrap *bwrap) { g_return_if_fail (PV_IS_RUNTIME (self)); g_return_if_fail (!pv_bwrap_was_finished (bwrap)); /* This will be true if pv_runtime_bind() was successfully called. */ - g_return_if_fail (self->with_lock_in_container != NULL); + g_return_if_fail (self->adverb_in_container != NULL); flatpak_bwrap_add_args (bwrap, - self->with_lock_in_container, + self->adverb_in_container, "--subreaper", NULL); + if (self->flags & PV_RUNTIME_FLAGS_GENERATE_LOCALES) + flatpak_bwrap_add_args (bwrap, "--generate-locales", NULL); + if (pv_bwrap_lock_is_ofd (self->runtime_lock)) { int fd = pv_bwrap_lock_steal_fd (self->runtime_lock); g_autofree gchar *fd_str = NULL; - g_debug ("Passing lock fd %d down to with-lock", fd); + g_debug ("Passing lock fd %d down to adverb", fd); flatpak_bwrap_add_fd (bwrap, fd); fd_str = g_strdup_printf ("%d", fd); flatpak_bwrap_add_args (bwrap, @@ -916,10 +921,10 @@ pv_runtime_append_lock_adverb (PvRuntime *self, { /* * We were unable to take out an open file descriptor lock, - * so it will be released on fork(). Tell the with-lock process + * so it will be released on fork(). Tell the adverb process * to take out its own compatible lock instead. There will be * a short window during which we have lost our lock but the - * with-lock process has not taken its lock - that's unavoidable + * adverb process has not taken its lock - that's unavoidable * if we want to use exec() to replace ourselves with the * container. * @@ -1142,117 +1147,6 @@ try_bind_dri (PvRuntime *self, return TRUE; } -/* - * Try to make sure we have all the locales we need, by running - * the helper from steam-runtime-tools in the container. If this - * fails, it isn't fatal - carry on anyway. - * - * @bwrap must be set up to have the same libc that we will be using - * for the container. - */ -static void -ensure_locales (PvRuntime *self, - gboolean on_host, - FlatpakBwrap *bwrap) -{ - g_autoptr(GError) local_error = NULL; - g_autoptr(FlatpakBwrap) run_locale_gen = NULL; - g_autofree gchar *locale_gen = NULL; - g_autofree gchar *locales = g_build_filename (self->overrides, "locales", NULL); - g_autofree gchar *locales_in_container = g_build_filename (self->overrides_in_container, - "locales", NULL); - g_autoptr(GDir) dir = NULL; - int exit_status; - - /* bwrap can't own any fds yet, because if it did, - * flatpak_bwrap_append_bwrap() would steal them. */ - g_return_if_fail (bwrap->fds == NULL || bwrap->fds->len == 0); - - g_mkdir (locales, 0700); - - run_locale_gen = flatpak_bwrap_new (NULL); - - if (on_host) - { - locale_gen = g_build_filename (self->tools_dir, - "pressure-vessel-locale-gen", - NULL); - /* We don't actually need to use bwrap when we're just running on - * the host system. */ - flatpak_bwrap_add_args (run_locale_gen, - locale_gen, - "--output-dir", locales, - "--verbose", - NULL); - } - else - { - locale_gen = g_build_filename ("/run/host/tools", - "pressure-vessel-locale-gen", - NULL); - - flatpak_bwrap_append_bwrap (run_locale_gen, bwrap); - flatpak_bwrap_add_args (run_locale_gen, - "--ro-bind", self->overrides, - self->overrides_in_container, - NULL); - - if (!flatpak_bwrap_bundle_args (run_locale_gen, 1, -1, FALSE, - &local_error)) - { - g_warning ("Unable to set up locale-gen command: %s", - local_error->message); - g_clear_error (&local_error); - } - - flatpak_bwrap_add_args (run_locale_gen, - "--ro-bind", self->tools_dir, "/run/host/tools", - "--bind", locales, locales_in_container, - locale_gen, - "--output-dir", locales_in_container, - "--verbose", - NULL); - } - - flatpak_bwrap_finish (run_locale_gen); - - /* locale-gen exits 72 (EX_OSFILE) if it had to correct for - * missing locales at OS level. This is not an error. */ - if (!pv_bwrap_run_sync (run_locale_gen, &exit_status, &local_error)) - { - if (exit_status == EX_OSFILE) - g_debug ("pressure-vessel-locale-gen created missing locales"); - else - g_warning ("Unable to generate locales: %s", local_error->message); - - g_clear_error (&local_error); - } - else - { - g_debug ("No locales generated"); - } - - dir = g_dir_open (locales, 0, NULL); - - /* If the directory is not empty, make it the container's LOCPATH */ - if (dir != NULL && g_dir_read_name (dir) != NULL) - { - g_autoptr(GString) locpath = NULL; - - g_debug ("%s is non-empty", locales); - - locpath = g_string_new (locales_in_container); - pv_search_path_append (locpath, g_getenv ("LOCPATH")); - flatpak_bwrap_add_args (bwrap, - "--setenv", "LOCPATH", locpath->str, - NULL); - } - else - { - g_debug ("%s is empty", locales); - } -} - typedef enum { ICD_KIND_NONEXISTENT, @@ -1597,11 +1491,6 @@ bind_runtime (PvRuntime *self, return FALSE; } - /* This needs to be done after pv_runtime_use_host_graphics_stack() - * has decided whether to bring in the host system's libc. */ - if (self->flags & PV_RUNTIME_FLAGS_GENERATE_LOCALES) - ensure_locales (self, self->any_libc_from_host, bwrap); - /* These can add data fds to @bwrap, so they must come last - after * other functions stop using @bwrap as a basis for their own bwrap * invocations with flatpak_bwrap_append_bwrap(). @@ -3083,7 +2972,7 @@ pv_runtime_bind (PvRuntime *self, if (!pv_cheap_tree_copy (pressure_vessel_prefix, dest, error)) return FALSE; - self->with_lock_in_container = "/usr/lib/pressure-vessel/from-host/bin/pressure-vessel-with-lock"; + self->adverb_in_container = "/usr/lib/pressure-vessel/from-host/bin/pressure-vessel-adverb"; } else { @@ -3092,7 +2981,7 @@ pv_runtime_bind (PvRuntime *self, pressure_vessel_prefix, "/run/pressure-vessel/pv-from-host", NULL); - self->with_lock_in_container = "/run/pressure-vessel/pv-from-host/bin/pressure-vessel-with-lock"; + self->adverb_in_container = "/run/pressure-vessel/pv-from-host/bin/pressure-vessel-adverb"; } pv_runtime_set_search_paths (self, bwrap); diff --git a/src/runtime.h b/src/runtime.h index 3c004c722..e9a7bf842 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -70,8 +70,8 @@ PvRuntime *pv_runtime_new (const char *source_files, PvRuntimeFlags flags, GError **error); -void pv_runtime_append_lock_adverb (PvRuntime *self, - FlatpakBwrap *bwrap); +void pv_runtime_append_adverbs (PvRuntime *self, + FlatpakBwrap *bwrap); gboolean pv_runtime_bind (PvRuntime *self, FlatpakBwrap *bwrap, GError **error); diff --git a/src/utils.c b/src/utils.c index f5063cdcd..b299b3c0c 100644 --- a/src/utils.c +++ b/src/utils.c @@ -408,3 +408,34 @@ pv_cheap_tree_copy (const char *source_root, g_assert (nftw_data.error == NULL); return (res == 0); } + +static gint +ftw_remove (const gchar *path, + const struct stat *sb, + gint typeflags, + struct FTW *ftwbuf) +{ + if (remove (path) < 0) + return -1; + + return 0; +} + +/* + * @directory: (type filename): The directory to remove. + * + * Recursively delete @directory within the same file system and + * without following symbolic links. + * + * Returns: %TRUE if the removal was successful + */ +gboolean +pv_rm_rf (const char *directory) +{ + g_return_val_if_fail (directory != NULL, FALSE); + + if (nftw (directory, ftw_remove, 10, FTW_DEPTH|FTW_MOUNT|FTW_PHYS) < 0) + return FALSE; + + return TRUE; +} diff --git a/src/utils.h b/src/utils.h index 11172fd54..47ea98d29 100644 --- a/src/utils.h +++ b/src/utils.h @@ -46,3 +46,5 @@ gpointer pv_hash_table_get_arbitrary_key (GHashTable *table); gboolean pv_cheap_tree_copy (const char *source_root, const char *dest_root, GError **error); + +gboolean pv_rm_rf (const char *directory); diff --git a/src/wrap.c b/src/wrap.c index b31162e11..99bfd7904 100644 --- a/src/wrap.c +++ b/src/wrap.c @@ -1369,7 +1369,7 @@ main (int argc, goto out; if (runtime != NULL) - pv_runtime_append_lock_adverb (runtime, bwrap); + pv_runtime_append_adverbs (runtime, bwrap); g_debug ("Adding wrapped command..."); flatpak_bwrap_append_args (bwrap, wrapped_command->argv); diff --git a/tests/containers.py b/tests/containers.py index b033fddf8..3a54a5104 100755 --- a/tests/containers.py +++ b/tests/containers.py @@ -162,8 +162,8 @@ class TestContainers(BaseTest): ) for exe in ( + 'pressure-vessel-adverb', 'pressure-vessel-try-setlocale', - 'pressure-vessel-with-lock', ): in_containers_dir = os.path.join( cls.containers_dir, @@ -583,7 +583,7 @@ class TestContainers(BaseTest): os.path.isfile( os.path.join( tree, 'usr', 'lib', 'pressure-vessel', 'from-host', - 'bin', 'pressure-vessel-with-lock', + 'bin', 'pressure-vessel-adverb', ) ) ) diff --git a/tests/relocatable-install.py b/tests/relocatable-install.py index 2829b243d..cc8270b85 100755 --- a/tests/relocatable-install.py +++ b/tests/relocatable-install.py @@ -78,7 +78,7 @@ class TapTest: EXES = [ - 'pressure-vessel-with-lock', + 'pressure-vessel-adverb', 'pressure-vessel-wrap', ] WRAPPED = [ -- GitLab