GNU bug report logs - #77653
[PATCH 0/4] Add WASM toolchain, wasi-libc, and browser WASM sandbox support

Previous Next

Full log

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ian Eure <ian <at> retrospec.tv>
Cc: 77653 <at> debbugs.gnu.org
Subject: Re: [bug#77653] [PATCH 4/4] gnu: Add wasm-sandboxed.
Date: Fri, 25 Apr 2025 22:01:13 +0900

Hi,

Ian Eure <ian <at> retrospec.tv> writes:

> * gnu/packages/gnuzilla.scm (wasm-sandboxed): New variable.
>
> Change-Id: I568e6cb9aca43122a06f46fd3a8d9a462754c36a
> ---
>  gnu/packages/gnuzilla.scm | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/gnu/packages/gnuzilla.scm b/gnu/packages/gnuzilla.scm
> index f4a912d8d5..32b2d13de5 100644
> --- a/gnu/packages/gnuzilla.scm
> +++ b/gnu/packages/gnuzilla.scm
> @@ -99,8 +99,38 @@ (define-module (gnu packages gnuzilla)
>    #:use-module (gnu packages xdisorg)
>    #:use-module (gnu packages readline)
>    #:use-module (gnu packages sqlite)
> +  #:use-module (gnu packages wasm)
>    #:autoload (json parser) (json->scm))
>  
> +(define-public (wasm-sandboxed orig-package)
> +  "Given a Firefox or Firefox-derived package ORIG-PACKAGE, return a
> +variant package which enables WASM sandboxing."
> +  (package
> +    (inherit orig-package)

If that is going to be called to define top-level packages in some other
module, that would also lead to the circular module dependency problem
discussed earlier.  Perhaps it's best to remain private then.

> +    (name (string-append (package-name orig-package) "-wasm-sandboxed"))
> +    (arguments
> +     (substitute-keyword-arguments (package-arguments orig-package)
> +       ((#:configure-flags flags)
> +        #~(let ((wasi-sysroot #$(this-package-native-input "wasm32-wasi-clang-toolchain")))
> +            (append (delq "--without-wasm-sandboxed-libraries" #$flags)

To compare strings, you want equal?, not eq?, so you should use delete
and not delq.

> +                    (list
> +                     (string-append "--with-wasi-sysroot=" wasi-sysroot "/wasm32-wasi")))))

Too wide: our guidelines says maximum width should be 80 columns.

> +       ((#:phases phases)

I find it's often a better practice to put a default value when using
`substitute-keyword-arguments', otherwise if there is no #:phases
argument in the base package, nothing happens.  So something like:

((#:phases phases '%standard-phases) ...)

would be more robust.

> +        #~(modify-phases #$phases
> +            (add-before 'configure 'set-wasm-env
> +              (lambda* (#:key inputs #:allow-other-keys)
> +                (setenv "WASM_CC"
> +                        (string-append (assoc-ref inputs
> +                                                  "wasm32-wasi-clang-toolchain")
> +                                       "/bin/clang"))

Instead of assoc-ref, you should use the more modern equivalent
#$(this-package-native-input "wasm32-wasi-clang-toolchain").

I'm not sure i we really want this wrapper, which is nice but could lead
to even more variants of firefox builds, which are expensive.  Perhaps
we should just make a decision to enable such support in our current
builds, if it doesn't cause performance problems yet (supposedly) helps
with security and be done.

-- 
Thanks,
Maxim

Previous Next

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