| Message ID | 337ee6c76e8326b875045f6c8bf54304ff017311.1713620642.git.abhi@quic.us |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#70446,v4] gnu: webkitgtk: Add access to system locale path and to paths from GUIX_LOCPATH, LOCPATH, and LIBVA_DRIVERS_PATH to gtk sandbox in order to silence gtk locale warnings and enable hardware accelerated video. | expand |
Am Samstag, dem 20.04.2024 um 09:44 -0400 schrieb Abhishek Cherath: > * gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch: > Add @dridir@ and @localedir@ to bubblewrap gtk sandbox > Add paths from GUIX_LOCPATH, LOCPATH, and LIBVA_DRIVERS_PATH > to bubblewrap gtk sandbox. > > * gnu/packages/webkit.scm (webkitgtk)[arguments]: In the > 'configure-bubblewrap-store-directory' phase, also supply system > locale to webkitgtk-adjust-bubblewrap-paths.patch template. > > Change-Id: I6be0c473ebaa6c04ebb00a2b4afcae2c89396e4f > --- > Thanks [liliana.prikler@gmail.com] for all the help :D, I thought > about this a bit more and looked at all the utility stuff in > BubblewrapLauncher.cpp. I realized that the correct thing to do here > is to simply mount the LIBVA_DRIVERS_PATH paths. I'm wondering if > this shouldn't be part of the gstreamer default mounts even upstream? > along with the LOCPATH mount. This patch LGTM. I think submitting it upstream sans GUIX_LOCPATH would be a great idea – that way, we'd have fewer things to patch. Is @localedir@ still needed with the bindPathVar in place? Otherwise, as already said, LGTM, and I'll look into forwarding it to/cherry- picking it from gnome-team once I got new Webkit over there (still need to wait for CI on that). Cheers
Hello, >> Thanks [liliana.prikler@gmail.com] for all the help :D, I thought >> about this a bit more and looked at all the utility stuff in >> BubblewrapLauncher.cpp. I realized that the correct thing to do here >> is to simply mount the LIBVA_DRIVERS_PATH paths. I'm wondering if >> this shouldn't be part of the gstreamer default mounts even upstream? >> along with the LOCPATH mount. > This patch LGTM. I think submitting it upstream sans GUIX_LOCPATH > would be a great idea – that way, we'd have fewer things to patch. Sweet! I'll get on that sometime next month. > Is @localedir@ still needed with the bindPathVar in place? Otherwise, > as already said, LGTM, and I'll look into forwarding it to/cherry- > picking it from gnome-team once I got new Webkit over there (still need > to wait for CI on that). Yes, it's still needed since Guix system doesn't generally set LOCPATH (or GUIX_LOCPATH.) Thanks again for the review and suggestions! Yours sincerely, Abhishek Cherath.
Hi, Abhishek Cherath <abhi@quic.us> writes: > Hello, > >>> Thanks [liliana.prikler@gmail.com] for all the help :D, I thought >>> about this a bit more and looked at all the utility stuff in >>> BubblewrapLauncher.cpp. I realized that the correct thing to do here >>> is to simply mount the LIBVA_DRIVERS_PATH paths. I'm wondering if >>> this shouldn't be part of the gstreamer default mounts even upstream? >>> along with the LOCPATH mount. >> This patch LGTM. I think submitting it upstream sans GUIX_LOCPATH >> would be a great idea – that way, we'd have fewer things to patch. > > Sweet! I'll get on that sometime next month. > >> Is @localedir@ still needed with the bindPathVar in place? Otherwise, >> as already said, LGTM, and I'll look into forwarding it to/cherry- >> picking it from gnome-team once I got new Webkit over there (still need >> to wait for CI on that). > > Yes, it's still needed since Guix system doesn't generally set LOCPATH > (or GUIX_LOCPATH.) > > Thanks again for the review and suggestions! I just finished catching up with the thread. Great to see the review back and forth converging to an increasingly fancier solution :-).
diff --git a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch index 18ddb645ad..4195aca388 100644 --- a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch +++ b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch @@ -1,11 +1,13 @@ Share /gnu/store in the BubbleWrap container and remove FHS mounts. +Also share system locale directory and paths in LOCPATH, GUIX_LOCPATH, +and LIBVA_DRIVERS_PATH This is a Guix-specific patch not meant to be upstreamed. diff --git a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp index f0a5e4b05dff..88b11f806968 100644 --- a/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp +++ b/Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp -@@ -854,27 +854,12 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces +@@ -854,27 +854,21 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces "--ro-bind", "/sys/dev", "/sys/dev", "--ro-bind", "/sys/devices", "/sys/devices", @@ -33,6 +35,15 @@ index f0a5e4b05dff..88b11f806968 100644 + + // Bind mount the store inside the WebKitGTK sandbox. + "--ro-bind", "@storedir@", "@storedir@", ++ ++ // This is needed for system locales ++ "--ro-bind-try", "@localedir@", "@localedir@", }; ++ // User specified locale directory ++ bindPathVar(sandboxArgs, "LOCPATH"); ++ // Locales in case of foreign system. ++ bindPathVar(sandboxArgs, "GUIX_LOCPATH"); ++ // Drivers for video hardware acceleration (va-api) ++ bindPathVar(sandboxArgs, "LIBVA_DRIVERS_PATH"); if (launchOptions.processType == ProcessLauncher::ProcessType::DBusProxy) { diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm index bf24a65e83..d057bb3aa2 100644 --- a/gnu/packages/webkit.scm +++ b/gnu/packages/webkit.scm @@ -8,6 +8,7 @@ ;;; Copyright © 2019 Marius Bakke <mbakke@fastmail.com> ;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer@gmail.com> ;;; Copyright © 2022, 2023 Efraim Flashner <efraim@flashner.co.il> +;;; Copyright © 2024 Abhishek Cherath <abhi@quic.us> ;;; ;;; This file is part of GNU Guix. ;;; @@ -190,7 +191,12 @@ (define-public webkitgtk (let ((store-directory (%store-directory))) (substitute* "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp" - (("@storedir@") store-directory))))) + (("@storedir@") store-directory) + ;; this silences gtk locale errors + ;; Unfortunately, simply bind mounting /run/current-system + ;; does not work since it leads to weird issues + ;; with symlinks that confuse bubblewrap. + (("@localedir@") "/run/current-system/locale"))))) (add-after 'unpack 'do-not-disable-new-dtags ;; Ensure the linker uses new dynamic tags as this is what Guix ;; uses and validates in the validate-runpath phase.