Subject: [PATCH 04/17] services: mpd: Obsolete the 'group' field.

From: Maxim Cournoyer <[email-masked]>
Date: Fri, 28 Apr 2023 10:26:57 -0400

Prior to this change, there was a discrepancy where a user could have
disagreeing groups between the group and user fields (the user field being a
<user-account> record, which includes its primary group as a string).  This
could have caused problems because the USER's group was being used to set the
file permissions, while the GROUP name was serialized to MPD's configuration,
and MPD would use it to set the group of its running process.  Synchronizing
both is not practical, as it can easily lead to slightly different
<user-account> objects conflicting, again causing problems.

The compromise is to obsolete the 'group' field.  A group can still be
configured via the 'user' field, which accepts a <user-account> object, with
the limitation that the group should already exist.

* gnu/services/audio.scm (%lazy-group): Delete variable.
(%set-user-group): Delete procedure.
(mpd-serialize-user-group): Likewise.
(%mpd-user) [group]: Default to "audio".
(%mpd-group): Delete variable.
(mpd-group-sanitizer, mympd-group-sanitizer): Adjust sanitizers.
(mpd-configuration, mympd-configuration) [group]: Default to #f.  Update doc.
(mpd-accounts, mympd-accounts): Remove group.
(%mympd-user) [group]: Default to "nogroup".
* doc/guix.texi: Regenerate doc.
---
 doc/guix.texi          | 15 ++++----
 gnu/services/audio.scm | 80 ++++++++++++------------------------------
 2 files changed, 28 insertions(+), 67 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index f8acdbd6b5..34703b1698 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33571,8 +33571,8 @@ Audio Services
 @item @code{user} (type: user-account)
 The user to run mpd as.
 
