From de82a412b3dd1eb8d5fd9e9d7542d82fe8907ec0 Mon Sep 17 00:00:00 2001 From: Simon McVittie <smcv@collabora.com> Date: Fri, 31 Jan 2025 14:23:35 +0000 Subject: [PATCH] pv-wrap: Have test coverage for the single-architecture case Until now, the case where only one architecture is supported has only been tested if pressure-vessel was built for a non-x86 architecture such as ARM, which is rarely (if ever) done. We can get coverage for this case by making pv-wrap behave as if only the first architecture was supported, as a runtime rather than compile-time decision: this means that an ordinary x86_64 build can exercise both the x86 and non-x86 code paths. This is important because steamrt/tasks#595 will add new code that differs according to whether there is only a single architecture, or more than one. In the test, this means we need to be able to mark which of the expected paths are expected on i386 only, and which ones are expected in general. Instead of the verbosity that would result from turning all of our test data into structs, I've done this with an ad-hoc mini-language: the ones that are only expected on i386 are prefixed with `i386:`. We also might conceivably want to make use of something similar to PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE in production in future, if we start providing legacy-free containers that only support running x86_64 games. Signed-off-by: Simon McVittie <smcv@collabora.com> --- pressure-vessel/wrap-setup.c | 9 +- pressure-vessel/wrap-setup.h | 3 + tests/pressure-vessel/wrap-setup.c | 157 +++++++++++++++++++++-------- 3 files changed, 125 insertions(+), 44 deletions(-) diff --git a/pressure-vessel/wrap-setup.c b/pressure-vessel/wrap-setup.c index a185fd1ac..31bf7b83c 100644 --- a/pressure-vessel/wrap-setup.c +++ b/pressure-vessel/wrap-setup.c @@ -710,9 +710,16 @@ append_preload_per_architecture (GPtrArray *argv, FlatpakExports *exports) { g_autoptr(SrtSystemInfo) system_info = srt_system_info_new (NULL); + gsize n_supported_architectures = PV_N_SUPPORTED_ARCHITECTURES; gsize i; - for (i = 0; i < PV_N_SUPPORTED_ARCHITECTURES; i++) + if (flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE) + { + G_STATIC_ASSERT (PV_N_SUPPORTED_ARCHITECTURES >= 1); + n_supported_architectures = 1; + } + + for (i = 0; i < n_supported_architectures; i++) { g_autoptr(GString) mock_path = NULL; g_autoptr(SrtLibrary) details = NULL; diff --git a/pressure-vessel/wrap-setup.h b/pressure-vessel/wrap-setup.h index ceb71fc69..f0308e8fb 100644 --- a/pressure-vessel/wrap-setup.h +++ b/pressure-vessel/wrap-setup.h @@ -68,6 +68,8 @@ void pv_wrap_move_into_scope (const char *steam_app_id); * @PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY: Disable the Steam Overlay * @PV_APPEND_PRELOAD_FLAGS_IN_UNIT_TESTS: Normalize $LIB and $PLATFORM, * for unit testing + * @PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE: Behave as though there is + * only one architecture supported, for test coverage * @PV_APPEND_PRELOAD_FLAGS_NONE: None of the above * * Flags affecting the behaviour of pv_wrap_append_preload(). @@ -77,6 +79,7 @@ typedef enum PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX = (1 << 0), PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY = (1 << 1), PV_APPEND_PRELOAD_FLAGS_IN_UNIT_TESTS = (1 << 2), + PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE = (1 << 3), PV_APPEND_PRELOAD_FLAGS_NONE = 0 } PvAppendPreloadFlags; diff --git a/tests/pressure-vessel/wrap-setup.c b/tests/pressure-vessel/wrap-setup.c index d22205181..885cea99b 100644 --- a/tests/pressure-vessel/wrap-setup.c +++ b/tests/pressure-vessel/wrap-setup.c @@ -94,6 +94,7 @@ typedef struct typedef struct { PvRuntimeFlags runtime_flags; + PvAppendPreloadFlags preload_flags; } Config; static const Config default_config = {}; @@ -106,6 +107,10 @@ static const Config interpreter_root_config = .runtime_flags = (PV_RUNTIME_FLAGS_COPY_RUNTIME | PV_RUNTIME_FLAGS_INTERPRETER_ROOT), }; +static const Config one_arch_config = +{ + .preload_flags = PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE, +}; static int open_or_die (const char *path, @@ -250,6 +255,7 @@ static void setup_ld_preload (Fixture *f, gconstpointer context) { + const Config *config = context; static const char * const touch[] = { "app/lib/libpreloadA.so", @@ -274,9 +280,14 @@ setup_ld_preload (Fixture *f, setup (f, context); fixture_populate_dir (f, f->mock_host->fd, touch, G_N_ELEMENTS (touch)); + + if (!(config->preload_flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE)) + { #if defined(__i386__) || defined(__x86_64__) - fixture_populate_dir (f, f->mock_host->fd, touch_i386, G_N_ELEMENTS (touch_i386)); + fixture_populate_dir (f, f->mock_host->fd, touch_i386, G_N_ELEMENTS (touch_i386)); #endif + } + f->env = g_environ_setenv (f->env, "STEAM_COMPAT_CLIENT_INSTALL_PATH", "/steam", TRUE); } @@ -1630,22 +1641,16 @@ static const char * const expected_preload_paths[] = { "/app/lib/libpreloadA.so", "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so:abi=" PRIMARY_ABI, -#if defined(__i386__) || defined(__x86_64__) - "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so:abi=" SRT_ABI_I386, -#endif + "i386:/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so:abi=" SRT_ABI_I386, "/opt/" PRIMARY_LIB "/libpreloadL.so:abi=" PRIMARY_ABI, -#if defined(__i386__) || defined(__x86_64__) - "/opt/" MOCK_LIB_32 "/libpreloadL.so:abi=" SRT_ABI_I386, -#endif + "i386:/opt/" MOCK_LIB_32 "/libpreloadL.so:abi=" SRT_ABI_I386, "/lib/libpreload-rootfs.so", "/usr/lib/libpreloadU.so", "/home/me/libpreloadH.so", "/steam/lib/gameoverlayrenderer.so", "/overlay/libs/${ORIGIN}/../lib/libpreloadO.so", "/future/libs-$FUTURE/libpreloadF.so", -#if defined(__i386__) || defined(__x86_64__) - "/in-root-plat-i686-only-32-bit.so:abi=" SRT_ABI_I386, -#endif + "i386:/in-root-plat-i686-only-32-bit.so:abi=" SRT_ABI_I386, "/in-root-${FUTURE}.so", "./${RELATIVE}.so", "./relative.so", @@ -1653,41 +1658,77 @@ static const char * const expected_preload_paths[] = * libfakeroot is not in the runtime or graphics stack provider, only * the current namespace */ "/path/to/" PRIMARY_LIB "/libfakeroot.so:abi=" PRIMARY_ABI, -#if defined(__i386__) || defined(__x86_64__) - "/path/to/" MOCK_LIB_32 "/libfakeroot.so:abi=" SRT_ABI_I386, -#endif + "i386:/path/to/" MOCK_LIB_32 "/libfakeroot.so:abi=" SRT_ABI_I386, /* Our mock implementation of pv_runtime_has_library() behaves as though * libpthread.so.0 *is* in the runtime, as we would expect */ "libpthread.so.0", }; +static GPtrArray * +filter_expected_paths (const Config *config) +{ + g_autoptr(GPtrArray) filtered = g_ptr_array_new_with_free_func (NULL); + gsize i; + + /* Some of the expected paths are only expected to appear on i386. + * Filter the list accordingly. */ + for (i = 0; i < G_N_ELEMENTS (expected_preload_paths); i++) + { + const char *path = expected_preload_paths[i]; + + if (g_str_has_prefix (path, "i386:")) + { +#if defined(__i386__) || defined(__x86_64__) + if (!(config->preload_flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE)) + g_ptr_array_add (filtered, (char *) (path + strlen ("i386:"))); +#endif + } + else + { + g_ptr_array_add (filtered, (char *) path); + } + } + + return g_steal_pointer (&filtered); +} + static void test_remap_ld_preload (Fixture *f, gconstpointer context) { + const Config *config = context; g_autoptr(FlatpakExports) exports = fixture_create_exports (f); g_autoptr(GPtrArray) argv = g_ptr_array_new_with_free_func (g_free); + g_autoptr(GPtrArray) filtered = filter_expected_paths (config); g_autoptr(PvRuntime) runtime = fixture_create_runtime (f, PV_RUNTIME_FLAGS_NONE); gsize i; + gboolean expect_i386 = FALSE; + +#if defined(__i386__) || defined(__x86_64__) + if (!(config->preload_flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE)) + expect_i386 = TRUE; +#endif - populate_ld_preload (f, argv, PV_APPEND_PRELOAD_FLAGS_NONE, runtime, exports); + populate_ld_preload (f, argv, config->preload_flags, runtime, exports); - g_assert_cmpuint (argv->len, ==, G_N_ELEMENTS (expected_preload_paths)); + g_assert_cmpuint (argv->len, ==, filtered->len); for (i = 0; i < argv->len; i++) { + char *expected = g_ptr_array_index (filtered, i); char *argument = g_ptr_array_index (argv, i); + g_assert_true (g_str_has_prefix (argument, "--ld-preload=")); argument += strlen("--ld-preload="); - if (g_str_has_prefix (expected_preload_paths[i], "/lib/") - || g_str_has_prefix (expected_preload_paths[i], "/usr/lib/")) + if (g_str_has_prefix (expected, "/lib/") + || g_str_has_prefix (expected, "/usr/lib/")) { g_assert_true (g_str_has_prefix (argument, "/run/host/")); argument += strlen("/run/host"); } - g_assert_cmpstr (argument, ==, expected_preload_paths[i]); + g_assert_cmpstr (argument, ==, expected); } /* FlatpakExports never exports /app */ @@ -1710,10 +1751,8 @@ test_remap_ld_preload (Fixture *f, g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" PRIMARY_LIB "/libpreloadL.so")); g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so")); -#if defined(__i386__) || defined(__x86_64__) - g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so")); -#endif + g_assert_cmpint (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so"), ==, expect_i386); + g_assert_cmpint (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so"), ==, expect_i386); /* FlatpakExports never exports /lib as /lib */ g_assert_false (flatpak_exports_path_is_visible (exports, "/lib")); @@ -1747,30 +1786,36 @@ static void test_remap_ld_preload_flatpak (Fixture *f, gconstpointer context) { + const Config *config = context; g_autoptr(GPtrArray) argv = g_ptr_array_new_with_free_func (g_free); + g_autoptr(GPtrArray) filtered = filter_expected_paths (config); g_autoptr(PvRuntime) runtime = fixture_create_runtime (f, PV_RUNTIME_FLAGS_FLATPAK_SUBSANDBOX); gsize i; - populate_ld_preload (f, argv, PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX, + populate_ld_preload (f, argv, + (config->preload_flags + | PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX), runtime, NULL); - g_assert_cmpuint (argv->len, ==, G_N_ELEMENTS (expected_preload_paths)); + g_assert_cmpuint (argv->len, ==, filtered->len); for (i = 0; i < argv->len; i++) { + char *expected = g_ptr_array_index (filtered, i); char *argument = g_ptr_array_index (argv, i); + g_assert_true (g_str_has_prefix (argument, "--ld-preload=")); argument += strlen("--ld-preload="); - if (g_str_has_prefix (expected_preload_paths[i], "/app/") - || g_str_has_prefix (expected_preload_paths[i], "/lib/") - || g_str_has_prefix (expected_preload_paths[i], "/usr/lib/")) + if (g_str_has_prefix (expected, "/app/") + || g_str_has_prefix (expected, "/lib/") + || g_str_has_prefix (expected, "/usr/lib/")) { g_assert_true (g_str_has_prefix (argument, "/run/parent/")); argument += strlen("/run/parent"); } - g_assert_cmpstr (argument, ==, expected_preload_paths[i]); + g_assert_cmpstr (argument, ==, expected); } } @@ -1783,31 +1828,44 @@ static void test_remap_ld_preload_no_runtime (Fixture *f, gconstpointer context) { + const Config *config = context; g_autoptr(GPtrArray) argv = g_ptr_array_new_with_free_func (g_free); + g_autoptr(GPtrArray) filtered = filter_expected_paths (config); g_autoptr(FlatpakExports) exports = fixture_create_exports (f); gsize i, j; + gboolean expect_i386 = FALSE; - populate_ld_preload (f, argv, PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY, +#if defined(__i386__) || defined(__x86_64__) + if (!(config->preload_flags & PV_APPEND_PRELOAD_FLAGS_ONE_ARCHITECTURE)) + expect_i386 = TRUE; +#endif + + populate_ld_preload (f, argv, + (config->preload_flags + | PV_APPEND_PRELOAD_FLAGS_REMOVE_GAME_OVERLAY), NULL, exports); - g_assert_cmpuint (argv->len, ==, G_N_ELEMENTS (expected_preload_paths) - 1); + g_assert_cmpuint (argv->len, ==, filtered->len - 1); for (i = 0, j = 0; i < argv->len; i++, j++) { + char *expected = g_ptr_array_index (filtered, j); char *argument = g_ptr_array_index (argv, i); + g_assert_true (g_str_has_prefix (argument, "--ld-preload=")); argument += strlen("--ld-preload="); /* /steam/lib/gameoverlayrenderer.so is missing because we used the * REMOVE_GAME_OVERLAY flag */ - if (g_str_has_suffix (expected_preload_paths[j], "/gameoverlayrenderer.so")) + if (g_str_has_suffix (expected, "/gameoverlayrenderer.so")) { /* We expect to skip only one element */ g_assert_cmpint (i, ==, j); j++; + expected = g_ptr_array_index (filtered, j); } - g_assert_cmpstr (argument, ==, expected_preload_paths[j]); + g_assert_cmpstr (argument, ==, expected); } /* FlatpakExports never exports /app */ @@ -1830,10 +1888,8 @@ test_remap_ld_preload_no_runtime (Fixture *f, g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" PRIMARY_LIB "/libpreloadL.so")); g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" PRIMARY_PLATFORM "/libpreloadP.so")); -#if defined(__i386__) || defined(__x86_64__) - g_assert_true (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so")); - g_assert_true (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so")); -#endif + g_assert_cmpint (flatpak_exports_path_is_visible (exports, "/opt/" MOCK_LIB_32 "/libpreloadL.so"), ==, expect_i386); + g_assert_cmpint (flatpak_exports_path_is_visible (exports, "/platform/plat-" MOCK_PLATFORM_32 "/libpreloadP.so"), ==, expect_i386); /* FlatpakExports never exports /lib as /lib */ g_assert_false (flatpak_exports_path_is_visible (exports, "/lib")); @@ -1862,21 +1918,27 @@ static void test_remap_ld_preload_flatpak_no_runtime (Fixture *f, gconstpointer context) { + const Config *config = context; g_autoptr(GPtrArray) argv = g_ptr_array_new_with_free_func (g_free); + g_autoptr(GPtrArray) filtered = filter_expected_paths (config); gsize i; - populate_ld_preload (f, argv, PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX, + populate_ld_preload (f, argv, + (config->preload_flags + | PV_APPEND_PRELOAD_FLAGS_FLATPAK_SUBSANDBOX), NULL, NULL); - g_assert_cmpuint (argv->len, ==, G_N_ELEMENTS (expected_preload_paths)); + g_assert_cmpuint (argv->len, ==, filtered->len); for (i = 0; i < argv->len; i++) { + char *expected = g_ptr_array_index (filtered, i); char *argument = g_ptr_array_index (argv, i); + g_assert_true (g_str_has_prefix (argument, "--ld-preload=")); argument += strlen("--ld-preload="); - g_assert_cmpstr (argument, ==, expected_preload_paths[i]); + g_assert_cmpstr (argument, ==, expected); } } @@ -2248,13 +2310,13 @@ main (int argc, g_test_add ("/options/true", Fixture, NULL, setup, test_options_true, teardown); g_test_add ("/passwd", Fixture, NULL, setup, test_passwd, teardown); - g_test_add ("/remap-ld-preload", Fixture, NULL, + g_test_add ("/remap-ld-preload", Fixture, &default_config, setup_ld_preload, test_remap_ld_preload, teardown); - g_test_add ("/remap-ld-preload-flatpak", Fixture, NULL, + g_test_add ("/remap-ld-preload-flatpak", Fixture, &default_config, setup_ld_preload, test_remap_ld_preload_flatpak, teardown); - g_test_add ("/remap-ld-preload-no-runtime", Fixture, NULL, + g_test_add ("/remap-ld-preload-no-runtime", Fixture, &default_config, setup_ld_preload, test_remap_ld_preload_no_runtime, teardown); - g_test_add ("/remap-ld-preload-flatpak-no-runtime", Fixture, NULL, + g_test_add ("/remap-ld-preload-flatpak-no-runtime", Fixture, &default_config, setup_ld_preload, test_remap_ld_preload_flatpak_no_runtime, teardown); g_test_add ("/supported-archs", Fixture, NULL, setup, test_supported_archs, teardown); @@ -2263,5 +2325,14 @@ main (int argc, g_test_add ("/use-host-os", Fixture, NULL, setup, test_use_host_os, teardown); + g_test_add ("/one-arch/remap-ld-preload", Fixture, &one_arch_config, + setup_ld_preload, test_remap_ld_preload, teardown); + g_test_add ("/one-arch/remap-ld-preload-flatpak", Fixture, &one_arch_config, + setup_ld_preload, test_remap_ld_preload_flatpak, teardown); + g_test_add ("/one-arch/remap-ld-preload-no-runtime", Fixture, &one_arch_config, + setup_ld_preload, test_remap_ld_preload_no_runtime, teardown); + g_test_add ("/one-arch/remap-ld-preload-flatpak-no-runtime", Fixture, &one_arch_config, + setup_ld_preload, test_remap_ld_preload_flatpak_no_runtime, teardown); + return g_test_run (); } -- GitLab