GNU bug report logs - #49578
[PATCH] Add bolt

Previous Next

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

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

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

From: phodina <phodina <at> protonmail.com>
To: "guix-patches <at> gnu.org" <guix-patches <at> gnu.org>
Subject: [PATCH] Add bolt
Date: Thu, 15 Jul 2021 16:46:01 +0000

---
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"))
+       #: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"))
+               (substitute* "meson.build"
+                 (("udev.get_pkgconfig_variable..udevdir..")
+                  (string-append "'" out "/lib'")))#t)))
+         (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)))))
+    (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

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

From: Sarah Morgensen <iskarian <at> mgsn.dev>
To: phodina <phodina <at> protonmail.com>
Cc: 49578 <at> debbugs.gnu.org
Subject: Re: bug#49578: [PATCH] Add bolt
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 <phodina <at> protonmail.com> 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

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

From: phodina <phodina <at> protonmail.com>
To: Sarah Morgensen <iskarian <at> mgsn.dev>
Cc: 49578 <at> debbugs.gnu.org
Subject: Re: bug#49578: [PATCH] Add bolt
Date: Thu, 22 Jul 2021 11:32:23 +0000

Hi Sarah,

Thanks for the suggestions. Fixes:

- polkit in alphabethical order
- line wraping
- /var dir location
- removed #t in phases
- simplified udev rules install
- more useful synopsis

---
index 41902e7785..6050871264 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -119,6 +119,7 @@
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pciutils)
   #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages polkit)
   #:use-module (gnu packages popt)
   #:use-module (gnu packages pulseaudio)
   #:use-module (gnu packages python)
