From 5570e4a8c49d0a694c933d072e2ffa364d4c841a Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 2 Feb 2021 20:20:32 +0000
Subject: [PATCH] pressure-vessel: Accept runtime with or without ./files

Previously, if we were using a Flatpak-style runtime, the argument
to --runtime= had to be its ./files subdirectory. Now we accept the
top-level directory as an alternative.

This will make it more straightforward for pressure-vessel-wrap to
take responsibility for unpacking and deploying runtimes that are
shipped as a tarball, which is currently done in the SteamLinuxRuntime
shell scripts, resulting in error handling and locking being less
robust.

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 pressure-vessel/pressure-vessel-test-ui |  3 +-
 pressure-vessel/runtime.c               | 53 +++++++++++++++++--------
 pressure-vessel/runtime.h               |  2 +-
 tests/pressure-vessel/containers.py     | 13 +-----
 4 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/pressure-vessel/pressure-vessel-test-ui b/pressure-vessel/pressure-vessel-test-ui
index fa84f93b3..2e2120235 100755
--- a/pressure-vessel/pressure-vessel-test-ui
+++ b/pressure-vessel/pressure-vessel-test-ui
@@ -516,8 +516,7 @@ class Gui:
         elif id == '/':
             argv.append('--runtime=')
         else:
-            argv.append('--runtime')
-            argv.append(os.path.join(id, 'files'))
+            argv.append('--runtime=' + id)
 
         if self.host_graphics_check.get_active():
             if os.path.isdir('/run/host'):
diff --git a/pressure-vessel/runtime.c b/pressure-vessel/runtime.c
index 465b39409..a5f967763 100644
--- a/pressure-vessel/runtime.c
+++ b/pressure-vessel/runtime.c
@@ -52,7 +52,8 @@ struct _PvRuntime
   GObject parent;
 
   gchar *bubblewrap;
-  gchar *source_files;
+  gchar *deployment;
+  gchar *source_files;          /* either deployment or that + "/files" */
   gchar *tools_dir;
   PvBwrapLock *runtime_lock;
   GStrv original_environ;
@@ -93,12 +94,12 @@ struct _PvRuntimeClass
 enum {
   PROP_0,
   PROP_BUBBLEWRAP,
+  PROP_DEPLOYMENT,
   PROP_ORIGINAL_ENVIRON,
   PROP_FLAGS,
   PROP_MUTABLE_PARENT,
   PROP_PROVIDER_IN_CURRENT_NAMESPACE,
   PROP_PROVIDER_IN_CONTAINER_NAMESPACE,
-  PROP_SOURCE_FILES,
   PROP_TOOLS_DIRECTORY,
   N_PROPERTIES
 };
@@ -408,8 +409,8 @@ pv_runtime_get_property (GObject *object,
         g_value_set_string (value, self->provider_in_container_namespace);
         break;
 
-      case PROP_SOURCE_FILES:
-        g_value_set_string (value, self->source_files);
+      case PROP_DEPLOYMENT:
+        g_value_set_string (value, self->deployment);
         break;
 
       case PROP_TOOLS_DIRECTORY:
@@ -480,20 +481,20 @@ pv_runtime_set_property (GObject *object,
         self->provider_in_container_namespace = g_value_dup_string (value);
         break;
 
-      case PROP_SOURCE_FILES:
+      case PROP_DEPLOYMENT:
         /* Construct-only */
-        g_return_if_fail (self->source_files == NULL);
+        g_return_if_fail (self->deployment == NULL);
         path = g_value_get_string (value);
 
         if (path != NULL)
           {
-            self->source_files = realpath (path, NULL);
+            self->deployment = realpath (path, NULL);
 
-            if (self->source_files == NULL)
+            if (self->deployment == NULL)
               {
                 /* It doesn't exist. Keep the non-canonical path so we
                  * can warn about it later */
-                self->source_files = g_strdup (path);
+                self->deployment = g_strdup (path);
               }
           }
 
@@ -521,7 +522,7 @@ pv_runtime_constructed (GObject *object)
   g_return_if_fail (self->original_environ != NULL);
   g_return_if_fail (self->provider_in_current_namespace != NULL);
   g_return_if_fail (self->provider_in_container_namespace != NULL);
-  g_return_if_fail (self->source_files != NULL);
+  g_return_if_fail (self->deployment != NULL);
   g_return_if_fail (self->tools_dir != NULL);
 }
 
@@ -854,12 +855,29 @@ pv_runtime_initable_init (GInitable *initable,
                          self->mutable_parent);
     }
 
-  if (!g_file_test (self->source_files, G_FILE_TEST_IS_DIR))
+  if (!g_file_test (self->deployment, G_FILE_TEST_IS_DIR))
     {
       return glnx_throw (error, "\"%s\" is not a directory",
-                         self->source_files);
+                         self->deployment);
     }
 
