diff mbox series

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

Message ID 099d959851cb9ce9bf4afdedd01825b7ccf78a73.1671891059.git.mirai@makinata.eu
State New
Headers show
Series [bug#59866,v5.1] services: mpd: Refactor MPD service. | expand

Commit Message

Bruno Victal Dec. 24, 2022, 2:11 p.m. UTC
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'.

* 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".

(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(-)


base-commit: aae8371f72805cc35e31817e4120468eee4a4a80
prerequisite-patch-id: e47455c06e8e73edcc3f36ccd7b6b289cdfa1e16

Comments

Liliana Marie Prikler Dec. 24, 2022, 5:20 p.m. UTC | #1
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
Bruno Victal Dec. 24, 2022, 7:17 p.m. UTC | #2
On 2022-12-24 17:20, Liliana Marie Prikler wrote:
>> -(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?

I agree with you that they belong to /var/log, though I prefer leaving this to
MPDs default behavior when the directives are not explicitly set.
IIRC MPD will store them under $XDG_CONFIG_HOME/log if the "log_file" directive
is absent.

They were previously stored under /var/run because it was set by default to store
under ~/.mpd and the user account had its $HOME set as "/var/run/mpd/mpd".
(Looking closer to the old behavior, this also meant that the user field was actually
ignored as the user-group/user-account was hardcoded).
Since /var/run/mpd(/mpd) is the $HOME directory in the old style configuration, it
also contains other files of interest, as most of the fields default values are
directives relative to $HOME.

Personally I've no use to this migration part and I'd prefer an outright breaking
change to the new style, most of the MPD generated files can be regenerated without much hardship
and this is introducing complexity and soon™ to be obsolete code.

Cheers
diff mbox series

Patch

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.
+
+@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,Global
+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,Global 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 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.")
+
   (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)))))
 
-         (shell (file-append shadow "/sbin/nologin")))))
+(define (mpd-accounts config)
+  (match-record config <mpd-configuration> (user)
+    (list (user-account
+           (name user)
+           (group "nogroup")
+           (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-service-type
   (service-type
@@ -235,7 +484,9 @@  (define mpd-service-type
     (list (service-extension shepherd-root-service-type
                              (compose list mpd-shepherd-service))
           (service-extension account-service-type
-                             (const %mpd-accounts))
+                             mpd-accounts)
           (service-extension activation-service-type
-                             mpd-service-activation)))
+                             mpd-service-activation)
+          (service-extension rottlog-service-type
+                             (compose list mpd-log-rotation))))
    (default-value (mpd-configuration))))