@@ -2479,6 +2480,59 @@ 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)))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1phgp8fs0dlj74kbkqlvfniwc32daz47b3pvsxlfxqzyrp77xrfm"))))
+    (build-system meson-build-system)
+    (arguments
+     `(#:configure-flags (list "--localstatedir=/var")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'replace-directories
+           (lambda* (#:key outputs #:allow-other-keys)
+             (substitute* "meson.build"
+               (("udev.get_pkgconfig_variable..udevdir..")
+                (string-append "'" (assoc-ref %outputs "out") "/lib/udev'")))
+	     (substitute* "meson.build"
+               ((".*scripts/meson-install.sh.*") ""))
+	       ))
+         (add-before 'install 'no-polkit-magic
+           (lambda* (#:key outputs #:allow-other-keys)
+             ;; Meson ‘magically’ invokes pkexec, which fails (not setuid).
+             (setenv "PKEXEC_UID" "something")
+             )))))
+    (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 "Userspace daemon 'boltd' exposes devices via D-Bus to clients.
+It stores database of previously authorized devices and depending
+on the policy set for the individual devices, auomatically authorize newly
+connected devices without user interaction. It also adapts its behaivour when
+iommu support is detected.
+Command line utility 'boltctl' can be used to manage thundebolt devices. It can
+list devices, monitor changes and initiate authorization of device.")
+    (home-page "https://gitlab.freedesktop.org/bolt/bolt")
+    (license license:gpl2+)))
+
 (define-public jitterentropy-rngd
   (package
     (name "jitterentropy-rngd")
--
2.31.1

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

From: phodina <phodina <at> protonmail.com>
To: Sarah Morgensen <iskarian <at> mgsn.dev>
Cc: 49578 <at> debbugs.gnu.org
Subject: Re: bug#49578: [PATCH] Add bolt
Date: Thu, 22 Jul 2021 11:36:53 +0000

Also would you have recommendation on how to create a service for the daemon boltd?

Currently it's a good step to have it packaged, but without a running service it's little bit pointless.

There is probably something similar that I can check, learn how the services work and modify it for the thunderbolt case.

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

From: Sarah Morgensen <iskarian <at> mgsn.dev>
To: phodina <phodina <at> protonmail.com>
Cc: 49578 <at> debbugs.gnu.org
Subject: Re: bug#49578: [PATCH] Add bolt
Date: Thu, 22 Jul 2021 13:02:02 -0700

Hi,

phodina <phodina <at> protonmail.com> writes:

> Also would you have recommendation on how to create a service for the daemon boltd?
>
> Currently it's a good step to have it packaged, but without a running service it's little bit pointless.
>
> There is probably something similar that I can check, learn how the services work and modify it for the thunderbolt case.

I'm assuming you mean a service to run on Guix System, yes?

There is a section in the manual on this

  https://guix.gnu.org/manual/en/html_node/Defining-Services.html

which has a couple examples. You'll probably want to make a shepherd
service (shepherd is Guix System's services manager). You can also read
the code for existing services in gnu/services/*.scm. I'm not very
familiar with services so unfortunately I can't point you at any similar
ones. Best of luck!

--
Sarah

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

From: phodina <phodina <at> protonmail.com>
To: Sarah Morgensen <iskarian <at> mgsn.dev>
Cc: 49578 <at> debbugs.gnu.org
Subject: [PATCH v3 1/2] gnu: Add bolt.
Date: Sat, 18 Sep 2021 11:20:38 +0000

* gnu/packages/linux.scm: (bolt): New variable.

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index 46c9f817a8..3ec896bba6 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -121,6 +121,7 @@
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pciutils)
   #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages polkit)
   #:use-module (gnu packages popt)
   #:use-module (gnu packages pulseaudio)
   #:use-module (gnu packages python)
@@ -2655,6 +2656,56 @@ 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)))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1phgp8fs0dlj74kbkqlvfniwc32daz47b3pvsxlfxqzyrp77xrfm"))))
+    (build-system meson-build-system)
+    (arguments
+     `(#:configure-flags (list "--localstatedir=/var")
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'replace-directories
+           (lambda* (#:key outputs #:allow-other-keys)
+             (substitute* "meson.build"
+               (("udev.get_pkgconfig_variable..udevdir..")
+                (string-append "'" (assoc-ref %outputs "out") "/lib/udev'")))
+             (substitute* "meson.build"
+               ((".*scripts/meson-install.sh.*") ""))))
+         (add-before 'install 'no-polkit-magic
+           (lambda* (#:key outputs #:allow-other-keys)
+             ;; Meson ‘magically’ invokes pkexec, which fails (not setuid).
+             (setenv "PKEXEC_UID" "something"))))))
+    (native-inputs
+     `(("pkg-config" ,pkg-config)
+       ("glib:bin" ,glib "bin")
+       ("asciidoc" ,asciidoc)
+       ("umockdev" ,umockdev)))
+    (inputs
+     `(("eudev" ,eudev)
+       ("dbus" ,dbus)
+       ("polkit" ,polkit)))
+    (synopsis "Userspace system daemon for Thunderbolt")
+    (description "Userspace daemon @code{boltd} exposes devices via D-Bus to clients.
+It stores database of previously authorized devices and depending
+on the policy set for the individual devices, automatically authorize newly
+connected devices without user interaction.  It also adapts its behaivour when
+iommu support is detected.
+Command line utility 'boltctl' can be used to manage thundebolt devices.
+It can list devices, monitor changes and initiate authorization of device.")
+    (home-page "https://gitlab.freedesktop.org/bolt/bolt")
+    (license license:gpl2+)))
+
 (define-public jitterentropy-rngd
   (package
     (name "jitterentropy-rngd")
--
2.32.0

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

From: phodina <phodina <at> protonmail.com>
To: Sarah Morgensen <iskarian <at> mgsn.dev>
Cc: 49578 <at> debbugs.gnu.org
Subject: [PATCH v3 2/2] services: Add a service for bolt.
Date: Sat, 18 Sep 2021 11:21:34 +0000

    * gnu/services/linux.scm (bolt-service-type)
    (bolt-shepherd-service, bolt-dbus-service)
    (bolt-configuration, bolt-configuration?): New procedures.

diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index 2eb02ac5a3..dc26f285bf 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -4,6 +4,7 @@
 ;;; Copyright © 2020 Efraim Flashner <efraim <at> flashner.co.il>
 ;;; Copyright © 2021 raid5atemyhomework <raid5atemyhomework <at> protonmail.com>
 ;;; Copyright © 2021 B. Wilson <elaexuotee <at> wilsonb.com>
+;;; Copyright © 2021 Petr Hodina <phodina <at> protonmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -24,7 +25,9 @@
   #:use-module (guix gexp)
   #:use-module (guix records)
   #:use-module (guix modules)
+  #:use-module (gnu system shadow)
   #:use-module (gnu services)
+  #:use-module (gnu services dbus)
   #:use-module (gnu services base)
   #:use-module (gnu services shepherd)
   #:use-module (gnu packages linux)
@@ -33,7 +36,11 @@
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
   #:use-module (ice-9 match)
-  #:export (earlyoom-configuration
+  #:export (bolt-configuration
+            bolt-configuration?
+            bolt-service-type
+
+            earlyoom-configuration
             earlyoom-configuration?
             earlyoom-configuration-earlyoom
             earlyoom-configuration-minimum-available-memory
@@ -61,6 +68,76 @@
             zram-device-configuration-priority
             zram-device-service-type))