+  /* If it contains ./files/, assume it's a Flatpak-style runtime where
+   * ./files is a merged /usr and ./metadata is an optional GKeyFile */
+  self->source_files = g_build_filename (self->deployment, "files", NULL);
+
+  if (g_file_test (self->source_files, G_FILE_TEST_IS_DIR))
+    {
+      g_debug ("Assuming %s is a Flatpak-style runtime", self->deployment);
+    }
+  else
+    {
+      g_debug ("Assuming %s is a sysroot or merged /usr", self->deployment);
+      g_clear_pointer (&self->source_files, g_free);
+      self->source_files = g_strdup (self->deployment);
+    }
+
+  g_debug ("Taking runtime files from: %s", self->source_files);
+
   if (!g_file_test (self->tools_dir, G_FILE_TEST_IS_DIR))
     {
       return glnx_throw (error, "\"%s\" is not a directory",
@@ -1034,6 +1052,7 @@ pv_runtime_finalize (GObject *object)
   g_free (self->runtime_files_on_host);
   g_free (self->runtime_usr);
   g_free (self->source_files);
+  g_free (self->deployment);
   g_free (self->tools_dir);
 
   if (self->runtime_lock != NULL)
@@ -1099,8 +1118,8 @@ pv_runtime_class_init (PvRuntimeClass *cls)
                          (G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY |
                           G_PARAM_STATIC_STRINGS));
 
-  properties[PROP_SOURCE_FILES] =
-    g_param_spec_string ("source-files", "Source files",
+  properties[PROP_DEPLOYMENT] =
+    g_param_spec_string ("deployment", "Deployment",
                          ("Path to read-only runtime files (merged-/usr "
                           "or sysroot) in current namespace"),
                          NULL,
@@ -1118,7 +1137,7 @@ pv_runtime_class_init (PvRuntimeClass *cls)
 }
 
 PvRuntime *
-pv_runtime_new (const char *source_files,
+pv_runtime_new (const char *deployment,
                 const char *mutable_parent,
                 const char *bubblewrap,
                 const char *tools_dir,
@@ -1128,7 +1147,7 @@ pv_runtime_new (const char *source_files,
                 PvRuntimeFlags flags,
                 GError **error)
 {
-  g_return_val_if_fail (source_files != NULL, NULL);
+  g_return_val_if_fail (deployment != NULL, NULL);
   g_return_val_if_fail (bubblewrap != NULL, NULL);
   g_return_val_if_fail (tools_dir != NULL, NULL);
   g_return_val_if_fail ((flags & ~(PV_RUNTIME_FLAGS_MASK)) == 0, NULL);
@@ -1139,7 +1158,7 @@ pv_runtime_new (const char *source_files,
                          "bubblewrap", bubblewrap,
                          "original-environ", original_environ,
                          "mutable-parent", mutable_parent,
-                         "source-files", source_files,
+                         "deployment", deployment,
                          "tools-directory", tools_dir,
                          "provider-in-current-namespace",
                            provider_in_current_namespace,
diff --git a/pressure-vessel/runtime.h b/pressure-vessel/runtime.h
index 4163d6c97..b0223bad6 100644
--- a/pressure-vessel/runtime.h
+++ b/pressure-vessel/runtime.h
@@ -71,7 +71,7 @@ typedef struct _PvRuntimeClass PvRuntimeClass;
 #define PV_RUNTIME_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj), PV_TYPE_RUNTIME, PvRuntimeClass)
 GType pv_runtime_get_type (void);
 
-PvRuntime *pv_runtime_new (const char *source_files,
+PvRuntime *pv_runtime_new (const char *deployment,
                            const char *mutable_parent,
                            const char *bubblewrap,
                            const char *tools_dir,
diff --git a/tests/pressure-vessel/containers.py b/tests/pressure-vessel/containers.py
index 7feec6482..9bea17f21 100755
--- a/tests/pressure-vessel/containers.py
+++ b/tests/pressure-vessel/containers.py
@@ -1062,9 +1062,6 @@ class TestContainers(BaseTest):
     def test_scout_sysroot(self) -> None:
         scout = os.path.join(self.containers_dir, 'scout_sysroot')
 
-        if os.path.isdir(os.path.join(scout, 'files')):
-            scout = os.path.join(scout, 'files')
-
         with self.subTest('only-prepare'):
             self._test_scout(
                 'scout_sysroot_prep', scout,
@@ -1080,9 +1077,6 @@ class TestContainers(BaseTest):
     def test_scout_sysroot_usrmerge(self) -> None:
         scout = os.path.join(self.containers_dir, 'scout_sysroot_usrmerge')
 
-        if os.path.isdir(os.path.join(scout, 'files')):
-            scout = os.path.join(scout, 'files')
-
         with self.subTest('only-prepare'):
             self._test_scout(
                 'scout_sysroot_prep_usrmerge', scout,
@@ -1099,7 +1093,7 @@ class TestContainers(BaseTest):
             self._test_scout('scout_sysroot_usrmerge', scout, locales=True)
 
     def test_scout_usr(self) -> None:
-        scout = os.path.join(self.containers_dir, 'scout', 'files')
+        scout = os.path.join(self.containers_dir, 'scout')
 
         with self.subTest('only-prepare'):
             self._test_scout('scout_prep', scout, copy=True, only_prepare=True)
@@ -1113,9 +1107,6 @@ class TestContainers(BaseTest):
     def test_soldier_sysroot(self) -> None:
         soldier = os.path.join(self.containers_dir, 'soldier_sysroot')
 
-        if os.path.isdir(os.path.join(soldier, 'files')):
-            soldier = os.path.join(soldier, 'files')
-
         with self.subTest('only-prepare'):
             self._test_soldier(
                 'soldier_sysroot_prep', soldier,
@@ -1131,7 +1122,7 @@ class TestContainers(BaseTest):
             self._test_soldier('soldier_sysroot', soldier, locales=True)
 
     def test_soldier_usr(self) -> None:
-        soldier = os.path.join(self.containers_dir, 'soldier', 'files')
+        soldier = os.path.join(self.containers_dir, 'soldier')
 
         with self.subTest('only-prepare'):
             self._test_soldier(
-- 
GitLab