[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.
Commit Message
* 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 @LillianaPrikler@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.
.../patches/webkitgtk-adjust-bubblewrap-paths.patch | 13 ++++++++++++-
gnu/packages/webkit.scm | 8 +++++++-
2 files changed, 19 insertions(+), 2 deletions(-)
base-commit: b05bb6608c7f25ddce6b563194ba5a3007009282
Comments
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 :-).
Hi,
This is finally pushed to the gnome-team branch, as commit e7d08eeba9.
@@ -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) {
@@ -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.