GNU bug report logs - #70446
[PATCH gnome-team] gnu: webkitgtk: Add system locale, dri access, and user profile access to gtk sandbox in order to silence gtk locale warnings and enable hardware accelerated video, respectively.

Previous Next

Package: guix-patches;

Reported by: Abhishek Cherath <abhi <at> quic.us>

Date: Thu, 18 Apr 2024 03:00:02 UTC

Severity: normal

Tags: patch

To reply to this bug, email your comments to 70446 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to liliana.prikler <at> gmail.com, maxim.cournoyer <at> gmail.com, vivien <at> planete-kraus.eu, guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Thu, 18 Apr 2024 03:00:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Abhishek Cherath <abhi <at> quic.us>:
New bug report received and forwarded. Copy sent to liliana.prikler <at> gmail.com, maxim.cournoyer <at> gmail.com, vivien <at> planete-kraus.eu, guix-patches <at> gnu.org. (Thu, 18 Apr 2024 03:00:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: guix-patches <at> gnu.org
Cc: Abhishek Cherath <abhi <at> quic.us>
Subject: [PATCH gnome-team] gnu: webkitgtk: Add system locale, dri access,
 and user profile access to gtk sandbox in order to silence gtk locale
 warnings and enable hardware accelerated video, respectively.
Date: Wed, 17 Apr 2024 22:52:04 -0400
* gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch:
Add @dridir@ and @localedir@ to bubblewrap gtk sandbox
Add ~/.guix-profile to bubblewrap gtk sandbox
* gnu/packages/webkit.scm (webkitgtk)[arguments]: In the
'configure-bubblewrap-store-directory' phase, also supply locale
and dri directory paths to webkitgtk-adjust-bubblewrap-paths.patch
template.
---
 .../webkitgtk-adjust-bubblewrap-paths.patch   | 28 +++++++++++++++++--
 gnu/packages/webkit.scm                       | 11 +++++++-
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
index 18ddb645ad..2b6f54c912 100644
--- a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
+++ b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
@@ -1,11 +1,21 @@
 Share /gnu/store in the BubbleWrap container and remove FHS mounts.
+Also share user profile directory.
 
 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
+index 99395d6..3604730 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
+@@ -765,1 +765,1 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
+         return adoptGRef(g_subprocess_launcher_spawnv(launcher, argv, error));
+
+     const char* runDir = g_get_user_runtime_dir();
++    const char* homeDir = g_get_home_dir();
++    char* profileDir = g_strconcat(homeDir, "/.guix-profile", NULL);
+     Vector<CString> sandboxArgs = {
+         "--die-with-parent",
+         "--unshare-uts",
+@@ -786,28 +788,24 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
          "--ro-bind", "/sys/dev", "/sys/dev",
          "--ro-bind", "/sys/devices", "/sys/devices",
  
@@ -33,6 +43,18 @@ index f0a5e4b05dff..88b11f806968 100644
 +
 +        // Bind mount the store inside the WebKitGTK sandbox.
 +        "--ro-bind", "@storedir@", "@storedir@",
++
++        // Bind mount the guix profile directory
++        "--ro-bind", profileDir, profileDir,
++
++        // This is needed for locales if not in profile
++        "--ro-bind-try", "@localedir@", "@localedir@",
++
++        // This is needed for video hardware acceleration (va-api)
++        // via /lib/dri if not in profile
++        "--ro-bind-try", "@dridir@", "@dridir@",
      };
++    free(profileDir);
  
-     if (launchOptions.processType == ProcessLauncher::ProcessType::DBusProxy) {
+     if (enableDebugPermissions()) {
+         const char* dataDir = g_get_user_data_dir();
diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
index bf24a65e83..a0d04f31d3 100644
--- a/gnu/packages/webkit.scm
+++ b/gnu/packages/webkit.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2019 Marius Bakke <mbakke <at> fastmail.com>
 ;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2022, 2023 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2024 Abhishek Cherath <abhi <at> quic.us>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -190,7 +191,15 @@ (define-public webkitgtk
               (let ((store-directory (%store-directory)))
                 (substitute*
                     "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp"
-                  (("@storedir@") store-directory)))))
+                  (("@storedir@") store-directory)
+                  ;; this adds access to drivers for va-api
+                  ;; for hardware accelerated video
+                  (("@dridir@") "/run/current-system/profile/lib/dri")
+                  ;; 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.

base-commit: b05bb6608c7f25ddce6b563194ba5a3007009282
-- 
2.41.0





Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Thu, 18 Apr 2024 03:15:04 GMT) Full text and rfc822 format available.

Message #8 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: 70446 <at> debbugs.gnu.org
Subject: Explanation
Date: Wed, 17 Apr 2024 23:14:11 -0400
This is to resubmit to gnome-team, along with adding the profile dir, as
recommended in https://issues.guix.gnu.org/69971




Information forwarded to liliana.prikler <at> gmail.com, maxim.cournoyer <at> gmail.com, vivien <at> planete-kraus.eu, guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Thu, 18 Apr 2024 04:09:03 GMT) Full text and rfc822 format available.

