Subject: bug#63082: [PATCH 09/17] services: mpd: Let Shepherd effect the user/group change.

From: Bruno Victal <[email-masked]>
Date: Fri, 28 Apr 2023 23:11:27 +0100

On 2023-04-28 15:27, Maxim Cournoyer wrote:
> Relates to <https://issues.guix.gnu.org/63082>.
> 
> Quoting a MPD developer, regarding MPD's feature to switch user itself:
> "that's legacy for the dark ages when proper service managers did not exist"
> :-).
> 
> * gnu/services/audio.scm (mpd-serialize-user-account)
> (mpd-serialize-user-group): Delete procedures.
> * gnu/services/audio.scm (mpd-configuration) [user]: Do not serialize.
> [group]: Likewise.
> (mpd-shepherd-service): Provide the #:user, #:group and #:supplementary-groups
> arguments.
> (mympd-shepherd-service): Likewise, and remove the '--user' argument.
> * doc/guix.texi (Audio Services): Decorate mpd with @command.
> ---
>  doc/guix.texi          |  4 ++--
>  gnu/services/audio.scm | 31 ++++++++++++++++++++++---------
>  2 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/doc/guix.texi b/doc/guix.texi
> index a71a05bcf3..19320c2185 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -33569,7 +33569,7 @@ Audio Services
>  The MPD package.
>  
>  @item @code{user} (type: user-account)
> -The user to run mpd as.
> +The user to run @command{mpd} as.
>  
>  @item @code{group} (default: @code{#f}) (type: boolean)
>  Obsolete.  Do not use.
> @@ -33612,7 +33612,7 @@ Audio Services
>  The location of the sticker database.
>  
>  @item @code{default-port} (default: @code{6600}) (type: maybe-port)
> -The default port to run mpd on.
> +The default port to run @command{mpd} on.
>  
>  @item @code{endpoints} (type: maybe-list-of-strings)
>  The addresses that mpd will bind to.  A port different from
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index cccf5c2693..550ccc542c 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -3,6 +3,7 @@
>  ;;; Copyright © 2019 Ricardo Wurmus <[email-masked]>
>  ;;; Copyright © 2020 Ludovic Courtès <[email-masked]>
>  ;;; Copyright © 2022–2023 Bruno Victal <[email-masked]>
> +;;; Copyright © 2023 Maxim Cournoyer <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -164,9 +165,6 @@ (define mpd-serialize-boolean mpd-serialize-field)
>  (define (mpd-serialize-list-of-strings field-name value)
>    #~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
>  
> -(define (mpd-serialize-user-account field-name value)
> -  (mpd-serialize-string field-name (user-account-name value)))
> -
>  (define-maybe string (prefix mpd-))
>  (define-maybe list-of-strings (prefix mpd-))
>  (define-maybe boolean (prefix mpd-))
> @@ -387,10 +385,14 @@ (define-configuration mpd-configuration
>     "The MPD package."
>     empty-serializer)
>  
> +  ;; Note: The user and its group are not serialized, otherwise MPD would
> +  ;; attempt to switch the user/group itself.  The task of switching the
> +  ;; user/group is left to Shepherd instead.
>    (user
>     (user-account %mpd-user)
> -   "The user to run mpd as."
> -   (sanitizer mpd-user-sanitizer))
> +   "The user to run @command{mpd} as."
> +   (sanitizer mpd-user-sanitizer)
> +   (serializer empty-serializer))

Simply write empty-serializer after sanitizer instead.

>                       "--cachedir" #$cache-directory)
>                     #:environment-variables (list #$log-level*)
> -                   #:log-file #$(if (string? log-to) log-to #f))))
> +                   #:log-file #$(if (string? log-to) log-to #f)

Generic advice but how about this instead?

--8<---------------cut here---------------start------------->8---
#$@(if (string? log-to) `(#:log-file ,log-to) '())
--8<---------------cut here---------------end--------------->8---

It's cleaner to not explicitly set the keyword argument values when they're not used.