Subject: Re: [bug#49969] [PATCH v2 5/7] gnu: desktop: Add seatd-service-type

From: Xinglu Chen <[email-masked]>
Date: Wed, 11 Aug 2021 13:52:37 +0200

On Tue, Aug 10 2021, muradm wrote:

> A seat management daemon, that does everything it needs to do.
> Nothing more, nothing less. Depends only on libc.

I don’t it is necessary to put this in the commit log.

> * gnu/services/desktop.scm: Add seatd-service-type
> ---
>  gnu/services/desktop.scm | 59 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
> index 64d0e85301..bfba9bccec 100644
> --- a/gnu/services/desktop.scm
> +++ b/gnu/services/desktop.scm
> @@ -13,6 +13,7 @@
>  ;;; Copyright © 2020 Tobias Geerinckx-Rice <[email-masked]>
>  ;;; Copyright © 2020 Reza Alizadeh Majd <[email-masked]>
>  ;;; Copyright © 2021 Brice Waegeneire <[email-masked]>
> +;;; Copyright © 2021 muradm <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -39,7 +40,9 @@
>    #:use-module (gnu services networking)
>    #:use-module (gnu services sound)
>    #:use-module ((gnu system file-systems)
> -                #:select (%elogind-file-systems file-system))
> +                #:select (%control-groups
> +                          %elogind-file-systems
> +                          file-system))
>    #:use-module (gnu system)
>    #:use-module (gnu system setuid)
>    #:use-module (gnu system shadow)
> @@ -154,6 +157,9 @@
>              gnome-keyring-configuration?
>              gnome-keyring-service-type
>  
> +            seatd-configuration
> +            seatd-service-type
> +
>              %desktop-services))
>  
>  ;;; Commentary:
> @@ -1182,6 +1188,57 @@ or setting its password with passwd.")))
>  (define polkit-wheel-service
>    (simple-service 'polkit-wheel polkit-service-type (list polkit-wheel)))
>  
> +
> +;;;
> +;;; seatd-service-type -- minimal seat management daemon
> +;;;
> +
> +(define-record-type* <seatd-configuration> seatd-configuration
> +  make-seatd-configuration
> +  seatd-configuration?
> +  (seatd seatd-package (default seatd))
> +  (user seatd-user (default "root"))
> +  (group seatd-group (default "users"))
> +  (socket seatd-socket (default "/run/seatd/socket"))
> +  (loglevel seatd-loglevel (default "error")))
> +
> +(define (seatd-shepherd-service config)
> +  (list (shepherd-service
> +         (requirement '())
> +         ;; TODO: once cgroups is separate dependency
> +         ;; here we should depend on it rather than elogind
> +         (provision '(seatd elogind))
> +         (start #~(make-forkexec-constructor
> +                   (list #$(file-append (seatd-package config) "/bin/seatd")
> +                         "-u" #$(seatd-user config)
> +                         "-g" #$(seatd-group config)
> +                         "-s" #$(seatd-socket config))
> +                   #:environment-variables
> +                   (list (string-append "SEATD_LOGLEVEL="
> +                                        #$(seatd-loglevel config)))
> +                   #:log-file "/tmp/seatd.log"))
> +         (stop #~(make-kill-destructor)))))

‘documentation’ field is missing.

> +
> +(define seatd-environment
> +  (match-lambda
> +    (($ <seatd-configuration> _ _ _ socket)
> +     `(("SEATD_SOCK" . ,socket)))))
> +
> +(define seatd-service-type
> +  (service-type
> +   (name 'seatd)
> +   (extensions
> +    (list
> +     (service-extension session-environment-service-type
> +                        seatd-environment)
> +     ;; TODO: once cgroups is separate dependency
> +     ;; we should not mount it here
> +     (service-extension file-system-service-type
> +                        (const %control-groups))
> +     (service-extension shepherd-root-service-type
> +                        seatd-shepherd-service)))
> +   (default-value (seatd-configuration))))

Missing ‘description’ field.

It might also be a good idea to write a system test for the service, you
can look at the (gnu tests …) modules for examples.

I am not familiar with how seatd works, so I can’t really comment too
much.  :-)