Message #11 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: 70446 <at> debbugs.gnu.org
Cc: Abhishek Cherath <abhi <at> quic.us>
Subject: [PATCH v2] gnu: webkitgtk: Add locale and dri access to gtk sandbox
 in order to silence gtk locale warnings and enable hardware accelerated video,
 respectively. Adjust bubblewrap wrapper to add user profile.
Date: Thu, 18 Apr 2024 00:06:12 -0400
* gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch:
Add @dridir@ and @localedir@ to bubblewrap gtk sandbox
Add ~/.guix-profile to bubblewrap gtk sandbox
* gnu/packages/webkit.scm (webkitgtk)[arguments]: In the
'configure-bubblewrap-store-directory' phase, also supply locale
and dri directory paths to webkitgtk-adjust-bubblewrap-paths.patch
template.

Change-Id: I6be0c473ebaa6c04ebb00a2b4afcae2c89396e4f
---
apparently the space on the second line of the patch is significant,
doesn't apply otherwise

 .../webkitgtk-adjust-bubblewrap-paths.patch   | 28 +++++++++++++++++--
 gnu/packages/webkit.scm                       | 11 +++++++-
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
index 18ddb645ad..c81916279e 100644
--- a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
+++ b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
@@ -1,11 +1,21 @@
 Share /gnu/store in the BubbleWrap container and remove FHS mounts.
+Also share user profile directory.
 
 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
+index 99395d6..3604730 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
+@@ -765,6 +765,8 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
+         return adoptGRef(g_subprocess_launcher_spawnv(launcher, argv, error));
+ 
+     const char* runDir = g_get_user_runtime_dir();
++    const char* homeDir = g_get_home_dir();
++    char* profileDir = g_strconcat(homeDir, "/.guix-profile", NULL);
+     Vector<CString> sandboxArgs = {
+         "--die-with-parent",
+         "--unshare-uts",
+@@ -786,28 +788,24 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
          "--ro-bind", "/sys/dev", "/sys/dev",
          "--ro-bind", "/sys/devices", "/sys/devices",
  
@@ -33,6 +43,18 @@ index f0a5e4b05dff..88b11f806968 100644
 +
 +        // Bind mount the store inside the WebKitGTK sandbox.
 +        "--ro-bind", "@storedir@", "@storedir@",
++
++        // Bind mount the guix profile directory
++        "--ro-bind", profileDir, profileDir,
++
++        // This is needed for locales if not in profile
++        "--ro-bind-try", "@localedir@", "@localedir@",
++
++        // This is needed for video hardware acceleration (va-api)
++        // via /lib/dri if not in profile
++        "--ro-bind-try", "@dridir@", "@dridir@",
      };
++    free(profileDir);
  
-     if (launchOptions.processType == ProcessLauncher::ProcessType::DBusProxy) {
+     if (enableDebugPermissions()) {
+         const char* dataDir = g_get_user_data_dir();
diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
index bf24a65e83..a0d04f31d3 100644
--- a/gnu/packages/webkit.scm
+++ b/gnu/packages/webkit.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2019 Marius Bakke <mbakke <at> fastmail.com>
 ;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2022, 2023 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2024 Abhishek Cherath <abhi <at> quic.us>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -190,7 +191,15 @@ (define-public webkitgtk
               (let ((store-directory (%store-directory)))
                 (substitute*
                     "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp"
-                  (("@storedir@") store-directory)))))
+                  (("@storedir@") store-directory)
+                  ;; this adds access to drivers for va-api
+                  ;; for hardware accelerated video
+                  (("@dridir@") "/run/current-system/profile/lib/dri")
+                  ;; 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.

base-commit: b05bb6608c7f25ddce6b563194ba5a3007009282
-- 
2.41.0





Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Thu, 18 Apr 2024 05:03:04 GMT) Full text and rfc822 format available.

Message #14 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: John Kehayias <john.kehayias <at> protonmail.com>
To: Abhishek Cherath <abhi <at> quic.us>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>,
 Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: bug#70446: [PATCH gnome-team] gnu: webkitgtk: Add system locale,
 dri access,
 and user profile access to gtk sandbox in order to silence gtk locale warnings
 and enable hardware accelerated video, respectively.