+

+;;;
+;;; Thunderbolt daemon.
+;;;
+
+(define-record-type* <bolt-configuration>
+  bolt-configuration make-bolt-configuration bolt-configuration?
+  (package bolt-configuration-package ; package
+           (default bolt)))
+
+(define bolt-shepherd-service
+  (match-lambda
+    (($ <bolt-configuration> package)
+     (with-imported-modules (source-module-closure
+                             '((gnu build shepherd)))
+       (shepherd-service
+        (documentation "Thunderbolt daemon")
+        (provision '(thunderbolt))
+        (requirement '(networking))
+        (modules '((gnu build shepherd)))
+        (start #~(make-forkexec-constructor/container
+                  (list #$(file-append package "/libexec/boltd"))
+		  ))
+        (stop #~(make-kill-destructor)))))))
+
+(define %bolt-activation
+  #~(begin
+      (use-modules (guix build utils))
+      (mkdir-p "/var/lib/boltd")))
+
+(define (bolt-dbus-service config)
+  (list (wrapped-dbus-service (bolt-configuration-bolt config)
+			      "libexec/boltd"
+			      `(("BOLT_CONF_FILE_NAME"
+				 '("share/dbus-1/interfaces/org.freedesktop.bolt.xml"))))))
+
+(define %bolt-accounts
+ (list (user-group (name "boltd") (system? #t))
+       (user-account
+	 (name "boltd")
+	 (group "boltd")
+	 (system? #t)
+	 (comment "Boltd daemon user")
+	 (home-directory "/var/empty")
+	 (shell "/run/current-system/profile/sbin/nologin"))))
+
+(define bolt-udev-rule
+  (match-lambda
+    (($ <bolt-configuration> package)
+  (file->udev-rule "90-bolt.rules" (file-append package "/lib/udev/rules.d/90-bolt.rules")))))
+
+(define bolt-service-type
+  (service-type
+   (name 'boltd)
+   (description
+    "Thunderbolt daemon")
+   (extensions
+    (list (service-extension udev-service-type
+			     (compose list bolt-udev-rule))
+	  (service-extension activation-service-type
+			     (const %bolt-activation))
+	  (service-extension dbus-root-service-type
+	  (compose list bolt-configuration-package))
+	;		     bolt-dbus-service)
+	  (service-extension account-service-type
+			     (const %bolt-accounts))
+          (service-extension shepherd-root-service-type
+                             (compose list bolt-shepherd-service))))
+   (default-value (bolt-configuration))))
+


 ;;;
 ;;; Early OOM daemon.
--
2.32.0

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

From: Sarah Morgensen <iskarian <at> mgsn.dev>
To: phodina <phodina <at> protonmail.com>
Cc: 49578 <at> debbugs.gnu.org
Subject: Re: [bug#49578] [PATCH v3 1/2] gnu: Add bolt.
Date: Wed, 22 Sep 2021 18:11:23 -0700

Hello Petr,

Thanks for the updated patch :)

I don't have any experiences with services, so I can't comment on the
second patch in this series, and I don't have a Thunderbolt controller
or devices, so I can't test the actual program, but other than the
commit message/synopsis/description this package LGTM.

phodina <phodina <at> protonmail.com> writes:

> * gnu/packages/linux.scm: (bolt): New variable.
                          ^ no ":"

> +    (synopsis "Userspace system daemon for Thunderbolt")

Consider this instead (taken from their repository tagline, more
understandable to more users):

       (synopsis "Thunderbolt 3 device manager")

> +    (description "Userspace daemon @code{boltd} exposes devices via D-Bus to clients.
> +It stores database of previously authorized devices and depending
> +on the policy set for the individual devices, automatically authorize newly
> +connected devices without user interaction.  It also adapts its behaivour when
> +iommu support is detected.
> +Command line utility 'boltctl' can be used to manage thundebolt devices.
> +It can list devices, monitor changes and initiate authorization of device.")

Consider this instead:

    (description "This package provides @command{boltd}, a userspace daemon
for Thunderbolt devices, and @command{boltctl}, a command-line utility for
managing those devices.

The daemon @command{boltd} exposes devices via D-Bus to clients.  It also
stores a database of previously authorized devices and will, depending on the
policy set for the individual devices, automatically authorize newly connected
devices without user interaction.

The command-line utility @command{boltctl} manages Thunderbolt devices via
@command{boltd}.  It can list devices, monitor changes, and initiate
authorization of devices.")


> +    (home-page "https://gitlab.freedesktop.org/bolt/bolt")
> +    (license license:gpl2+)))
> +
>  (define-public jitterentropy-rngd
>    (package
>      (name "jitterentropy-rngd")
> --
> 2.32.0

With those changes, or similar:

* Package review checklist (WIP)
Lack of a check is not proof of noncompliance.

1. [X] Package builds (guix build package)
     [X] x86_64  [ ] aarch64  [ ] armhf     [ ] powerpc64le
     [ ] i686    [ ] i586     [ ] mips64le
2. [X] Build is reproducible (guix build --rounds=n package)
3. [X] Tests enabled (if available) or disabled with in-source comment
4. [X] No extraneous dependencies (guix size package)
5. [X] No unnecessary pre-built binaries/blobs
6. [X] Dependencies unvendored when available in Guix
7. [X] Cross-compile friendly (cc-for-target, patches with inputs)
8. [X] License matches source
9. [X] No problematic lints (guix lint package)
10. [X] Formatting follows guidelines
    - [X] Lines wrap at 80 characters
    - [X] Indentation passes etc/indent-code.el
    - [X] Two spaces used between sentences

For new packages:
1. [X] Source url is robust
   * mirror:// used with url-fetch when possible
   * No auto-generated archives from source repo (prefer cloning)
2. [X] Synopsis is short, descriptive, and meaningful to a wide audience
3. [X] Description is objective, takes 5-10 lines, uses full sentences,
   and provides the information users need to decide whether the
   software fits their needs.

There are lints for CVEs, but they are for the Bolt CMS, not this
project.

--
Sarah

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

From: phodina <phodina <at> protonmail.com>
To: Sarah Morgensen <iskarian <at> mgsn.dev>
Cc: 49578 <at> debbugs.gnu.org
Subject: Re: [bug#49578] [PATCH v3 1/2] gnu: Add bolt.
Date: Fri, 05 Nov 2021 08:39:00 +0000

Hi,

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Thursday, September 23rd, 2021 at 3:11 AM, Sarah Morgensen <iskarian <at> mgsn.dev> wrote:

> Hello Petr,
>
> Thanks for the updated patch :)
>
> I don't have any experiences with services, so I can't comment on the
>
> second patch in this series, and I don't have a Thunderbolt controller
>
> or devices, so I can't test the actual program, but other than the
>
> commit message/synopsis/description this package LGTM.

Thanks Sarah for the review and tips on packaging.
>
> phodina phodina <at> protonmail.com writes:
>
> > -   gnu/packages/linux.scm: (bolt): New variable.
>
>                           ^ no ":"
>
>
> > -   (synopsis "Userspace system daemon for Thunderbolt")
>
> Consider this instead (taken from their repository tagline, more
>
> understandable to more users):
>
> (synopsis "Thunderbolt 3 device manager")
>
> > -   (description "Userspace daemon @code{boltd} exposes devices via D-Bus to clients.
> >
> >     +It stores database of previously authorized devices and depending
> >
> >     +on the policy set for the individual devices, automatically authorize newly
> >
> >     +connected devices without user interaction. It also adapts its behaivour when
> >
> >     +iommu support is detected.
> >
> >     +Command line utility 'boltctl' can be used to manage thundebolt devices.
> >
> >     +It can list devices, monitor changes and initiate authorization of device.")
>
> Consider this instead:
>
> (description "This package provides @command{boltd}, a userspace daemon
>
> for Thunderbolt devices, and @command{boltctl}, a command-line utility for
>
> managing those devices.
>
> The daemon @command{boltd} exposes devices via D-Bus to clients. It also
>
> stores a database of previously authorized devices and will, depending on the
>
> policy set for the individual devices, automatically authorize newly connected
>
> devices without user interaction.
>
> The command-line utility @command{boltctl} manages Thunderbolt devices via
>
> @command{boltd}. It can list devices, monitor changes, and initiate
>
> authorization of devices.")
>
> > -   (home-page "https://gitlab.freedesktop.org/bolt/bolt")
> > -   (license license:gpl2+)))
> >
> > (define-public jitterentropy-rngd
> >
> > (package
> >
> > (name "jitterentropy-rngd")
> > ------------------------------------------------------------------------
> >
> > 2.32.0
>
> With those changes, or similar:
>
> -   Package review checklist (WIP)
>
>     Lack of a check is not proof of noncompliance.
>
> 1.  [X] Package builds (guix build package)
>
>     [X] x86_64 [ ] aarch64 [ ] armhf [ ] powerpc64le
>
>     [ ] i686 [ ] i586 [ ] mips64le
> 2.  [X] Build is reproducible (guix build --rounds=n package)
> 3.  [X] Tests enabled (if available) or disabled with in-source comment
> 4.  [X] No extraneous dependencies (guix size package)
> 5.  [X] No unnecessary pre-built binaries/blobs
> 6.  [X] Dependencies unvendored when available in Guix
> 7.  [X] Cross-compile friendly (cc-for-target, patches with inputs)
> 8.  [X] License matches source
> 9.  [X] No problematic lints (guix lint package)
> 10.  [X] Formatting follows guidelines
>     -   [X] Lines wrap at 80 characters
>     -   [X] Indentation passes etc/indent-code.el
>     -   [X] Two spaces used between sentences
>
>         For new packages:
> 11.  [X] Source url is robust
>     -   mirror:// used with url-fetch when possible
>     -   No auto-generated archives from source repo (prefer cloning)
> 12.  [X] Synopsis is short, descriptive, and meaningful to a wide audience
> 13.  [X] Description is objective, takes 5-10 lines, uses full sentences,
>
>     and provides the information users need to decide whether the
>
>     software fits their needs.
>
>     There are lints for CVEs, but they are for the Bolt CMS, not this
>
>     project.
>
>     --
>
>     Sarah

Is there somebody who has Thunderbolt on their machine and could test this patch?

Petr

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

From: David Conner <aionfork <at> gmail.com>
To: 49578 <at> debbugs.gnu.org
Subject: [PATCH] Add bolt
Date: Sat, 22 Jan 2022 01:55:55 -0500

[Message part 1 (text/plain, inline)]

I have a late 2013 Macbook Pro with two Thunderbolt 2 ports.

They work if the TB device is inserted on boot, but after you disconnect a
device, it no longer functions when you reconnect it. It show that the
kernel recognizes the device in `dmesg` and  that the PCIe port is
registered in `lspci -k` But the device is not found in `nmcli dev status`.
The device I'm testing is an Apple Ethernet adapter.

I am still in the learning phase where I'm trying to reconfigure my
machine, but I have the guix repo cloned and I'm planning on contributing
patches to packages on other channels. I will hopefully be in the position
where I can test this soon.

[Message part 2 (text/html, inline)]

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

From: David Conner <aionfork <at> gmail.com>
To: 49578 <at> debbugs.gnu.org
Subject: Re: [PATCH] Add bolt
Date: Wed, 2 Feb 2022 11:19:01 -0500

[Message part 1 (text/plain, inline)]

I am ready to test this. I finally reconfigured my system last night, but I
have some questions about patching guix and testing.

The guix I would reconfigure in order to test this would be ‘master’ with
the patch applied. I think I should be alright applying the patch (I am
noob). If it doesn’t apply right, I can figure it out, but then I’ll need
to update the patch right?

Is git capable of doing this?

Or should I use a tool like ediff/patchwork?

I have the guix checked out with Google repo via
https://github.com/ectorepo/guix. This pulls down the latest copy of
everything on savannah under the guix project.

2022年1月22日(土) 午前1:55 David Conner <aionfork <at> gmail.com>:

> I have a late 2013 Macbook Pro with two Thunderbolt 2 ports.
>
> They work if the TB device is inserted on boot, but after you disconnect a
> device, it no longer functions when you reconnect it. It show that the
> kernel recognizes the device in `dmesg` and  that the PCIe port is
> registered in `lspci -k` But the device is not found in `nmcli dev status`.
> The device I'm testing is an Apple Ethernet adapter.
>
> I am still in the learning phase where I'm trying to reconfigure my
> machine, but I have the guix repo cloned and I'm planning on contributing
> patches to packages on other channels. I will hopefully be in the position
> where I can test this soon.
>

[Message part 2 (text/html, inline)]

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

From: phodina <phodina <at> protonmail.com>
To: Sarah Morgensen <iskarian <at> mgsn.dev>
Cc: 49578 <at> debbugs.gnu.org
Subject: Re: [bug#49578] [PATCH v4] gnu: Add bolt.
Date: Sun, 14 Aug 2022 22:00:32 +0000

[Message part 1 (text/plain, inline)]

Hi Sarah,

here's updated patch with your tips.

David, the way I test new packages is either by running:
[1]
guix shell --check --pure -D guix

or 
[2]
guix build -L <load_path>


[1] https://guix.gnu.org/manual/en/html_node/Invoking-guix-environment.html
[2] https://guix.gnu.org/manual/en/html_node/Common-Build-Options.html

----
Petr

[v4-0001-gnu-Add-bolt.patch (text/x-patch, attachment)]

[v4-0002-services-Add-a-service-for-bolt.patch (text/x-patch, attachment)]

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: phodina <phodina <at> protonmail.com>
Cc: Sarah Morgensen <iskarian <at> mgsn.dev>, 49578 <at> debbugs.gnu.org
Subject: Re: bug#49578: [PATCH] Add bolt
Date: Wed, 14 Dec 2022 12:22:48 +0100

Hi,

Thanks a lot, Sarah, for the review work.  Somehow it eventually fell
through the cracks but it’s never too late.  :-)

phodina <phodina <at> protonmail.com> skribis:

> From 538eeedf7ee64f98b17507ea11d38512525ef29f Mon Sep 17 00:00:00 2001
> From: Petr Hodina <phodina <at> protonmail.com>
> Date: Sun, 18 Jul 2021 12:11:55 +0200
> Subject: [PATCH v4 1/2] gnu: Add bolt.
>
> * gnu/packages/linux.scm (bolt): New variable.

LGTM!  I went ahead and applied it.

> From e054c89f9964686670e7716c820ca9ebb9f41543 Mon Sep 17 00:00:00 2001
> From: Petr Hodina <phodina <at> protonmail.com>
> Date: Sat, 18 Sep 2021 13:11:18 +0200
> Subject: [PATCH v4 2/2] services: Add a service for bolt.
>
> * gnu/services/linux.scm (bolt-service-type)
> (bolt-shepherd-service, bolt-dbus-service)
> (bolt-configuration, bolt-configuration?): New procedures.

[...]

> +(define-record-type* <bolt-configuration>
> +  bolt-configuration make-bolt-configuration bolt-configuration?
> +  (package bolt-configuration-package ; package
> +           (default bolt)))
> +
> +(define bolt-shepherd-service
> +  (match-lambda
> +    (($ <bolt-configuration> package)

Just call ‘bolt-configuration-package’ instead of using ‘match’ here.

> +     (with-imported-modules (source-module-closure
> +                             '((gnu build shepherd)))
> +       (shepherd-service
> +        (documentation "Thunderbolt daemon")
> +        (provision '(thunderbolt))
> +        (requirement '(networking))
> +        (modules '((gnu build shepherd)))
> +        (start #~(make-forkexec-constructor/container
> +                  (list #$(file-append package "/libexec/boltd"))
> +		  ))

Please move the parens to the previous line.  :-)

> +        (stop #~(make-kill-destructor)))))))
> +
> +(define %bolt-activation
> +  #~(begin
> +      (use-modules (guix build utils))
> +      (mkdir-p "/var/lib/boltd")))
> +
> +(define (bolt-dbus-service config)
> +  (list (wrapped-dbus-service (bolt-configuration-bolt config)
> +			      "libexec/boltd"
> +			      `(("BOLT_CONF_FILE_NAME"
> +				 '("share/dbus-1/interfaces/org.freedesktop.bolt.xml"))))))
> +
> +(define %bolt-accounts
> + (list (user-group (name "boltd") (system? #t))
> +       (user-account
> +	 (name "boltd")
> +	 (group "boltd")
> +	 (system? #t)
> +	 (comment "Boltd daemon user")
> +	 (home-directory "/var/empty")
> +	 (shell "/run/current-system/profile/sbin/nologin"))))

This is creating accounts, but ‘boltd’ is started as root.  Is that
expected?  Does ‘boltd’ setuids by itself?  If so, please add a comment
above the ‘make-forkexec-constructor’ call.

> +(define bolt-udev-rule
> +  (match-lambda
> +    (($ <bolt-configuration> package)

Same comment as above.

> +(define bolt-service-type
> +  (service-type
> +   (name 'boltd)
> +   (description
> +    "Thunderbolt daemon")

Please write full sentences here, giving enough context to make ‘guix
system search’ useful.

> +   (extensions
> +    (list (service-extension udev-service-type
> +			     (compose list bolt-udev-rule))
> +	  (service-extension activation-service-type
> +			     (const %bolt-activation))
> +	  (service-extension dbus-root-service-type
> +	  (compose list bolt-configuration-package))
> +	;		     bolt-dbus-service)

Maybe remove this line?

One last thing: please document it in ‘doc/guix.texi’.  In particular,
make sure to add a paragraph that gives a bit of context andq a
configuration example.

Could you send an updated patch?

Thanks,
Ludo’.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: phodina <phodina <at> protonmail.com>
Cc: Sarah Morgensen <iskarian <at> mgsn.dev>, 49578 <at> debbugs.gnu.org
Subject: Re: bug#49578: [PATCH] Add bolt
Date: Thu, 05 Jan 2023 22:28:57 +0100

Hi,

Just noticed an issue:

phodina <phodina <at> protonmail.com> skribis:

> +    (($ <bolt-configuration> package)
> +     (with-imported-modules (source-module-closure
> +                             '((gnu build shepherd)))
> +       (shepherd-service
> +        (documentation "Thunderbolt daemon")
> +        (provision '(thunderbolt))
> +        (requirement '(networking))
> +        (modules '((gnu build shepherd)))
> +        (start #~(make-forkexec-constructor/container
> +                  (list #$(file-append package "/libexec/boltd"))

‘make-forkexec-constructor/container’ is deprecated in favor of the
facilities in (guix least-authority).

However, running boltd in a container might prevent it from accessing
useful files or devices under /dev, /sys, or whatever.  I recommend
checking whether boltd can actually run in a container; you can strace
it to get an idea of the files it needs to access.  It needs to run as
root anyway so perhaps running it in a container is not that important.

Ludo’.

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

From: phodina <phodina <at> protonmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Sarah Morgensen <iskarian <at> mgsn.dev>, 49578 <at> debbugs.gnu.org
Subject: Re: bug#49578: [PATCH] Add bolt
Date: Tue, 10 Jan 2023 02:49:12 +0000

[Message part 1 (text/plain, inline)]

Hi Ludo’,


removed the deprecated contruct make-forkexec-constructor/container and changed bolt into boltd.

Also added plasma-thunderbolt package and documentation for the service.

----
Petr

[v5-0003-gnu-Add-plasma-thunderbolt.patch (text/x-patch, attachment)]

[v5-0004-gnu-kinfocenter-Add-plasma-thunderbolt.patch (text/x-patch, attachment)]

[v5-0001-gnu-Add-bolt-service-type.patch (text/x-patch, attachment)]

[v5-0002-gnu-linux-Add-boltd-service-type.patch (text/x-patch, attachment)]

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

From: Sergio Pastor Pérez <sergio.pastorperez <at> gmail.com>
To: 49578 <at> debbugs.gnu.org
Cc: David Conner <aionfork <at> gmail.com>,
 Ludovic Courtès <ludo <at> gnu.org>,
 Sughosha <sughosha <at> disroot.org>, Zheng Junjie <z572 <at> z572.online>,
 guix-devel <at> gnu.org, Sarah Morgensen <iskarian <at> mgsn.dev>,
 phodina <phodina <at> protonmail.com>
Subject: Re: [PATCH] Add bolt
Date: Tue, 06 May 2025 18:01:31 +0200

Hello!

It's been two years since this patch series was updated. Is there any
reason why it didn't get merged?

> Hi Ludo’,
> 
> 
> removed the deprecated contruct make-forkexec-constructor/container and changed bolt into boltd.
> 
> Also added plasma-thunderbolt package and documentation for the service.

The `plasma-thunderbolt' package depends on this.

How can we help to push this patch series through the finish line?


Best regards,
Sergio.

Previous Next

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