Subject: Re: bug#49578: [PATCH] Add bolt

From: Sarah Morgensen <[email-masked]>
Date: Wed, 21 Jul 2021 18:50:45 -0700

Hello,

Thanks for the patch. It's always good to see new contributors around
here! I have a few suggestions for your patch.

phodina <[email-masked]> writes:

> ---
> index 4b57bc1f24..d07ee33a7e 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -123,6 +123,7 @@
>    #:use-module (gnu packages pulseaudio)
>    #:use-module (gnu packages python)
>    #:use-module (gnu packages python-xyz)
> +  #:use-module (gnu packages polkit)
>    #:use-module (gnu packages readline)
>    #:use-module (gnu packages rrdtool)
>    #:use-module (gnu packages samba)
> @@ -2479,6 +2480,52 @@ IPv6 packet filter.
>  Both commands are targeted at system administrators.")
>      (license license:gpl2+)))
>
> +(define-public bolt
> +  (package
> +    (name "bolt")
> +    (version "0.9.1")
> +    (source (origin
> +              (method git-fetch)
> +              (uri
> +               (git-reference
> +                (url "https://gitlab.freedesktop.org/bolt/bolt")
> +                (commit version)))
> +              (sha256
> +               (base32
> +                "1phgp8fs0dlj74kbkqlvfniwc32daz47b3pvsxlfxqzyrp77xrfm"))))
> +    (build-system meson-build-system)
> +    (arguments
> +     `(#:configure-flags (list (string-append "--localstatedir=" (assoc-ref %outputs "out") "/var"))

Are you sure this shouldn't be "--localstatedir=/var"? As it is, it
refers to the read-only directory /gnu/store/...-bolt-0.9.1/var.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'fix-udev-rules-directory
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let ((out (assoc-ref outputs "out")))
> +               (mkdir-p (string-append out "/lib/udev/rules.d"))
                   ^ I think this is not necessary...

> +               (substitute* "meson.build"
> +                 (("udev.get_pkgconfig_variable..udevdir..")
> +                  (string-append "'" out "/lib'")))#t)))
                   ...with this change:      ^ /lib/udev

> +         (add-before 'install 'no-polkit-magic
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             ;; Meson ‘magically’ invokes pkexec, which fails (not setuid).
> +             (setenv "PKEXEC_UID" "something")
> +             #t)))))
                ^ Phases no longer need to end with #t, so you can omit this.

> +    (native-inputs `(("pkg-config" ,pkg-config) ("dbus" ,dbus) ("asciidoc" ,asciidoc) ("umockdev" ,umockdev)))

Please wrap lines at 80 characters, and in the special case of package
inputs like here, each input should get its own line, like:

  (native-inputs
   `(("pkg-config" ,pkg-config)
     ("dbus" ,dbus)
     ("asciidoc" ,asciidoc)
     ("umockdev" ,umockdev)))

> +    (inputs `(("glib:bin" ,glib "bin") ("eudev" ,eudev) ("polkit" ,polkit)))
> +    (synopsis "Userspace system daemon to enable security levels for Thunderbolt™
> +on GNU/Linux®.")
> +    (description "Thunderbolt™ is the brand name of a hardware interface developed by
> +Intel® that allows the connection of external peripherals to a
> +computer.
> +Devices connected via Thunderbolt can be DMA masters and thus read
> +system memory without interference of the operating system (or even
> +the CPU). Version 3 of the interface introduced 5 different security
> +levels, in order to mitigate the aforementioned security risk that
> +connected devices pose to the system. The security level is set by the
> +system firmware.")
> +    (home-page "https://gitlab.freedesktop.org/bolt/bolt")
> +    (license license:gpl2+)))
> +
>  (define-public jitterentropy-rngd
>    (package
>      (name "jitterentropy-rngd")
> --
> 2.31.1

--
Sarah