Date: Thu, 18 Apr 2024 05:02:02 +0000
On Wed, Apr 17, 2024 at 10:52 PM, Abhishek Cherath wrote:

> * gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch:
> Add @dridir@ and @localedir@ to bubblewrap gtk sandbox
> Add ~/.guix-profile to bubblewrap gtk sandbox
> * gnu/packages/webkit.scm (webkitgtk)[arguments]: In the
> 'configure-bubblewrap-store-directory' phase, also supply locale
> and dri directory paths to webkitgtk-adjust-bubblewrap-paths.patch
> template.
> ---

Perhaps combine with update for security issues as in
https://issues.guix.gnu.org/70404 ?





Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Thu, 18 Apr 2024 13:51:02 GMT) Full text and rfc822 format available.

Message #17 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: John Kehayias <john.kehayias <at> protonmail.com>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>,
 Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: bug#70446: [PATCH gnome-team] gnu: webkitgtk: Add system
 locale, dri access, and user profile access to gtk sandbox in order to
 silence gtk locale warnings and enable hardware accelerated video,
 respectively.
Date: Thu, 18 Apr 2024 09:50:22 -0400
> Perhaps combine with update for security issues as in
> https://issues.guix.gnu.org/70404 ?

In this patch?




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Fri, 19 Apr 2024 15:25:03 GMT) Full text and rfc822 format available.

Message #20 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>
Cc: John Kehayias <john.kehayias <at> protonmail.com>,
 Vivien Kraus <vivien <at> planete-kraus.eu>,
 Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: bug#70446: [PATCH gnome-team] gnu: webkitgtk: Add system
 locale, dri access, and user profile access to gtk sandbox in order to
 silence gtk locale warnings and enable hardware accelerated video,
 respectively.
Date: Fri, 19 Apr 2024 11:24:03 -0400
Hi,

Abhishek Cherath <abhi <at> quic.us> writes:

>> Perhaps combine with update for security issues as in
>> https://issues.guix.gnu.org/70404 ?
>
> In this patch?

No, patches should remain separated, but I think John meant combining as
in merging at the same time, to avoid large rebuilds twice.

-- 
Thanks,
Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Fri, 19 Apr 2024 18:54:04 GMT) Full text and rfc822 format available.

Message #23 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>, 70446 <at> debbugs.gnu.org
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#70446] [PATCH v2] gnu: webkitgtk: Add locale and dri
 access to gtk sandbox in order to silence gtk locale warnings and enable
 hardware accelerated video, respectively. Adjust bubblewrap wrapper to add
 user profile.
Date: Fri, 19 Apr 2024 20:53:19 +0200
Am Donnerstag, dem 18.04.2024 um 00:06 -0400 schrieb Abhishek Cherath:
> * gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch:
> Add @dridir@ and @localedir@ to bubblewrap gtk sandbox
> Add ~/.guix-profile to bubblewrap gtk sandbox
> * gnu/packages/webkit.scm (webkitgtk)[arguments]: In the
> 'configure-bubblewrap-store-directory' phase, also supply locale
> and dri directory paths to webkitgtk-adjust-bubblewrap-paths.patch
> template.
> 
> Change-Id: I6be0c473ebaa6c04ebb00a2b4afcae2c89396e4f
> ---
> apparently the space on the second line of the patch is significant,
> doesn't apply otherwise
Wrapping the entire user profile looks evil.  Why?




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Fri, 19 Apr 2024 20:26:01 GMT) Full text and rfc822 format available.

Message #26 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 70446 <at> debbugs.gnu.org
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#70446] [PATCH v2] gnu: webkitgtk: Add locale and dri access to gtk sandbox in order to silence gtk locale warnings and enable hardware accelerated video, respectively. Adjust bubblewrap wrapper to add user profile.
Date: Fri, 19 Apr 2024 16:24:56 -0400
Could just add the locale and dri dir, but afaik the user profile is just stuff in the store, right? And the thing has access to the whole store anyhow, so no change, right?




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Fri, 19 Apr 2024 20:37:01 GMT) Full text and rfc822 format available.

Message #29 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 70446 <at> debbugs.gnu.org
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#70446] [PATCH v2] gnu: webkitgtk: Add locale and dri access to gtk sandbox in order to silence gtk locale warnings and enable hardware accelerated video, respectively. Adjust bubblewrap wrapper to add user profile.
Date: Fri, 19 Apr 2024 16:33:11 -0400
Will say, I thought it was kinda odd to begin with that it has access to the whole store, though. 

On 19 April 2024 4:24:56 pm GMT-04:00, Abhishek Cherath <abhi <at> quic.us> wrote:
>Could just add the locale and dri dir, but afaik the user profile is just stuff in the store, right? And the thing has access to the whole store anyhow, so no change, right?




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Fri, 19 Apr 2024 21:20:05 GMT) Full text and rfc822 format available.

