On 2023-04-28 15:26, Maxim Cournoyer wrote:
> 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.
I'd skip documenting obsolete fields.
>
> -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.
Unrelated change?
> (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)
You can drop the trailing #f I think.
>
> ;;;
>
> @@ -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))
You can simply use empty-serializer after (or before) sanitizer, it is a
recognized literal for define-configuration.
>
> (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)
Trailing #f as mentioned above.
> @@ -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))
empty-serializer is a literal here. (although it's simply being used for
indication per the comment above this record-type)
>
> (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)))
Nitpick, personally I'm a fan of styling this part as:
--8<---------------cut here---------------start------------->8---
(match-record config <mympd-configuration> (user)
(list user))
--8<---------------cut here---------------end--------------->8---
