From e44edef7f1aa93e4d6ceb24077a2f7d7e0c0d262 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 17 Aug 2021 16:09:09 +0100
Subject: [PATCH] wrap: Always try to use --copy-runtime unless specifically
 told not to

We are using --copy-runtime mode in production, it's the only thing
that can work in Flatpak, and it's currently the only thing that will
work on Ubuntu 20.04 and older Debian due to their use of
/sbin/ldconfig.real (see !358), so we should try harder to make use of
it. The only time we should not be copying the runtime is when we don't
have a variable directory to copy it into, or when explicitly told not to
for testing purposes.

Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 pressure-vessel/wrap.c | 70 ++++++++++++++++++++++++++++++------------
 1 file changed, 51 insertions(+), 19 deletions(-)

diff --git a/pressure-vessel/wrap.c b/pressure-vessel/wrap.c
index 7d0fbce2d..1baeee625 100644
--- a/pressure-vessel/wrap.c
+++ b/pressure-vessel/wrap.c
@@ -678,7 +678,7 @@ typedef enum
 } Tristate;
 
 static gboolean opt_batch = FALSE;
-static gboolean opt_copy_runtime = FALSE;
+static Tristate opt_copy_runtime = TRISTATE_MAYBE;
 static char **opt_env_if_host = NULL;
 static char *opt_fake_home = NULL;
 static char **opt_filesystems = NULL;
@@ -715,6 +715,22 @@ static gboolean opt_test = FALSE;
 static PvTerminal opt_terminal = PV_TERMINAL_AUTO;
 static char *opt_write_final_argv = NULL;
 
+static gboolean
+opt_copy_runtime_cb (const gchar *option_name,
+                     const gchar *value,
+                     gpointer data,
+                     GError **error)
+{
+  if (g_strcmp0 (option_name, "--copy-runtime") == 0)
+    opt_copy_runtime = TRISTATE_YES;
+  else if (g_strcmp0 (option_name, "--no-copy-runtime") == 0)
+    opt_copy_runtime = TRISTATE_NO;
+  else
+    g_return_val_if_reached (FALSE);
+
+  return TRUE;
+}
+
 typedef enum
 {
   PRELOAD_VARIABLE_INDEX_LD_AUDIT,
@@ -805,14 +821,14 @@ opt_copy_runtime_into_cb (const gchar *option_name,
     {
       g_warning ("%s is deprecated, disable with --no-copy-runtime instead",
                  option_name);
-      opt_copy_runtime = FALSE;
+      opt_copy_runtime = TRISTATE_NO;
     }
   else
     {
       g_warning ("%s is deprecated, use --copy-runtime and "
                  "--variable-dir instead",
                  option_name);
-      opt_copy_runtime = TRUE;
+      opt_copy_runtime = TRISTATE_YES;
       g_free (opt_variable_dir);
       opt_variable_dir = g_strdup (value);
     }
@@ -1035,14 +1051,13 @@ static GOptionEntry options[] =
     "Disable all interactivity and redirection: ignore --shell*, "
     "--terminal, --xterm, --tty. [Default: if $PRESSURE_VESSEL_BATCH]", NULL },
   { "copy-runtime", '\0',
-    G_OPTION_FLAG_NONE, G_OPTION_ARG_NONE, &opt_copy_runtime,
+    G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK, opt_copy_runtime_cb,
     "If a --runtime is used, copy it into --variable-dir and edit the "
-    "copy in-place.",
+    "copy in-place. [Default, if possible]",
     NULL },
   { "no-copy-runtime", '\0',
-    G_OPTION_FLAG_REVERSE, G_OPTION_ARG_NONE, &opt_copy_runtime,
-    "Don't behave as described for --copy-runtime. "
-    "[Default unless $PRESSURE_VESSEL_COPY_RUNTIME is 1 or running in Flatpak]",
+    G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK, opt_copy_runtime_cb,
+    "Don't behave as described for --copy-runtime.",
     NULL },
   { "copy-runtime-into", '\0',
     G_OPTION_FLAG_FILENAME|G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_CALLBACK,
@@ -1376,14 +1391,16 @@ main (int argc,
 
   /* Set defaults */
   opt_batch = pv_boolean_environment ("PRESSURE_VESSEL_BATCH", FALSE);
-  opt_copy_runtime = is_flatpak_env;
-  /* Process COPY_RUNTIME_INFO first so that COPY_RUNTIME and VARIABLE_DIR
-   * can override it */
-  opt_copy_runtime_into_cb ("$PRESSURE_VESSEL_COPY_RUNTIME_INTO",
-                            g_getenv ("PRESSURE_VESSEL_COPY_RUNTIME_INTO"),
-                            NULL, NULL);
-  opt_copy_runtime = pv_boolean_environment ("PRESSURE_VESSEL_COPY_RUNTIME",
-                                             opt_copy_runtime);
+  opt_copy_runtime = tristate_environment ("PRESSURE_VESSEL_COPY_RUNTIME");
+
+  if (opt_copy_runtime == TRISTATE_MAYBE)
+    opt_copy_runtime_into_cb ("$PRESSURE_VESSEL_COPY_RUNTIME_INTO",
+                              g_getenv ("PRESSURE_VESSEL_COPY_RUNTIME_INTO"),
+                              NULL, NULL);
+
+  if (opt_copy_runtime == TRISTATE_MAYBE && is_flatpak_env)
+    opt_copy_runtime = TRISTATE_YES;
+
   opt_runtime_id = g_strdup (g_getenv ("PRESSURE_VESSEL_RUNTIME_ID"));
 
     {
@@ -1674,12 +1691,27 @@ main (int argc,
         }
     }
 
-  if (opt_copy_runtime && opt_variable_dir == NULL)
+
+  if (is_flatpak_env && opt_variable_dir == NULL)
+    {
+      usage_error ("Running under Flatpak requires --variable-dir");
+      goto out;
+    }
+
+  if (opt_copy_runtime == TRISTATE_YES && opt_variable_dir == NULL)
     {
       usage_error ("--copy-runtime requires --variable-dir");
       goto out;
     }
 
+  if (opt_copy_runtime == TRISTATE_MAYBE)
+    {
+      if (opt_variable_dir != NULL)
+        opt_copy_runtime = TRISTATE_YES;
+      else
+        opt_copy_runtime = TRISTATE_NO;
+    }
+
   /* Finished parsing arguments, so any subsequent failures will make
    * us exit 1. */
   ret = 1;
@@ -1907,7 +1939,7 @@ main (int argc,
       if (opt_import_vulkan_layers)
         flags |= PV_RUNTIME_FLAGS_IMPORT_VULKAN_LAYERS;
 
-      if (opt_copy_runtime)
+      if (opt_copy_runtime != TRISTATE_NO)
         flags |= PV_RUNTIME_FLAGS_COPY_RUNTIME;
 
       if (opt_single_thread)
@@ -1939,7 +1971,7 @@ main (int argc,
 
       g_debug ("Configuring runtime %s...", runtime_path);
 
-      if (is_flatpak_env && !opt_copy_runtime)
+      if (is_flatpak_env && opt_copy_runtime == TRISTATE_NO)
         {
           glnx_throw (error,
                       "Cannot set up a runtime inside Flatpak without "
-- 
GitLab