Message #32 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>, 70446 <at> debbugs.gnu.org
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#70446] [PATCH v2] gnu: webkitgtk: Add locale and dri
 access to gtk sandbox in order to silence gtk locale warnings and enable
 hardware accelerated video, respectively. Adjust bubblewrap wrapper to add
 user profile.
Date: Fri, 19 Apr 2024 23:19:27 +0200
Am Freitag, dem 19.04.2024 um 16:24 -0400 schrieb Abhishek Cherath:
> Could just add the locale and dri dir, but afaik the user profile is
> just stuff in the store, right? And the thing has access to the whole
> store anyhow, so no change, right?
The user dir *is* just stuff in the store, but it is particularly stuff
in the store that's linked to the currently logged-in user.  That is,
you're giving the sandbox extra information by exposing it, and I don't
think it'd be solely (or even largely) useful for beneficial purposes.

Cheers




Information forwarded to liliana.prikler <at> gmail.com, maxim.cournoyer <at> gmail.com, vivien <at> planete-kraus.eu, guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Fri, 19 Apr 2024 21:58:13 GMT) Full text and rfc822 format available.

Message #35 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: 70446 <at> debbugs.gnu.org
Cc: Abhishek Cherath <abhi <at> quic.us>
Subject: [PATCH v3] gnu: webkitgtk: Add locale and dri access to gtk sandbox
 in order to silence gtk locale warnings and enable hardware accelerated video,
 respectively. Adjust bubblewrap wrapper to add user profile locale and dri
 directories.
Date: Fri, 19 Apr 2024 17:55:11 -0400
* gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch:
Add @dridir@ and @localedir@ to bubblewrap gtk sandbox
Add ~/.guix-profile/lib/dri and ~/.guix-profile/share/locale
to bubblewrap gtk sandbox.

* gnu/packages/webkit.scm (webkitgtk)[arguments]: In the
'configure-bubblewrap-store-directory' phase, also supply locale
and dri directory paths to webkitgtk-adjust-bubblewrap-paths.patch
template.

Change-Id: I6be0c473ebaa6c04ebb00a2b4afcae2c89396e4f
---
Only shares user profile locale and dri folders.

 .../webkitgtk-adjust-bubblewrap-paths.patch   | 33 +++++++++++++++++--
 gnu/packages/webkit.scm                       | 11 ++++++-
 2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
index 18ddb645ad..0cf1498b92 100644
--- a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
+++ b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
@@ -1,11 +1,22 @@
 Share /gnu/store in the BubbleWrap container and remove FHS mounts.
+Also share locale and dri directories (user and system.)
 
 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
+index 99395d6..3604730 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
+@@ -765,6 +765,9 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
+         return adoptGRef(g_subprocess_launcher_spawnv(launcher, argv, error));
+ 
+     const char* runDir = g_get_user_runtime_dir();
++    const char* homeDir = g_get_home_dir();
++    char* userDriDir = g_strconcat(homeDir, "/.guix-profile/lib/dri", NULL);
++    char* userLocaleDir = g_strconcat(homeDir, "/.guix-profile/share/locale", NULL);
+     Vector<CString> sandboxArgs = {
+         "--die-with-parent",
+         "--unshare-uts",
+@@ -786,28 +788,28 @@ GRefPtr<GSubprocess> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
          "--ro-bind", "/sys/dev", "/sys/dev",
          "--ro-bind", "/sys/devices", "/sys/devices",
  
@@ -33,6 +44,22 @@ index f0a5e4b05dff..88b11f806968 100644
 +
 +        // Bind mount the store inside the WebKitGTK sandbox.
 +        "--ro-bind", "@storedir@", "@storedir@",
++
++        // Bind mount the locales in profile
++        "--ro-bind-try", userLocaleDir, userLocaleDir,
++
++        // Bind mount the dri dir in profile
++        "--ro-bind-try", userDriDir, userDriDir,
++
++        // This is needed for locales if not in profile
++        "--ro-bind-try", "@localedir@", "@localedir@",
++
++        // This is needed for video hardware acceleration (va-api)
++        // via /lib/dri if not in profile
++        "--ro-bind-try", "@dridir@", "@dridir@",
      };
++    free(userLocaleDir);
++    free(userDriDir);
  