-@item @code{group} (type: user-group)
-The group to run mpd as.
+@item @code{group} (default: @code{#f}) (type: boolean)
+Obsolete.  Do not use.
 
 @item @code{shepherd-requirement} (default: @code{()}) (type: list-of-symbol)
 This is a list of symbols naming Shepherd services that this service
@@ -33824,15 +33824,12 @@ Audio Services
 This is a list of symbols naming Shepherd services that this service
 will depend on.
 
-@item @code{user} (default: @code{%mympd-user}) (type: user-account)
+@item @code{user} (type: user-account)
 Owner of the @command{mympd} process.
 
-The default @code{%mympd-user} is a system user with the name ``mympd'',
-who is a part of the group @var{group} (see below).
-@item @code{group} (default: @code{%mympd-group}) (type: user-group)
-Owner group of the @command{mympd} process.
+@item @code{group} (default: @code{#f}) (type: boolean)
+Obsolete.  Do not use.
 
-The default @code{%mympd-group} is a system group with name ``mympd''.
 @item @code{work-directory} (default: @code{"/var/lib/mympd"}) (type: string)
 Where myMPD will store its data.
 
@@ -33872,7 +33869,7 @@ Audio Services
 Override URI to myMPD.  See
 @uref{https://github.com/jcorporation/myMPD/issues/950}.
 
-@item @code{script-acl} (default: @code{(mympd-ip-acl (allow '("127.0.0.1")))}) (type: maybe-mympd-ip-acl)
+@item @code{script-acl} (type: maybe-mympd-ip-acl)
 ACL to access the myMPD script backend.
 
 @item @code{ssl?} (default: @code{#f}) (type: boolean)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 7874539810..58262f7842 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,14 +140,6 @@ (define (uglify-field-name field-name)
 (define list-of-symbol?
   (list-of symbol?))
 
-;; Helpers for deprecated field types, to be removed later.
-(define %lazy-group (make-symbol "%lazy-group"))
-
-(define (set-user-group user group)
-  (user-account
-   (inherit user)
-   (group (user-group-name group))))
-
 
 ;;;
 ;;; MPD
@@ -175,9 +167,6 @@ (define (mpd-serialize-list-of-strings field-name value)
 (define (mpd-serialize-user-account field-name value)
   (mpd-serialize-string field-name (user-account-name value)))
 
-(define (mpd-serialize-user-group field-name value)
-  (mpd-serialize-string field-name (user-group-name value)))
-
 (define-maybe string (prefix mpd-))
 (define-maybe list-of-strings (prefix mpd-))
 (define-maybe boolean (prefix mpd-))
@@ -185,20 +174,13 @@ (define-maybe boolean (prefix mpd-))
 (define %mpd-user
   (user-account
    (name "mpd")
-   ;; XXX: This is a place-holder to be lazily substituted in (…-accounts)
-   ;; with the value from the 'group' field of <mpd-configuration>.
-   (group %lazy-group)
+   (group "audio")
    (system? #t)
    (comment "Music Player Daemon (MPD) user")
    ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data.
    (home-directory "/var/lib/mpd")
    (shell (file-append shadow "/sbin/nologin"))))
 
-(define %mpd-group
-  (user-group
-   (name "mpd")
-   (system? #t)))
-
 ;;; TODO: Procedures for deprecated fields, to be removed.
 
 (define mpd-deprecated-fields '((music-dir . music-directory)
@@ -242,15 +224,9 @@ (define (mpd-user-sanitizer value)
          (configuration-field-error #f 'user value))))
 
 (define (mpd-group-sanitizer value)
-  (cond ((user-group? value) value)
-        ((string? value)
-         (warning (G_ "string value for 'group' is deprecated, use \
-user-group instead~%"))
-         (user-group
-          (inherit %mpd-group)
-          (name value)))
-        (else
-         (configuration-field-error #f 'group value))))
+  (when value
+    (warning (G_ "'group' in <mpd-configuration> is obsolete; ignoring~%")))
+  #f)
 
 ;;;
 
@@ -407,9 +383,10 @@ (define-configuration mpd-configuration
    (sanitizer mpd-user-sanitizer))
 
   (group
-   (user-group %mpd-group)
-   "The group to run mpd as."
-   (sanitizer mpd-group-sanitizer))
+   (boolean #f)
+   "Obsolete.  Do not use."
+   (sanitizer mpd-group-sanitizer)
+   (serializer empty-serializer))
 
   (shepherd-requirement
    (list-of-symbol '())
@@ -633,12 +610,9 @@ (define (mpd-shepherd-service config)
                         (format #t "Service MPD is not running.")))))))))))
 
 (define (mpd-accounts config)
-  (match-record config <mpd-configuration> (user group)
-    ;; TODO: Deprecation code, to be removed.
-    (let ((user (if (eq? (user-account-group user) %lazy-group)
-                    (set-user-group user group)
-                    user)))
-      (list user group))))
+  (match-record config <mpd-configuration>
+    (user)
+    (list user)))
 
 (define mpd-service-type
   (service-type
@@ -676,9 +650,7 @@ (define-maybe/no-serialization mympd-ip-acl)
 (define %mympd-user
   (user-account
    (name "mympd")
-   ;; XXX: This is a place-holder to be lazily substituted in 'mympd-accounts'
-   ;; with the value from the 'group' field of <mympd-configuration>.
-   (group %lazy-group)
+   (group "nogroup")
    (system? #t)
    (comment "myMPD user")
    (home-directory "/var/empty")
@@ -702,15 +674,10 @@ (define (mympd-user-sanitizer value)
          (configuration-field-error #f 'user value))))
 
 (define (mympd-group-sanitizer value)
-  (cond ((user-group? value) value)
-        ((string? value)
-         (warning (G_ "string value for 'group' is not supported, use \
-user-group instead~%"))
-         (user-group
-          (inherit %mympd-group)
-          (name value)))
-        (else
-         (configuration-field-error #f 'group value))))
+  (when value
+    (warning (G_ "'group' in <mympd-configuration> is obsolete; ignoring~%")))
+  #f)
+
 ;;;

@@ -737,10 +704,10 @@ (define-configuration/no-serialization mympd-configuration
    empty-serializer)
 
   (group
-   (user-group %mympd-group)
-   "Owner group of the @command{mympd} process."
+   (boolean #f)
+   "Obsolete.  Do not use."
    (sanitizer mympd-group-sanitizer)
-   empty-serializer)
+   (serializer empty-serializer))
 
   (work-directory
    (string "/var/lib/mympd")
@@ -904,12 +871,9 @@ (define (mympd-shepherd-service config)
        (stop #~(make-kill-destructor))))))
 
 (define (mympd-accounts config)
-  (match-record config <mympd-configuration> (user group)
-    ;; TODO: Deprecation code, to be removed.
-    (let ((user (if (eq? (user-account-group user) %lazy-group)
-                    (set-user-group user group)
-                    user)))
-      (list user group))))
+  (match-record config <mympd-configuration>
+    (user)
+    (list user)))
 
 (define (mympd-log-rotation config)
   (match-record config <mympd-configuration> (log-to)
-- 
2.39.2