[bug#59866] [PATCH v5.1] services: mpd: Refactor MPD service.

[Liliana Marie Prikler]

Am Samstag, dem 24.12.2022 um 14:11 +0000 schrieb mirai@makinata.eu:
> From: Bruno Victal <mirai@makinata.eu>
> 
> Introduces 'mpd-plugin' and 'mpd-partition' records.
> Expands 'mpd-output' and 'mpd-configuration' records.
> Deprecates redundant abbreviated fields in 'mpd-configuration' and
> avoids
> serializing unused fields that may introduce undesired behavior.
> Replace free-form-args serialization by making 'mpd-serialize-field'
> handle
> multiple types and using it with 'generic-serialize-alist'.
> Reduce code weight by removing cosmetic indented serialization
> procedures.
> Offload logging from shepherd to MPD.
> Implement log-rotation via rottlog.
> Implement Shepherd actions: 'reopen' and 'configuration'.
The things you wrote here read a like a less structured ChangeLog.  The
section between header and ChangeLog should instead be used to give
some wider context if needed, imo.

> * gnu/services/audio.scm
> (mpd-plugin, mpd-partition): New record.
> 
> (mpd-plugin?, mpd-partition?, list-of-string?, list-of-symbol?)
> (list-of-mpd-plugin?, list-of-mpd-partition?)
> (list-of-mpd-plugin-or-output?, port?): New predicate.
> 
> (mpd-serialize-field): Handle multiple types.
> 
> (mpd-configuration)
> [package, group, shepherd-requirement, log-file, log-level, music-
> directory]
> [playlist-directory, endpoints, database, partitions, neighbors,
> inputs]
> [archive-plugins, input-cache-size, decoders, resampler, filters]
> [playlist-plugins, extra-options]: New field.
> [music-dir, playlist-dir, address]: Deprecate shorthand field.
> [db-file, state-file, sticker-file, port, outputs]: Change admissible
> type.
> 
> (mpd-log-rotation): New procedure.
> 
> (mpd-shepherd-service)
> [actions]: New shepherd actions: 'reopen' and 'configuration'.
> [requirement]: Splice with 'shepherd-requirement' field.
> [start]: Use 'package' field. Remove #:log-file parameter.
> 
> (mpd-service-activation): Create logging directory. Handle migration
> from old-style configuration.
> 
> (mpd-accounts): Do not hardcode username, change primary group to
> "nogroup".
While I welcome the extensibility here, I think the default should
still be mpd:mpd.  I think you should make it so that you can pass a
user-account and user-group to the mpd service so that they can be
reused (with a sanitizer that creates a user/group from string).

> (mpd-service-type): Extend rottlog-service-type for log-rotation.
> Update activation-service-type extension to reflect mpd-accounts
> procedure change.
> 
> * doc/guix.texi (Audio Services)[Music Player Daemon]: Update doc.
> ---
> 
>  Changes since v5:
>  * Serialize default_port field-name as "port".
> 
>  doc/guix.texi          | 177 +++++++++++++---
>  gnu/services/audio.scm | 463 +++++++++++++++++++++++++++++++--------
> --
>  2 files changed, 506 insertions(+), 134 deletions(-)
> 
> diff --git a/doc/guix.texi b/doc/guix.texi
> index e25692fd27..5663d1913a 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -109,6 +109,7 @@ Copyright @copyright{} 2022 Reily Siegel@*
>  Copyright @copyright{} 2022 Simon Streit@*
>  Copyright @copyright{} 2022 (@*
>  Copyright @copyright{} 2022 John Kehayias@*
> +Copyright @copyright{} 2022 Bruno Victal@*
>  
>  Permission is granted to copy, distribute and/or modify this
> document
>  under the terms of the GNU Free Documentation License, Version 1.3
> or
> @@ -33014,79 +33015,187 @@ The service type for @command{mpd}
>  Data type representing the configuration of @command{mpd}.
>  
>  @table @asis
> -@item @code{user} (default: @code{"mpd"})
> +@item @code{package} (default: @code{mpd}) (type: file-like)
> +The MPD package.
> +
> +@item @code{user} (default: @code{"mpd"}) (type: string)
>  The user to run mpd as.
>  
> -@item @code{music-dir} (default: @code{"~/Music"})
> +@item @code{group} (type: maybe-string)
> +The group to run mpd as.
> +
> +@item @code{shepherd-requirement} (default: @code{()}) (type: list-
> of-symbol)
> +This is a list of symbols naming Shepherd services that this service
> +will depend on.
> +
> +@item @code{log-file} (default: @code{"/var/log/mpd/log"}) (type:
> maybe-string)
> +The location of the log file.  Set to @code{syslog} to use the local
> +syslog daemon or @code{%unset-value} to omit this directive from the
> +configuration file.
> +
> +@item @code{log-level} (type: maybe-string)
> +Supress any messages below this threshold.  Available values:
> +@code{notice}, @code{info}, @code{verbose}, @code{warning} and
> +@code{error}.
> +
> +@item @code{music-directory} (type: maybe-string)
>  The directory to scan for music files.
>  
> -@item @code{playlist-dir} (default: @code{"~/.mpd/playlists"})
> +@item @code{playlist-directory} (type: maybe-string)
>  The directory to store playlists.
>  
> -@item @code{db-file} (default: @code{"~/.mpd/tag_cache"})
> +@item @code{db-file} (type: maybe-string)
>  The location of the music database.
>  
> -@item @code{state-file} (default: @code{"~/.mpd/state"})
> +@item @code{state-file} (type: maybe-string)
>  The location of the file that stores current MPD's state.
>  
> -@item @code{sticker-file} (default: @code{"~/.mpd/sticker.sql"})
> +@item @code{sticker-file} (type: maybe-string)
>  The location of the sticker database.
>  
> -@item @code{port} (default: @code{"6600"})
> -The port to run mpd on.
> +@item @code{default-port} (default: @code{6600}) (type: maybe-
> integer)
> +The default port to run mpd on.
> +
> +@item @code{endpoints} (type: maybe-list-of-string)
> +The addresses that mpd will bind to.  A different port may be
> +specified, e.g. @code{localhost:6602}.  IPv6 addresses must be
> +enclosed in square brackets if a different port is used.
> +To use a Unix domain socket, an absolute path or a path starting
> with @code{~}
> +can be specified here.
LGTM, but "A different port" should probably be "A port different from
@var{default-port}.

> +@item @code{database} (type: maybe-mpd-plugin)
> +MPD database plugin configuration.
> +
> +@item @code{partitions} (default: @code{()}) (type: list-of-mpd-
> partition)
> +List of MPD "partitions".
>  
> -@item @code{address} (default: @code{"any"})
> -The address that mpd will bind to.  To use a Unix domain socket,
> -an absolute path can be specified here.
> +@item @code{neighbors} (default: @code{()}) (type: list-of-mpd-
> plugin)
> +List of MPD neighbor plugin configurations.
>  
> -@item @code{outputs} (default: @code{"(list (mpd-output))"})
> -The audio outputs that MPD can use.  By default this is a single
> output using pulseaudio.
> +@item @code{inputs} (default: @code{()}) (type: list-of-mpd-plugin)
> +List of MPD input plugin configurations.
> +
> +@item @code{archive-plugins} (default: @code{()}) (type: list-of-
> mpd-plugin)
> +List of MPD archive plugin configurations.
> +
> +@item @code{input-cache-size} (type: maybe-string)
> +MPD input cache size.
> +
> +@item @code{decoders} (default: @code{()}) (type: list-of-mpd-
> plugin)
> +List of MPD decoder plugin configurations.
> +
> +@item @code{resampler} (type: maybe-mpd-plugin)
> +MPD resampler plugin configuration.
> +
> +@item @code{filters} (default: @code{()}) (type: list-of-mpd-plugin)
> +List of MPD filter plugin configurations.
> +
> +@item @code{outputs} (type: list-of-mpd-plugin-or-output)
> +The audio outputs that MPD can use.  By default this is a single
> output
> +using pulseaudio.
> +
> +@item @code{playlist-plugins} (default: @code{()}) (type: list-of-
> mpd-plugin)
> +List of MPD playlist plugin configurations.
> +
> +@item @code{extra-options} (default: @code{()}) (type: alist)
> +An association list of option symbols/strings to string values to be
> +appended to the configuration.
> +
> +@end table
> +@end deftp
> +
> +@deftp {Data Type} mpd-plugin
> +Data type representing a @command{mpd} plugin.
> +
> +@table @asis
> +@item @code{plugin} (type: maybe-string)
> +Plugin name.
> +
> +@item @code{name} (type: maybe-string)
> +Name.
> +
> +@item @code{enabled?} (type: maybe-boolean)
> +Whether the plugin is enabled/disabled.
> +
> +@item @code{extra-options} (default: @code{()}) (type: alist)
> +An association list of option symbols/strings to string values to be
> +appended to the plugin configuration.  See
> +@uref{https://mpd.readthedocs.io/en/latest/plugins.html,MPD plugin
> +reference} for available options.
> +
> +@end table
> +@end deftp
> +
> +@deftp {Data Type} mpd-partition
> +Data type representing a @command{mpd} partition.
> +
> +@table @asis
> +@item @code{name} (type: string)
> +Partition name.
> +
> +@item @code{extra-options} (default: @code{()}) (type: alist)
> +An association list of option symbols/strings to string values to be
> +appended to the partition configuration.  See
> +@uref{
> https://mpd.readthedocs.io/en/latest/user.html#configuring-partitions
> ,Configuring
> +Partitions} for available options.
>  
>  @end table
>  @end deftp
>  
>  @deftp {Data Type} mpd-output
> -Data type representing an @command{mpd} audio output.
> +Data type representing a @command{mpd} audio output.
>  
>  @table @asis
> -@item @code{name} (default: @code{"MPD"})
> +@item @code{name} (default: @code{"MPD"}) (type: string)
>  The name of the audio output.
>  
> -@item @code{type} (default: @code{"pulse"})
> +@item @code{type} (default: @code{"pulse"}) (type: string)
>  The type of audio output.
>  
> -@item @code{enabled?} (default: @code{#t})
> +@item @code{enabled?} (default: @code{#t}) (type: boolean)
>  Specifies whether this audio output is enabled when MPD is started. 
> By
>  default, all audio outputs are enabled.  This is just the default
>  setting when there is no state file; with a state file, the previous
>  state is restored.
>  
> -@item @code{tags?} (default: @code{#t})
> +@item @code{format} (type: maybe-string)
> +Force a specific audio format on output.  See
> +@uref{
> https://mpd.readthedocs.io/en/latest/user.html#audio-output-format,Gl
> obal
> +Audio Format} for a more detailed description.
> +
> +@item @code{tags?} (default: @code{#t}) (type: boolean)
>  If set to @code{#f}, then MPD will not send tags to this output. 
> This
>  is only useful for output plugins that can receive tags, for example
> the
>  @code{httpd} output plugin.
>  
> -@item @code{always-on?} (default: @code{#f})
> +@item @code{always-on?} (default: @code{#f}) (type: boolean)
>  If set to @code{#t}, then MPD attempts to keep this audio output
> always
> -open.  This may be useful for streaming servers, when you don’t want
> to
> +open.  This may be useful for streaming servers, when you don?t want
> to
>  disconnect all listeners even when playback is accidentally stopped.
>  
> -@item @code{mixer-type}
> -This field accepts a symbol that specifies which mixer should be
> used
> +@item @code{mixer-type} (default: @code{"none"}) (type: string)
> +This field accepts a string that specifies which mixer should be
> used
>  for this audio output: the @code{hardware} mixer, the
> @code{software}
>  mixer, the @code{null} mixer (allows setting the volume, but with no
>  effect; this can be used as a trick to implement an external mixer
>  External Mixer) or no mixer (@code{none}).
>  
> -@item @code{extra-options} (default: @code{'()})
> -An association list of option symbols to string values to be
> appended to
> -the audio output configuration.
> +@item @code{replay-gain-handler} (type: maybe-string)
> +This field accepts a string that specifies how
> +@uref{
> https://mpd.readthedocs.io/en/latest/user.html#replay-gain,Replay
> +Gain} is to be applied.  @code{software} uses an internal software
> +volume control, @code{mixer} uses the configured (hardware) mixer
> +control and @code{none} disables replay gain on this audio output.
> +
> +@item @code{extra-options} (default: @code{()}) (type: alist)
> +An association list of option symbols/strings to string values to be
> +appended to the audio output configuration.
>  
>  @end table
>  @end deftp
>  
> -The following example shows a configuration of @code{mpd} that
> provides
> -an HTTP audio streaming output.
> +The following example shows a configuration of @command{mpd} that
> +configures some of its plugins and provides a HTTP audio streaming
> output.
>  
>  @lisp
>  (service mpd-service-type
> @@ -33098,7 +33207,19 @@ an HTTP audio streaming output.
>                       (mixer-type 'null)
>                       (extra-options
>                        `((encoder . "vorbis")
> -                        (port    . "8080"))))))))
> +                        (port    . "8080"))))))
> +           (decoders
> +             (list (mpd-plugin
> +                     (plugin "mikmod")
> +                     (enabled? #f))
> +                   (mpd-plugin
> +                     (plugin "openmpt")
> +                     (enabled? #t)
> +                     (extra-options `((repeat-count . -1)
> +                                      (interpolation-filter .
> 1))))))
> +           (resampler (mpd-plugin
> +                        (plugin "libsamplerate")
> +                        (extra-options `((type . 0)))))))
>  @end lisp
>  
>  
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index 1a1026f342..e456205e99 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -21,19 +21,27 @@
>  
>  (define-module (gnu services audio)
>    #:use-module (guix gexp)
> +  #:use-module (guix deprecation)
> +  #:use-module (guix diagnostics)
> +  #:use-module (guix i18n)
>    #:use-module (gnu services)
>    #:use-module (gnu services configuration)
>    #:use-module (gnu services shepherd)
> +  #:use-module (gnu services admin)
>    #:use-module (gnu system shadow)
>    #:use-module (gnu packages admin)
>    #:use-module (gnu packages mpd)
>    #:use-module (guix records)
>    #:use-module (ice-9 match)
> -  #:use-module (ice-9 format)
>    #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-8)
>    #:use-module (srfi srfi-26)
>    #:export (mpd-output
>              mpd-output?
> +            mpd-plugin
> +            mpd-plugin?
> +            mpd-partition
> +            mpd-partition?
>              mpd-configuration
>              mpd-configuration?
>              mpd-service-type))
> @@ -52,180 +60,421 @@ (define (uglify-field-name field-name)
>                                 #\-)
>                   "_")))
>  
> -(define (free-form-args? val)
> -  (match val
> -    (() #t)
> -    ((((? symbol?) . (? string?)) . val) (free-form-args? val))
> -    (_ #f)))
> +(define list-of-string?
> +  (list-of string?))
>  
> -(define* (mpd-serialize-field field-name value #:optional (indent-
> level 0))
> -  #~(begin
> -      (use-modules ((ice-9 format)))
> -      (format #f "~v/~a \"~a\"~%" #$indent-level #$(if (string?
> field-name)
> -                                                       field-name
> -                                                       (uglify-
> field-name field-name)) #$value)))
> +(define list-of-symbol?
> +  (list-of symbol?))
>  
> -(define* (mpd-serialize-free-form-args field-name value #:optional
> (indent-level 0))
> -  (generic-serialize-alist string-append (cut mpd-serialize-field <>
> <> indent-level) value))
> +(define (mpd-serialize-field field-name value)
> +  (let ((field (if (string? field-name) field-name
> +                   (uglify-field-name field-name)))
> +        (value (if (boolean? value) (if value "yes" "no") value)))
> +    #~(format #f "~a \"~a\"~%" #$field #$value)))
>  
>  (define mpd-serialize-number mpd-serialize-field)
>  
>  (define mpd-serialize-string mpd-serialize-field)
>  
> -(define* (mpd-serialize-boolean field-name value #:optional (indent-
> level 0))
> -  (mpd-serialize-field field-name (if value "yes" "no") indent-
> level))
> +(define mpd-serialize-boolean mpd-serialize-field)
>  
> -(define (mpd-serialize-list-of-mpd-output field-name value)
> -  #~(string-append "\naudio_output {\n"
> -                   #$@(map (cut serialize-configuration <>
> -                                mpd-output-fields)
> -                           value)
> -                   "}\n"))
> +(define (mpd-serialize-alist field-name value)
> +  #~(string-append #$@(generic-serialize-alist list
> +                                               mpd-serialize-field
> +                                               value)))
>  
> -(define (mpd-serialize-configuration configuration)
> -  (mixed-text-file
> -   "mpd.conf"
> -   (serialize-configuration configuration mpd-configuration-
> fields)))
> +(define-maybe string (prefix mpd-))
> +(define-maybe list-of-string (prefix mpd-))
> +(define-maybe boolean (prefix mpd-))
> +
> +;;; TODO: Procedures for deprecated fields, to be removed.
> +
> +(define mpd-deprecated-fields '((music-dir . music-directory)
> +                                (playlist-dir . playlist-directory)
> +                                (address . endpoints)))
> +
> +(define (port? value) (or (string? value) (integer? value)))
> +
> +(define (mpd-serialize-deprecated-field field-name value)
> +  (if (maybe-value-set? value)
> +      (begin
> +        (warn-about-deprecation
> +         field-name #f
> +         #:replacement (assoc-ref mpd-deprecated-fields field-name))
> +        (match field-name
> +          ('playlist-dir (mpd-serialize-string "playlist_directory"
> value))
> +          ('music-dir (mpd-serialize-string "music_directory"
> value))
> +          ('address (mpd-serialize-string "bind_to_address"
> value))))
> +      ""))
> +
> +(define (mpd-serialize-port field-name value)
> +  (when (string? value)
> +    (warning
> +     (G_ "string value for '~a' is deprecated, use integer
> instead~%")
> +     field-name))
> +  (mpd-serialize-field "port" value))
> +
> +(define-maybe port (prefix mpd-))
> +
> +;;;
> +
> +;; Generic MPD plugin record, lists only the most prevalent fields.
> +(define-configuration mpd-plugin
> +  (plugin
> +   maybe-string
> +   "Plugin name.")
> +
> +  (name
> +   maybe-string
> +   "Name.")
> +
> +  (enabled?
> +   maybe-boolean
> +   "Whether the plugin is enabled/disabled.")
> +
> +  (extra-options
> +   (alist '())
> +   "An association list of option symbols/strings to string values
> +to be appended to the plugin configuration. See
> +@uref{https://mpd.readthedocs.io/en/latest/plugins.html,MPD plugin
> reference}
> +for available options.")
> +
> +  (prefix mpd-))
> +
> +(define (mpd-serialize-mpd-plugin field-name value)
> +  #~(format #f "~a {~%~a}~%"
> +            '#$field-name
> +            #$(serialize-configuration value mpd-plugin-fields)))
> +
> +(define (mpd-serialize-list-of-mpd-plugin field-name value)
> +  #~(string-append #$@(map (cut mpd-serialize-mpd-plugin field-name
> <>)
> +                           value)))
>  
> -(define mpd-subsystem-serialize-field (cut mpd-serialize-field <> <>
> 1))
> -(define mpd-subsystem-serialize-string (cut mpd-serialize-string <>
> <> 1))
> -(define mpd-subsystem-serialize-number (cut mpd-serialize-number <>
> <> 1))
> -(define mpd-subsystem-serialize-boolean (cut mpd-serialize-boolean
> <> <> 1))
> -(define mpd-subsystem-serialize-free-form-args (cut mpd-serialize-
> free-form-args <> <> 1))
> +(define list-of-mpd-plugin? (list-of mpd-plugin?))
> +
> +(define-maybe mpd-plugin (prefix mpd-))
> +
> +(define-configuration mpd-partition
> +  (name
> +   string
> +   "Partition name.")
> +
> +  (extra-options
> +   (alist '())
> +   "An association list of option symbols/strings to string values
> +to be appended to the partition configuration. See
> +@uref{
> https://mpd.readthedocs.io/en/latest/user.html#configuring-partitions
> ,Configuring Partitions}
> +for available options.")
> +
> +  (prefix mpd-))
> +
> +(define (mpd-serialize-mpd-partition field-name value)
> +  #~(format #f "partition {~%~a}~%"
> +            #$(serialize-configuration value mpd-partition-fields)))
> +
> +(define (mpd-serialize-list-of-mpd-partition field-name value)
> +  #~(string-append #$@(map (cut mpd-serialize-mpd-partition #f <>)
> value)))
> +
> +(define list-of-mpd-partition?
> +  (list-of mpd-partition?))
>  
>  (define-configuration mpd-output
>    (name
>     (string "MPD")
>     "The name of the audio output.")
> +
>    (type
>     (string "pulse")
>     "The type of audio output.")
> +
>    (enabled?
>     (boolean #t)
>     "Specifies whether this audio output is enabled when MPD is
> started. By
>  default, all audio outputs are enabled. This is just the default
>  setting when there is no state file; with a state file, the previous
>  state is restored.")
> +
> +  (format
> +   maybe-string
> +   "Force a specific audio format on output. See
> +@uref{
> https://mpd.readthedocs.io/en/latest/user.html#audio-output-format,Gl
> obal Audio Format}
> +for a more detailed description.")
> +   
>    (tags?
>     (boolean #t)
>     "If set to @code{#f}, then MPD will not send tags to this output.
> This
>  is only useful for output plugins that can receive tags, for example
> the
>  @code{httpd} output plugin.")
> +
>    (always-on?
>     (boolean #f)
>     "If set to @code{#t}, then MPD attempts to keep this audio output
> always
>  open. This may be useful for streaming servers, when you don’t want
> to
>  disconnect all listeners even when playback is accidentally
> stopped.")
> +
>    (mixer-type
>     (string "none")
> -   "This field accepts a symbol that specifies which mixer should be
> used
> +   "This field accepts a string that specifies which mixer should be
> used
>  for this audio output: the @code{hardware} mixer, the
> @code{software}
>  mixer, the @code{null} mixer (allows setting the volume, but with no
>  effect; this can be used as a trick to implement an external mixer
>  External Mixer) or no mixer (@code{none}).")
> +
> +  (replay-gain-handler
> +   maybe-string
> +   "This field accepts a string that specifies how
> +@uref{
> https://mpd.readthedocs.io/en/latest/user.html#replay-gain,Replay Gai
> n}
> +is to be applied. @code{software} uses an internal software volume
> control,
> +@code{mixer} uses the configured (hardware) mixer control and
> @code{none}
> +disables replay gain on this audio output.")
> +
>    (extra-options
> -   (free-form-args '())
> -   "An association list of option symbols to string values to be
> appended to
> -the audio output configuration.")
> -  (prefix mpd-subsystem-))
> +   (alist '())
> +   "An association list of option symbols/strings to string values
> +to be appended to the audio output configuration.")
>  
> -(define list-of-mpd-output?
> -  (list-of mpd-output?))
> +  (prefix mpd-))
> +
> +(define (mpd-serialize-mpd-output field-name value)
> +  #~(format #f "audio_output {~%~a}~%"
> +            #$(serialize-configuration value mpd-output-fields)))
> +
> +(define (mpd-serialize-list-of-mpd-plugin-or-output field-name
> value)
> +  (receive (plugins outputs)
> +      (partition mpd-plugin? value)
> +    #~(string-append #$@(map (cut mpd-serialize-mpd-plugin
> "audio_output" <>)
> +                             plugins)
> +                     #$@(map (cut mpd-serialize-mpd-output #f <>)
> outputs))))
> +
> +(define list-of-mpd-plugin-or-output?
> +  (list-of (lambda (x)
> +             (or (mpd-output? x) (mpd-plugin? x)))))
>  
>  (define-configuration mpd-configuration
> +  (package
> +   (file-like mpd)
> +   "The MPD package."
> +   empty-serializer)
> +
>    (user
>     (string "mpd")
>     "The user to run mpd as.")
> -  (music-dir
> -   (string "~/Music")
> +
> +  (group
> +   maybe-string
> +   "The group to run mpd as.")
> +
> +  (shepherd-requirement
> +   (list-of-symbol '())
> +   "This is a list of symbols naming Shepherd services that this
> service
> +will depend on."
> +   empty-serializer)
> +
> +  (log-file
> +   (maybe-string "/var/log/mpd/log")
> +   "The location of the log file. Set to @code{syslog} to use the
> +local syslog daemon or @code{%unset-value} to omit this directive
> +from the configuration file.")
> +
> +  (log-level
> +   maybe-string
> +   "Supress any messages below this threshold.
> +Available values: @code{notice}, @code{info}, @code{verbose},
> +@code{warning} and @code{error}.")
> +
> +  (music-directory
> +   maybe-string
> +   "The directory to scan for music files.")
> +
> +  (music-dir ; TODO: deprecated, remove later
> +   maybe-string
>     "The directory to scan for music files."
> -   (lambda (_ x)
> -     (mpd-serialize-field "music_directory" x)))
> -  (playlist-dir
> -   (string "~/.mpd/playlists")
> +   mpd-serialize-deprecated-field)
> +
> +  (playlist-directory
> +   maybe-string
> +   "The directory to store playlists.")
> +
> +  (playlist-dir ; TODO: deprecated, remove later
> +   maybe-string
>     "The directory to store playlists."
> -   (lambda (_ x)
> -     (mpd-serialize-field "playlist_directory" x)))
> +   mpd-serialize-deprecated-field)
> +
>    (db-file
> -   (string "~/.mpd/tag_cache")
> +   maybe-string
>     "The location of the music database.")
> +
>    (state-file
> -   (string "~/.mpd/state")
> +   maybe-string
>     "The location of the file that stores current MPD's state.")
> +
>    (sticker-file
> -   (string "~/.mpd/sticker.sql")
> +   maybe-string
>     "The location of the sticker database.")
> -  (port
> -   (string "6600")
> -   "The port to run mpd on.")
> -  (address
> -   (string "any")
> +
> +  (default-port
> +   (maybe-port 6600) ; TODO: switch to integer
> +   "The default port to run mpd on.")
> +
> +  (endpoints
> +   maybe-list-of-string
> +   "The addresses that mpd will bind to. A different port may be
> +specified, e.g. @code{localhost:6602}. IPv6 addresses must be
> +enclosed in square brackets if a different port is used.
> +To use a Unix domain socket, an absolute path or a path starting
> with @code{~}
> +can be specified here."
> +   (lambda (_ x)
> +     (if (maybe-value-set? x)
> +         #~(string-append #$@(map
> +                              (cut mpd-serialize-field
> "bind_to_address" <>)
> +                              x)) "")))
> +
> +  (address ; TODO: deprecated, remove later
> +   maybe-string
>     "The address that mpd will bind to.
>  To use a Unix domain socket, an absolute path can be specified
> here."
> +   mpd-serialize-deprecated-field)
> +
> +  (database
> +   maybe-mpd-plugin
> +   "MPD database plugin configuration.")
> +
> +  (partitions
> +   (list-of-mpd-partition '())
> +   "List of MPD \"partitions\".")
> +  
> +  (neighbors
> +   (list-of-mpd-plugin '())
> +   "List of MPD neighbor plugin configurations.")
> +
> +  (inputs
> +   (list-of-mpd-plugin '())
> +   "List of MPD input plugin configurations."
> +   (lambda (_ x)
> +     (mpd-serialize-list-of-mpd-plugin "input" x)))
> +
> +  (archive-plugins
> +   (list-of-mpd-plugin '())
> +   "List of MPD archive plugin configurations."
>     (lambda (_ x)
> -     (mpd-serialize-field "bind_to_address" x)))
> +     (mpd-serialize-list-of-mpd-plugin "archive_plugin" x)))
> +
> +  (input-cache-size
> +   maybe-string
> +   "MPD input cache size."
> +   (lambda (_ x)
> +     (if (maybe-value-set? x)
> +         #~(string-append "\ninput_cache {\n"
> +                          #$(mpd-serialize-string "size" x)
> +                          "}\n") "")))
> +
> +  (decoders
> +   (list-of-mpd-plugin '())
> +   "List of MPD decoder plugin configurations."
> +   (lambda (_ x)
> +     (mpd-serialize-list-of-mpd-plugin "decoder" x)))
> +
> +  (resampler
> +   maybe-mpd-plugin
> +   "MPD resampler plugin configuration.")
> +
> +  (filters
> +   (list-of-mpd-plugin '())
> +   "List of MPD filter plugin configurations."
> +   (lambda (_ x)
> +     (mpd-serialize-list-of-mpd-plugin "filter" x)))
> +
>    (outputs
> -   (list-of-mpd-output (list (mpd-output)))
> +   (list-of-mpd-plugin-or-output (list (mpd-output)))
>     "The audio outputs that MPD can use.
>  By default this is a single output using pulseaudio.")
> +
> +  (playlist-plugins
> +   (list-of-mpd-plugin '())
> +   "List of MPD playlist plugin configurations."
> +   (lambda (_ x)
> +     (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x)))
> +
> +  (extra-options
> +   (alist '())
> +   "An association list of option symbols/strings to string values
> to be
> +appended to the configuration.")
> +
>    (prefix mpd-))
>  
> -(define (mpd-file-name config file)
> -  "Return a path in /var/run/mpd/ that is writable
> -   by @code{user} from @code{config}."
> -  (string-append "/var/run/mpd/"
> -                 (mpd-configuration-user config)
> -                 "/" file))
> +(define (mpd-serialize-configuration configuration)
> +  (mixed-text-file
> +   "mpd.conf"
> +   (serialize-configuration configuration mpd-configuration-
> fields)))
> +
> +(define (mpd-log-rotation config)
> +  (match-record config <mpd-configuration> (log-file)
> +    (log-rotation
> +     (files (list log-file))
> +     (post-rotate #~(begin
> +                      (use-modules (gnu services herd))
> +                      (with-shepherd-action 'mpd ('reopen) #f))))))
>  
>  (define (mpd-shepherd-service config)
> -  (shepherd-service
> -   (documentation "Run the MPD (Music Player Daemon)")
> -   (requirement '(user-processes))
> -   (provision '(mpd))
> -   (start #~(make-forkexec-constructor
> -             (list #$(file-append mpd "/bin/mpd")
> -                   "--no-daemon"
> -                   #$(mpd-serialize-configuration config))
> -             #:environment-variables
> -             ;; Required to detect PulseAudio when run under a user
> account.
> -             (list (string-append
> -                    "XDG_RUNTIME_DIR=/run/user/"
> -                    (number->string
> -                     (passwd:uid
> -                      (getpwnam #$(mpd-configuration-user
> config))))))
> -             #:log-file #$(mpd-file-name config "log")))
> -   (stop  #~(make-kill-destructor))))
> +  (match-record config <mpd-configuration> (user package shepherd-
> requirement)
> +    (let* ((config-file (mpd-serialize-configuration config)))
> +      (shepherd-service
> +       (documentation "Run the MPD (Music Player Daemon)")
> +       (requirement `(user-processes loopback ,@shepherd-
> requirement))
> +       (provision '(mpd))
> +       (start #~(make-forkexec-constructor
> +                 (list #$(file-append package "/bin/mpd")
> +                       "--no-daemon"
> +                       #$config-file)
> +                 #:environment-variables
> +                 ;; Required to detect PulseAudio when run under a
> user account.
> +                 (list (string-append
> +                        "XDG_RUNTIME_DIR=/run/user/"
> +                        (number->string (passwd:uid (getpwnam
> #$user)))))))
> +       (stop  #~(make-kill-destructor))
> +       (actions
> +        (list (shepherd-configuration-action config-file)
> +              (shepherd-action
> +               (name 'reopen)
> +               (documentation "Re-open log files and flush caches.")
> +               (procedure
> +                #~(lambda (pid)
> +                    (if pid
> +                        (begin
> +                          (kill pid SIGHUP)
> +                          (format #t
> +                                  "Issued SIGHUP to Service MPD (PID
> ~a)."
> +                                  pid))
> +                        (format #t "Service MPD is not
> running.")))))))))))
>  
>  (define (mpd-service-activation config)
> -  (with-imported-modules '((guix build utils))
> +  (match-record config <mpd-configuration> (user log-file)
>      #~(begin
>          (use-modules (guix build utils))
> -        (define %user
> -          (getpw #$(mpd-configuration-user config)))
> -
> -        (let ((directory #$(mpd-file-name config ".mpd")))
> -          (mkdir-p directory)
> -          (chown directory (passwd:uid %user) (passwd:gid %user))
> -
> -          ;; Make /var/run/mpd/USER user-owned as well.
> -          (chown (dirname directory)
> -                 (passwd:uid %user) (passwd:gid %user))))))
> -
> -
> -(define %mpd-accounts
> -  ;; Default account and group for MPD.
> -  (list (user-group (name "mpd") (system? #t))
> -        (user-account
> -         (name "mpd")
> -         (group "mpd")
> -         (system? #t)
> -         (comment "Music Player Daemon (MPD) user")
>  
> -         ;; Note: /var/run/mpd hosts one sub-directory per user, of
> which
> -         ;; /var/run/mpd/mpd corresponds to the "mpd" user.
> -         (home-directory "/var/run/mpd/mpd")
> +        ;; TODO: remove me, migrates from the old location at
> /var/run/mpd
> +        ;;       to the new one at /var/lib/mpd.
> +        (let* ((user (getpw #$user))
> +               (deprecated-directory (string-append "/var/run/mpd/"
> +                                                    #$user "/.mpd"))
> +               (new-directory (string-append (passwd:dir user)
> +                                             "/.config/mpd")))
> +          (when (and (file-exists? deprecated-directory)
> +                     (not (file-exists? new-directory)))
> +            (rename-file deprecated-directory new-directory)
> +            (chown new-directory (passwd:uid user) (passwd:gid
> user))))
> +        (mkdir-p (dirname #$log-file)))))
I'm not sure whether we should migrate the logs here.  I do think the
logs should be stored in /var/log rather than /var/run, but other than
that I'm not even sure there's much value in changing the /var/run/mpd
structure.  What's your use case for doing so?


Cheers