-     if (launchOptions.processType == ProcessLauncher::ProcessType::DBusProxy) {
+     if (enableDebugPermissions()) {
+         const char* dataDir = g_get_user_data_dir();
diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
index bf24a65e83..a0d04f31d3 100644
--- a/gnu/packages/webkit.scm
+++ b/gnu/packages/webkit.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2019 Marius Bakke <mbakke <at> fastmail.com>
 ;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2022, 2023 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2024 Abhishek Cherath <abhi <at> quic.us>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -190,7 +191,15 @@ (define-public webkitgtk
               (let ((store-directory (%store-directory)))
                 (substitute*
                     "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp"
-                  (("@storedir@") store-directory)))))
+                  (("@storedir@") store-directory)
+                  ;; this adds access to drivers for va-api
+                  ;; for hardware accelerated video
+                  (("@dridir@") "/run/current-system/profile/lib/dri")
+                  ;; 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.

base-commit: b05bb6608c7f25ddce6b563194ba5a3007009282
-- 
2.41.0





Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Fri, 19 Apr 2024 22:01:11 GMT) Full text and rfc822 format available.

Message #38 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 70446 <at> debbugs.gnu.org
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#70446] [PATCH v2] gnu: webkitgtk: Add locale and dri access to gtk sandbox in order to silence gtk locale warnings and enable hardware accelerated video, respectively. Adjust bubblewrap wrapper to add user profile.
Date: Fri, 19 Apr 2024 17:59:55 -0400
That makes sense. I've modified the patch and sent a v3.

I only used the profile path instead of the specific paths because it's the first thing I got working, and I figured there wasn't really anything sensitive in the profile anyway.




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Fri, 19 Apr 2024 22:45:04 GMT) Full text and rfc822 format available.

Message #41 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>, 70446 <at> debbugs.gnu.org
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#70446] [PATCH v3] gnu: webkitgtk: Add locale and dri
 access to gtk sandbox in order to silence gtk locale warnings and enable
 hardware accelerated video, respectively. Adjust bubblewrap wrapper to add
 user profile locale and dri directories.
Date: Sat, 20 Apr 2024 00:43:56 +0200
Am Freitag, dem 19.04.2024 um 17:55 -0400 schrieb Abhishek Cherath:
> * gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch:
> Add @dridir@ and @localedir@ to bubblewrap gtk sandbox
> Add ~/.guix-profile/lib/dri and ~/.guix-profile/share/locale
> to bubblewrap gtk sandbox.
> 
> * gnu/packages/webkit.scm (webkitgtk)[arguments]: In the
> 'configure-bubblewrap-store-directory' phase, also supply locale
> and dri directory paths to webkitgtk-adjust-bubblewrap-paths.patch
> template.
> 
> Change-Id: I6be0c473ebaa6c04ebb00a2b4afcae2c89396e4f
> ---
> Only shares user profile locale and dri folders.
> 
>  .../webkitgtk-adjust-bubblewrap-paths.patch   | 33
> +++++++++++++++++--
>  gnu/packages/webkit.scm                       | 11 ++++++-
>  2 files changed, 40 insertions(+), 4 deletions(-)
> 
> diff --git a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-
> paths.patch b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-
> paths.patch
> index 18ddb645ad..0cf1498b92 100644
> --- a/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
> +++ b/gnu/packages/patches/webkitgtk-adjust-bubblewrap-paths.patch
> @@ -1,11 +1,22 @@
>  Share /gnu/store in the BubbleWrap container and remove FHS mounts.
> +Also share locale and dri directories (user and system.)
>  
>  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
> +index 99395d6..3604730 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
> +@@ -765,6 +765,9 @@ GRefPtr<GSubprocess>
> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
> +         return adoptGRef(g_subprocess_launcher_spawnv(launcher,
> argv, error));
> + 
> +     const char* runDir = g_get_user_runtime_dir();
> ++    const char* homeDir = g_get_home_dir();
> ++    char* userDriDir = g_strconcat(homeDir, "/.guix-
> profile/lib/dri", NULL);
> ++    char* userLocaleDir = g_strconcat(homeDir, "/.guix-
> profile/share/locale", NULL);
> +     Vector<CString> sandboxArgs = {
> +         "--die-with-parent",
> +         "--unshare-uts",
> +@@ -786,28 +788,28 @@ GRefPtr<GSubprocess>
> bubblewrapSpawn(GSubprocessLauncher* launcher, const Proces
>           "--ro-bind", "/sys/dev", "/sys/dev",
>           "--ro-bind", "/sys/devices", "/sys/devices",
>   
> @@ -33,6 +44,22 @@ index f0a5e4b05dff..88b11f806968 100644
>  +
>  +        // Bind mount the store inside the WebKitGTK sandbox.
>  +        "--ro-bind", "@storedir@", "@storedir@",
> ++
> ++        // Bind mount the locales in profile
> ++        "--ro-bind-try", userLocaleDir, userLocaleDir,
> ++
> ++        // Bind mount the dri dir in profile
> ++        "--ro-bind-try", userDriDir, userDriDir,
For reference, why are these two needed here?  Can't we do this with
the locales and drivers referenced below?  Should we perhaps expand
GUIX_LOCPATH here?
> ++
> ++        // This is needed for locales if not in profile
> ++        "--ro-bind-try", "@localedir@", "@localedir@",
> ++
> ++        // This is needed for video hardware acceleration (va-api)
> ++        // via /lib/dri if not in profile
> ++        "--ro-bind-try", "@dridir@", "@dridir@",
>       };
> ++    free(userLocaleDir);
> ++    free(userDriDir);
>   
> -     if (launchOptions.processType ==
> ProcessLauncher::ProcessType::DBusProxy) {
> +     if (enableDebugPermissions()) {
> +         const char* dataDir = g_get_user_data_dir();
> diff --git a/gnu/packages/webkit.scm b/gnu/packages/webkit.scm
> index bf24a65e83..a0d04f31d3 100644
> --- a/gnu/packages/webkit.scm
> +++ b/gnu/packages/webkit.scm
> @@ -8,6 +8,7 @@
>  ;;; Copyright © 2019 Marius Bakke <mbakke <at> fastmail.com>
>  ;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer
> <maxim.cournoyer <at> gmail.com>
>  ;;; Copyright © 2022, 2023 Efraim Flashner <efraim <at> flashner.co.il>
> +;;; Copyright © 2024 Abhishek Cherath <abhi <at> quic.us>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -190,7 +191,15 @@ (define-public webkitgtk
>                (let ((store-directory (%store-directory)))
>                  (substitute*
>                     
> "Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp"
> -                  (("@storedir@") store-directory)))))
> +                  (("@storedir@") store-directory)
> +                  ;; this adds access to drivers for va-api
> +                  ;; for hardware accelerated video
> +                  (("@dridir@") "/run/current-
> system/profile/lib/dri")
> +                  ;; 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.
> 
> base-commit: b05bb6608c7f25ddce6b563194ba5a3007009282
Note that any item you add here which references the user home will
fail to be loaded correctly when using `guix shell' in a way that hides
it; or even just using `guix shell' normally with a user who doesn't
have the hardware-accelerated drivers in their home.  For system paths,
this is somewhat different, since we can more or less expect them to
exist and mirror the layout of other distros to some extent.

Cheers




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 00:23:06 GMT) Full text and rfc822 format available.

Message #44 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: [bug#70446] [PATCH v3] gnu: webkitgtk: Add locale and dri
 access to gtk sandbox in order to silence gtk locale warnings and enable
 hardware accelerated video, respectively. Adjust bubblewrap wrapper to add
 user profile locale and dri directories.
Date: Fri, 19 Apr 2024 20:22:08 -0400
Hello,

>> ++        "--ro-bind-try", userLocaleDir, userLocaleDir,
>> ++
>> ++        // Bind mount the dri dir in profile
>> ++        "--ro-bind-try", userDriDir, userDriDir,
> For reference, why are these two needed here?  Can't we do this with
> the locales and drivers referenced below?  Should we perhaps expand
> GUIX_LOCPATH here?

Initially, I only had the system paths below those. I added these
so that people could have hardware accel by only installing the required
drivers in their local profiles (as recommended in 69971, unless I
entirely misunderstood)

I'm afraid I don't really know what adding stuff to GUIX_LOCPATH would
do. That's for foreign distros, correct? To reiterate, The locale
problem here is that the bubblewrapped process doesn't have access to
the locales, without which it throws warnings.

> Note that any item you add here which references the user home will
> fail to be loaded correctly when using `guix shell' in a way that hides
> it; or even just using `guix shell' normally with a user who doesn't
> have the hardware-accelerated drivers in their home.  For system paths,
> this is somewhat different, since we can more or less expect them to
> exist and mirror the layout of other distros to some extent.

Hmm, since it's in an ro-bind-try, that'll cause the drivers not to
work, and fall back to trying the system drivers. Is there a better
solution you could recommend?

Yours sincerely,
Abhishek Cherath.




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 00:41:02 GMT) Full text and rfc822 format available.

Message #47 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: [bug#70446] [PATCH v3] gnu: webkitgtk: Add locale and dri
 access to gtk sandbox in order to silence gtk locale warnings and enable
 hardware accelerated video, respectively. Adjust bubblewrap wrapper to add
 user profile locale and dri directories.
Date: Sat, 20 Apr 2024 02:40:23 +0200
Am Freitag, dem 19.04.2024 um 20:22 -0400 schrieb Abhishek Cherath:
> Hello,
> 
> > > ++        "--ro-bind-try", userLocaleDir, userLocaleDir,
> > > ++
> > > ++        // Bind mount the dri dir in profile
> > > ++        "--ro-bind-try", userDriDir, userDriDir,
> > For reference, why are these two needed here?  Can't we do this
> > with the locales and drivers referenced below?  Should we perhaps
> > expand GUIX_LOCPATH here?
> 
> Initially, I only had the system paths below those. I added these
> so that people could have hardware accel by only installing the
> required drivers in their local profiles (as recommended in 69971,
> unless I entirely misunderstood)
Ah, yes, Maxim did mention this, but yeah, non-static paths are NG
(nogood) here.  There really is no reason that those paths ought to
exist or be useful in a container, for example.

> I'm afraid I don't really know what adding stuff to GUIX_LOCPATH
> would do. That's for foreign distros, correct? To reiterate, The
> locale problem here is that the bubblewrapped process doesn't have
> access to the locales, without which it throws warnings.
Adding stuff *from* GUIX_LOCPATH, the idea being that this is where we
already advocate locales be put.

> > Note that any item you add here which references the user home will
> > fail to be loaded correctly when using `guix shell' in a way that
> > hides it; or even just using `guix shell' normally with a user who
> > doesn't have the hardware-accelerated drivers in their home.  For
> > system paths, this is somewhat different, since we can more or less
> > expect them to exist and mirror the layout of other distros to some
> > extent.
> 
> Hmm, since it's in an ro-bind-try, that'll cause the drivers not to
> work, and fall back to trying the system drivers. Is there a better
> solution you could recommend?
Unless a hard dependency on Mesa is appropriate (which we'd have to
confirm), I think just rolling with the system ones is okay.

Cheers 




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 01:54:02 GMT) Full text and rfc822 format available.

Message #50 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: [bug#70446] [PATCH v3] gnu: webkitgtk: Add locale and dri
 access to gtk sandbox in order to silence gtk locale warnings and enable
 hardware accelerated video, respectively. Adjust bubblewrap wrapper to add
 user profile locale and dri directories.
Date: Fri, 19 Apr 2024 21:52:58 -0400
Hello,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:

>> Initially, I only had the system paths below those. I added these
>> so that people could have hardware accel by only installing the
>> required drivers in their local profiles (as recommended in 69971,
>> unless I entirely misunderstood)
> Ah, yes, Maxim did mention this, but yeah, non-static paths are NG
> (nogood) here.  There really is no reason that those paths ought to
> exist or be useful in a container, for example.
>

Gotcha.

>> I'm afraid I don't really know what adding stuff to GUIX_LOCPATH
>> would do. That's for foreign distros, correct? To reiterate, The
>> locale problem here is that the bubblewrapped process doesn't have
>> access to the locales, without which it throws warnings.
> Adding stuff *from* GUIX_LOCPATH, the idea being that this is where we
> already advocate locales be put.

I see, so something along these lines?
```C
const char* guixLocPath = g_getenv("GUIX_LOCPATH");
char** locPaths = NULL;
if (guixLocPath != NULL) {
   locPaths = g_strsplit(guixLocPath,':', 4096);
   for (int i = 0; i < g_strv_length(locPaths); i++) {
       sandboxArgs.appendVector(Vector<CString>({
        "--ro-bind", *locPaths[i], *locPaths[i]
       }));
   }
   g_strfreev(locPaths);
}
```

>> > Note that any item you add here which references the user home will
>> > fail to be loaded correctly when using `guix shell' in a way that
>> > hides it; or even just using `guix shell' normally with a user who
>> > doesn't have the hardware-accelerated drivers in their home.  For
>> > system paths, this is somewhat different, since we can more or less
>> > expect them to exist and mirror the layout of other distros to some
>> > extent.
>> 
>> Hmm, since it's in an ro-bind-try, that'll cause the drivers not to
>> work, and fall back to trying the system drivers. Is there a better
>> solution you could recommend?
> Unless a hard dependency on Mesa is appropriate (which we'd have to
> confirm), I think just rolling with the system ones is okay.

Sounds good to me! Will send v4 with just that.




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 02:52:03 GMT) Full text and rfc822 format available.

Message #53 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: [bug#70446] [PATCH v3] gnu: webkitgtk: Add locale and dri
 access to gtk sandbox in order to silence gtk locale warnings and enable
 hardware accelerated video, respectively. Adjust bubblewrap wrapper to add
 user profile locale and dri directories.
Date: Sat, 20 Apr 2024 04:51:10 +0200
Am Freitag, dem 19.04.2024 um 21:52 -0400 schrieb Abhishek Cherath:
> 
> Hello,
> 
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:
> 
> > > Initially, I only had the system paths below those. I added these
> > > so that people could have hardware accel by only installing the
> > > required drivers in their local profiles (as recommended in
> > > 69971,
> > > unless I entirely misunderstood)
> > Ah, yes, Maxim did mention this, but yeah, non-static paths are NG
> > (nogood) here.  There really is no reason that those paths ought to
> > exist or be useful in a container, for example.
> > 
> 
> Gotcha.
> 
> > > I'm afraid I don't really know what adding stuff to GUIX_LOCPATH
> > > would do. That's for foreign distros, correct? To reiterate, The
> > > locale problem here is that the bubblewrapped process doesn't
> > > have
> > > access to the locales, without which it throws warnings.
> > Adding stuff *from* GUIX_LOCPATH, the idea being that this is where
> > we already advocate locales be put.
> 
> I see, so something along these lines?
> ```C
> const char* guixLocPath = g_getenv("GUIX_LOCPATH");
> char** locPaths = NULL;
> if (guixLocPath != NULL) {
>    locPaths = g_strsplit(guixLocPath,':', 4096);
>    for (int i = 0; i < g_strv_length(locPaths); i++) {
>        sandboxArgs.appendVector(Vector<CString>({
>         "--ro-bind", *locPaths[i], *locPaths[i]
>        }));
>    }
>    g_strfreev(locPaths);
> }
> ```
You can (and arguably should) use C++ semantics, and should not attempt
to hardcode any magic numbers here.  Historically, there used to be
more patches to deal with e.g. fonts, try to check if a procedure by
the name "bindIfExists" can still be found in the Webkit source.


Cheers




Information forwarded to liliana.prikler <at> gmail.com, maxim.cournoyer <at> gmail.com, vivien <at> planete-kraus.eu, guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 13:51:07 GMT) Full text and rfc822 format available.

Message #56 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: 70446 <at> debbugs.gnu.org
Cc: Abhishek Cherath <abhi <at> quic.us>
Subject: [PATCH 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.
Date: Sat, 20 Apr 2024 09:44:03 -0400
* 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 <at> 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(-)

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 <at> fastmail.com>
 ;;; Copyright © 2021, 2022, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2022, 2023 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2024 Abhishek Cherath <abhi <at> 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.

base-commit: b05bb6608c7f25ddce6b563194ba5a3007009282
-- 
2.41.0





Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 15:01:01 GMT) Full text and rfc822 format available.

Message #59 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>, 70446 <at> debbugs.gnu.org
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#70446] [PATCH 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.
Date: Sat, 20 Apr 2024 16:59:53 +0200
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 <at> 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





Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 15:32:04 GMT) Full text and rfc822 format available.

Message #62 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Abhishek Cherath <abhi <at> quic.us>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: [bug#70446] [PATCH 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.
Date: Sat, 20 Apr 2024 11:31:10 -0400
Hello,

>> Thanks [liliana.prikler <at> 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.




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 21:40:03 GMT) Full text and rfc822 format available.

Message #65 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: [bug#70446] [PATCH v3] gnu: webkitgtk: Add locale and dri
 access to gtk sandbox in order to silence gtk locale warnings and enable
 hardware accelerated video, respectively. Adjust bubblewrap wrapper to add
 user profile locale and dri directories.
Date: Sat, 20 Apr 2024 17:39:32 -0400
Hi Abhishek,

Abhishek Cherath <abhi <at> quic.us> writes:

> Hello,
>
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:
>
>>> Initially, I only had the system paths below those. I added these
>>> so that people could have hardware accel by only installing the
>>> required drivers in their local profiles (as recommended in 69971,
>>> unless I entirely misunderstood)
>> Ah, yes, Maxim did mention this, but yeah, non-static paths are NG
>> (nogood) here.  There really is no reason that those paths ought to
>> exist or be useful in a container, for example.
>>
>
> Gotcha.

Sorry for the confusion; I agree with Liliana that honoring GUIX_LOCPATH
is better than hard-coding any specific file name.

-- 
Thanks,
Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#70446; Package guix-patches. (Sat, 20 Apr 2024 21:44:04 GMT) Full text and rfc822 format available.

Message #68 received at 70446 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Abhishek Cherath <abhi <at> quic.us>
Cc: Vivien Kraus <vivien <at> planete-kraus.eu>,
 Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 70446 <at> debbugs.gnu.org
Subject: Re: [bug#70446] [PATCH 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.
Date: Sat, 20 Apr 2024 17:42:54 -0400
Hi,

Abhishek Cherath <abhi <at> quic.us> writes:

> Hello,
>
>>> Thanks [liliana.prikler <at> 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 :-).

-- 
Thanks,
Maxim




This bug report was last modified 13 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.