Subject: Re: [bug#28059] [PATCH] gnu: Add mpd service.

From: Peter Mikkelsen <[email-masked]>
Date: Sun, 13 Aug 2017 00:04:04 +0200

Christopher Baines <[email-masked]> writes:

> On Sat, 12 Aug 2017 19:10:08 +0200
> Peter Mikkelsen <[email-masked]> wrote:
>
>> Hi, thanks for the quick review!
>>
>> Christopher Baines <[email-masked]> writes:
>>
>> > Hey,
>> >
>> > This looks great Peter, awesome job :) I've made some notes about
>> > potential improvements inline below.
>> >
>> > I've succeeded in running the system test locally, but there was
>> > some suspicious output in the log:
>> >
>> > exception: bind to '0.0.0.0:6600' failed (continuing anyway, because
>> > binding to '[::]:6600' succeeded): Failed to bind socket: Address
>> > already in use exception: Failed to access /root/.mpd/playlists: No
>> > such file or directory
>> >
>>
>> This is pretty normal for mpd, and I believe happens because it first
>> binds on IPv6 and then fails for IPv4. The error about the playlist
>> dir happens because it does not exist, but AFAIK it is no problem
>> unless you want to save playlists. The user can just create the dir.
>
> Ok, good to know :)
>
>> > On Sat, 12 Aug 2017 03:52:11 +0200
>> > Peter Mikkelsen <[email-masked]> wrote:
>> >> * doc/guix.text: Add documentation.
>> >
>> > Typo above, text rather than texi.
>> >
>>
>> Ups, my mistake.
>>
>> >> * gnu/services/music.scm (<mpd-configuration>): New record type.
>> >>   (mpd-service): New service extension.
>> >>   (mpd-service-type): New service type.
>> >> * gnu/tests/music.scm: New file.
>> >> * gnu/local.mk: Add new files.
>> >>
>> >> ---
>> >>  doc/guix.texi          | 53 +++++++++++++++++++++++++++++++
>> >>  gnu/local.mk           |  2 ++
>> >>  gnu/services/music.scm | 84
>> >> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> >> gnu/tests/music.scm    | 83
>> >> +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed,
>> >> 222 insertions(+) create mode 100644 gnu/services/music.scm create
>> >> mode 100644 gnu/tests/music.scm
>> >>
>> >> diff --git a/doc/guix.texi b/doc/guix.texi
>> >> index 8f14ddd50..e565dfdc9 100644
>> >> --- a/doc/guix.texi
>> >> +++ b/doc/guix.texi
>> >> @@ -227,6 +227,7 @@ Services
>> >>  * Network File System::         NFS related services.
>> >>  * Continuous Integration::      The Cuirass service.
>> >>  * Power management Services::   The TLP tool.
>> >> +* Music Services::              The MPD.
>> >>  * Miscellaneous Services::      Other services.
>> >>
>> >>  Defining Services
>> >> @@ -9035,6 +9036,7 @@ declaration.
>> >>  * Network File System::         NFS related services.
>> >>  * Continuous Integration::      The Cuirass service.
>> >>  * Power management Services::   The TLP tool.
>> >> +* Music Services::              The MPD.
>> >>  * Miscellaneous Services::      Other services.
>> >>  @end menu
>> >>
>> >> @@ -15635,6 +15637,57 @@ Package object of thermald.
>> >>  @end table
>> >>  @end deftp
>> >>
>> >> +@node Music Services
>> >> +@subsubsection Music Services
>> >
>> > I'm wondering if Audio services, rather than Music services might be
>> > better? Maybe this would fit in other services related to audio,
>> > e.g. Jack, MIDI stuff, etc...
>> >
>>
>> That sounds like a good idea.
>>
>> >> +@cindex mpd
>> >> +@subsubheading Music Player Daemon
>> >> +
>> >> +The @code{(gnu services music)} provides a service to start MPD
>> >> (the Music +Player Daemon). It uses pulseaudio for output.
>> >> +
>> >> +@defvr {Scheme Variable} mpd-service-type
>> >> +The service type for @command{mpd}
>> >> +@end defvr
>> >> +
>> >> +@deftp {Data Type} mpd-configuration
>> >> +Data type representing the configuration of @command{mpd}.
>> >> +
>> >> +@table @asis
>> >> +@item @code{user} (default: @code{"mpd"})
>> >> +The user to run mpd as.
>> >> +
>> >> +@item @code{music-dir} (default: @code{"~/Music"})
>> >> +The directory to scan for music files.
>> >> +
>> >> +@item @code{playlist-dir} (default: @code{"~/.mpd/playlists"})
>> >> +The directory to store playlists.
>> >> +
>> >> +@item @code{pid-file} (default: @code{"~/.mpd-pid"})
>> >> +The file mpd wil store its PID.
>> >> +
>> >> +@item @code{port} (default: @code{"6600"})
>> >> +The port to run mpd on.
>> >> +
>> >> +@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.
>> >
>> > The style for Guix is to use two spaces between sentences, I always
>> > forget about this too.
>> >
>>
>> Oh yes, me too.
>>
>> >> +
>> >> +@end table
>> >> +@end deftp
>> >> +
>> >> +@deffn {Scheme Procedure} mpd-service [#:config
>> >> (mpd-configuration)] +Return a service that runs @code{mpd} using
>> >> @var{configuration}, +a @code{<mpd-configuration>} object.
>> >> +
>> >> +The following example shows how one might run @code{mpd} as user
>> >> +@code{"bob"} on port @code{6666}.
>> >> +@example
>> >> +(mpd-service (mpd-configuration
>> >> +              (user "bob")
>> >> +              (port "6666")))
>> >> +@end example
>> >> +@end deffn
>> >>
>> >>  @node Miscellaneous Services
>> >>  @subsubsection Miscellaneous Services
>> >> diff --git a/gnu/local.mk b/gnu/local.mk
>> >> index b1ff72d6a..cad0ba38d 100644
>> >> --- a/gnu/local.mk
>> >> +++ b/gnu/local.mk
>> >> @@ -441,6 +441,7 @@ GNU_SYSTEM_MODULES
>> >> =				\
>> >> %D%/services/mail.scm				\
>> >> %D%/services/mcron.scm			\
>> >> %D%/services/messaging.scm			\
>> >> +  %D%/services/music.scm                        \
>> >>    %D%/services/networking.scm			\
>> >>    %D%/services/nfs.scm			\
>> >>    %D%/services/shepherd.scm			\
>> >> @@ -488,6 +489,7 @@ GNU_SYSTEM_MODULES
>> >> =				\
>> >> %D%/tests/install.scm				\
>> >> %D%/tests/mail.scm				\
>> >> %D%/tests/messaging.scm			\
>> >> +  %D%/tests/music.scm				\
>> >>    %D%/tests/networking.scm			\
>> >>    %D%/tests/ssh.scm				\
>> >>    %D%/tests/web.scm
>> >> diff --git a/gnu/services/music.scm b/gnu/services/music.scm
>> >> new file mode 100644
>> >> index 000000000..77912d5c6
>> >> --- /dev/null
>> >> +++ b/gnu/services/music.scm
>> >> @@ -0,0 +1,84 @@
>> >> +;;; GNU Guix --- Functional package management for GNU
>> >> +;;; Copyright © 2017 Peter Mikkelsen <[email-masked]>
>> >> +;;;
>> >> +;;; This file is part of GNU Guix.
>> >> +;;;
>> >> +;;; GNU Guix is free software; you can redistribute it and/or
>> >> modify it +;;; under the terms of the GNU General Public License as
>> >> published by +;;; the Free Software Foundation; either version 3 of
>> >> the License, or (at +;;; your option) any later version.
>> >> +;;;
>> >> +;;; GNU Guix is distributed in the hope that it will be useful,
>> >> but +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
>> >> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> >> +;;; GNU General Public License for more details.
>> >> +;;;
>> >> +;;; You should have received a copy of the GNU General Public
>> >> License +;;; along with GNU Guix.  If not, see
>> >> <http://www.gnu.org/licenses/>. +
>> >> +(define-module (gnu services music)
>> >> +  #:use-module (guix gexp)
>> >> +  #:use-module (gnu services)
>> >> +  #:use-module (gnu services shepherd)
>> >> +  #:use-module (gnu packages mpd)
>> >> +  #:use-module (guix records)
>> >> +  #:export (mpd-configuration
>> >> +            mpd-configuration?
>> >> +            mpd-service
>> >> +            mpd-service-type))
>> >> +
>> >> +;;; Commentary:
>> >> +;;;
>> >> +;;; Music related services
>> >> +;;;
>> >> +;;; Code:
>> >> +
>> >> +(define-record-type* <mpd-configuration>
>> >> +  mpd-configuration make-mpd-configuration
>> >> +  mpd-configuration?
>> >> +  (user         mpd-configuration-user
>> >> +                (default "mpd"))
>> >> +  (music-dir    mpd-configuration-music-dir
>> >> +                (default "~/Music"))
>> >> +  (playlist-dir mpd-configuration-playlist-dir
>> >> +                (default "~/.mpd/playlists"))
>> >> +  (port         mpd-configuration-port
>> >> +                (default "6600"))
>> >> +  (address      mpd-configuration-address
>> >> +                (default "any"))
>> >> +  (pid-file     mpd-configuration-pid-file
>> >> +                (default "~/.mpd-pid")))
>> >> +
>> >> +(define (mpd-config->file config)
>> >> +  (apply
>> >> +   mixed-text-file "mpd.conf"
>> >> +   "audio_output {\n"
>> >> +   "  type \"pulse\"\n"
>> >> +   "  name \"MPD\"\n"
>> >> +   "}\n"
>> >> +   (map (lambda (config-line)
>> >> +          (let ((config-name (car config-line))
>> >> +                (config-val (cadr config-line)))
>> >> +            (string-append config-name " \"" (config-val config)
>> >> "\"\n")))
>> >> +        `(("user" ,mpd-configuration-user)
>> >> +          ("music_directory" ,mpd-configuration-music-dir)
>> >> +          ("playlist_directory" ,mpd-configuration-playlist-dir)
>> >> +          ("port" ,mpd-configuration-port)
>> >> +          ("bind_to_address" ,mpd-configuration-address)
>> >> +          ("pid_file" ,mpd-configuration-pid-file)))))
>> >> +
>> >> +(define mpd-service-type
>> >> +  (shepherd-service-type
>> >> +   'mpd
>> >> +   (lambda (config)
>> >> +     (shepherd-service
>> >> +      (documentation "Run the MPD (Music Player Daemon)")
>> >> +      (provision '(mpd))
>> >> +      (start #~(make-forkexec-constructor
>> >> +                (list #$(file-append mpd "/bin/mpd")
>> >> +                      "--no-daemon"
>> >> +                      #$(mpd-config->file config))))
>> >> +      (stop  #~(make-kill-destructor))))))
>> >> +
>> >> +(define* (mpd-service #:optional (config (mpd-configuration)))
>> >> +  (service mpd-service-type config))
>> >
>> > I've been trying a slightly different style for this recently. At
>> > the moment, if you had a configuration file for MPD, you couldn't
>> > use this with the service here. One way of addressing this is to do
>> > something like the Tailon service, and define a gexp compiler for
>> > the configuration file (e.g. [1]). For the Tailon service, this
>> > means that you should be able to pass your own configuration file
>> > to the service.
>>
>> I am not sure I get how this works, and I would probs just make a big
>> mess. What about we take my approach first, and then I can come back
>> to it when I learn some more about guix?
>
> No problem. Looking at it again, I think it might be a bit trickier
> than I initially assumed, as the pid-file value is used.
>
Oh i see.
>> > Also, now that a <service-type> can have a default-value, I think
>> > its easier to just have the mpd-service-type, without the
>> > mpd-service procedure. If you add a default value, you should be
>> > able to do (service mpd-service-type).
>> >
>> Ok with me.
>>
>> > 1:
>> > https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/services/admin.scm#n255
>> >
>> >> diff --git a/gnu/tests/music.scm b/gnu/tests/music.scm
>> >> new file mode 100644
>> >> index 000000000..158513098
>> >> --- /dev/null
>> >> +++ b/gnu/tests/music.scm
>> >> @@ -0,0 +1,83 @@
>> >> +;;; GNU Guix --- Functional package management for GNU
>> >> +;;; Copyright © 2017 Peter Mikkelsen <[email-masked]>
>> >> +;;;
>> >> +;;; This file is part of GNU Guix.
>> >> +;;;
>> >> +;;; GNU Guix is free software; you can redistribute it and/or
>> >> modify it +;;; under the terms of the GNU General Public License as
>> >> published by +;;; the Free Software Foundation; either version 3 of
>> >> the License, or (at +;;; your option) any later version.
>> >> +;;;
>> >> +;;; GNU Guix is distributed in the hope that it will be useful,
>> >> but +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
>> >> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> >> +;;; GNU General Public License for more details.
>> >> +;;;
>> >> +;;; You should have received a copy of the GNU General Public
>> >> License +;;; along with GNU Guix.  If not, see
>> >> <http://www.gnu.org/licenses/>. +
>> >> +(define-module (gnu tests music)
>> >> +  #:use-module (gnu tests)
>> >> +  #:use-module (gnu system)
>> >> +  #:use-module (gnu system vm)
>> >> +  #:use-module (gnu services)
>> >> +  #:use-module (gnu services music)
>> >> +  #:use-module (gnu packages mpd)
>> >> +  #:use-module (guix gexp)
>> >> +  #:export (%test-mpd))
>> >> +
>> >> +(define %mpd-os
>> >> +  (simple-operating-system
>> >> +   (mpd-service (mpd-configuration
>> >> +                 (user "root")))))
>> >> +
>> >> +(define (run-mpd-test)
>> >> +  "Run tests in %mpd-os, which has mpd running."
>> >> +  (define os
>> >> +    (marionette-operating-system
>> >> +     %mpd-os
>> >> +     #:imported-modules '((gnu services herd))))
>> >> +
>> >> +  (define vm
>> >> +    (virtual-machine os))
>> >> +
>> >> +  (define test
>> >> +    (with-imported-modules '((gnu build marionette))
>> >> +      #~(begin
>> >> +          (use-modules (srfi srfi-64)
>> >> +                       (gnu build marionette))
>> >> +          (define marionette
>> >> +            (make-marionette (list #$vm)))
>> >> +
>> >> +          (mkdir #$output)
>> >> +          (chdir #$output)
>> >> +
>> >> +          (test-begin "mpd")
>> >> +
>> >> +          (test-eq "service is running"
>> >> +            'running!
>> >> +            (marionette-eval
>> >> +             '(begin
>> >> +                (use-modules (gnu services herd))
>> >> +                (start-service 'mpd)
>> >> +                'running!)
>> >> +             marionette))
>> >
>> > Recently, the start-service procedure was changed to return the
>> > information from the shepherd, and this can be used to make this
>> > check a bit more rigorous.
>> >
>> > I've got an patch for the Memcached service which demonstrates this
>> > here [2], as with the test above, it will not always fail, even if
>> > the service fails to create the PID file.
>> >
>> > 2: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28021
>> >
>> >> +          (test-assert "pid file"
>> >> +            (wait-for-file "/root/.mpd-pid"
>> >> +             marionette))
>> >
>> > If this is useful when using MPD, then I think it would be valuable
>> > to get the shepherd to wait for the PID file. I think you can do
>> > this by adding #:pid-file to the make-forkexec-constructor call.
>> >
>> > If you do this, them I'm not sure this test adds anything, as I
>> > think the start-service call would only return successfully when
>> > the service has started, and created the PID file.
>> >
>>
>> Right, I have removed this test.
>
> Great. Just to check I wasn't wrong, I've just tested what happens if
> you break the service by getting mpd to create the PID file, and
> shepherd to look for it in different places, and the previous test
> about starting the service does indeed fail, which is what we want :D
>

Great!
>> >> +          (test-assert "mpc connect"
>> >> +            (marionette-eval
>> >> +             '(zero? (system #$(file-append mpd-mpc "/bin/mpc")))
>> >> +             marionette))
>> >> +
>> >> +          (test-end)
>> >> +          (exit (= (test-runner-fail-count (test-runner-current))
>> >> 0)))))
>> >> +  (gexp->derivation "mpd-test" test))
>> >> +
>> >> +(define %test-mpd
>> >> +  (system-test
>> >> +   (name "mpd")
>> >> +   (description "Test that the mpd can run and be connected to.")
>> >> +   (value (run-mpd-test))))
>>
>> I think I have fixed all of your suggestions (apart from the
>> gexp-compiler one). Please see my new patch.
>
> Awesome, I've put a couple more suggestions below, but just on the docs
> and a bit of code style. Regardless of these, I think this is good to
> go.
>
>
>> From 419a8df59bc958ee87ece5519393b32cfbef609c Mon Sep 17 00:00:00 2001
>> From: Peter Mikkelsen <[email-masked]>
>> Date: Sat, 12 Aug 2017 03:40:25 +0200
>> Subject: [PATCH] gnu: Add mpd service.
>>
>> * doc/guix.texi: Add documentation.
>> * gnu/services/audio.scm (<mpd-configuration>): New record type.
>>   (mpd-service-type): New service type.
>> * gnu/tests/audio.scm: New file.
>> * gnu/local.mk: Add new files.
>> ---
>>  doc/guix.texi          | 49 ++++++++++++++++++++++++++++
>>  gnu/local.mk           |  2 ++
>>  gnu/services/audio.scm | 86
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> gnu/tests/audio.scm    | 78
>> +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 215
>> insertions(+) create mode 100644 gnu/services/audio.scm create mode
>> 100644 gnu/tests/audio.scm
>>
>> diff --git a/doc/guix.texi b/doc/guix.texi
>> index 8f14ddd50..754408ade 100644
>> --- a/doc/guix.texi
>> +++ b/doc/guix.texi
>> @@ -227,6 +227,7 @@ Services
>>  * Network File System::         NFS related services.
>>  * Continuous Integration::      The Cuirass service.
>>  * Power management Services::   The TLP tool.
>> +* Audio Services::              The MPD.
>>  * Miscellaneous Services::      Other services.
>>
>>  Defining Services
>> @@ -9035,6 +9036,7 @@ declaration.
>>  * Network File System::         NFS related services.
>>  * Continuous Integration::      The Cuirass service.
>>  * Power management Services::   The TLP tool.
>> +* Audio Services::              The MPD.
>>  * Miscellaneous Services::      Other services.
>>  @end menu
>>
>> @@ -15635,6 +15637,53 @@ Package object of thermald.
>>  @end table
>>  @end deftp
>>
>> +@node Audio Services
>> +@subsubsection Audio Services
>> +
>> +@cindex mpd
>> +@subsubheading Music Player Daemon
>> +
>> +The @code{(gnu services audio)} provides a service to start MPD (the
>> Music +Player Daemon).  It uses pulseaudio for output.
>> +
>> +@defvr {Scheme Variable} mpd-service-type
>> +The service type for @command{mpd}
>> +@end defvr
>> +
>> +@deftp {Data Type} mpd-configuration
>> +Data type representing the configuration of @command{mpd}.
>> +
>> +@table @asis
>> +@item @code{user} (default: @code{"mpd"})
>> +The user to run mpd as.
>> +
>> +@item @code{music-dir} (default: @code{"~/Music"})
>> +The directory to scan for music files.
>> +
>> +@item @code{playlist-dir} (default: @code{"~/.mpd/playlists"})
>> +The directory to store playlists.
>> +
>> +@item @code{pid-file} (default: @code{"/var/run/mpd.pid"})
>> +The file mpd wil store its PID.  This must be an absolute path.
>> +
>> +@item @code{port} (default: @code{"6600"})
>> +The port to run mpd on.
>> +
>> +@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.
>> +
>> +@end table
>> +@end deftp
>> +
>> +The following example shows how one might run @code{mpd} as user
>> +@code{"bob"} on port @code{6666}.
>> +@example
>> +(service mpd-service-type
>> +         (mpd-configuration
>> +          (user "bob")
>> +          (port "6666")))
>> +@end example
>
> I've got a few suggestions for the docs. Nothing too important, and I'm
> fine if you still prefer docs without the suggestions below, but feel
> free to pick and choose any changes that you think are good, I've put
> my reasoning inline in round brackets.
>
>
> @node Audio Services
> @subsubsection Audio Services
>
> The @code{(gnu services audio)} module provides a service to
> start MPD (the Music Player Daemon).
>
>   (
>   Having the introduction to the module above the Music Player Daemon
>   subsubheading seems neater. Also, I think adding "module" after
>   @code{(gnu services audio)} helps with readability.
>   )
>
> @cindex mpd
> @subsubheading Music Player Daemon
>
> The Music Player Daemon (MPD) is a service that can play music while
> being controlled from the local machine or over the network by a
> variety of clients.
>
>   (
>   An introductory paragraph about what the service does might be
>   useful, so I've written one above.
>   )
>
> The following example shows how one might run @code{mpd} as user
> @code{"bob"} on port @code{6666}.  It uses pulseaudio for output.
>
> @example
> (service mpd-service-type
>          (mpd-configuration
>           (user "bob")
>           (port "6666")))
> @end example
>
>   (
>   Moving the example here might be more visible, rather than below the
>   reference documentation.
>   )
>
> @defvr {Scheme Variable} mpd-service-type
> The service type for @command{mpd}
> @end defvr
>
> @deftp {Data Type} mpd-configuration
> Data type representing the configuration of @command{mpd}.
>
> ...
>
> @end table
> @end deftp
>
>
I like all your suggestions and they are all part of the new patch, thanks!
>>  @node Miscellaneous Services
>>  @subsubsection Miscellaneous Services
>> diff --git a/gnu/local.mk b/gnu/local.mk
>> index cffb18d3a..c12fd8559 100644
>> --- a/gnu/local.mk
>> +++ b/gnu/local.mk
>> @@ -426,6 +426,7 @@ GNU_SYSTEM_MODULES
>> =				\ \
>>    %D%/services.scm				\
>>    %D%/services/admin.scm			\
>> +  %D%/services/audio.scm                        \
>>    %D%/services/avahi.scm			\
>>    %D%/services/base.scm				\
>>    %D%/services/configuration.scm		\
>> @@ -481,6 +482,7 @@ GNU_SYSTEM_MODULES
>> =				\ \
>>    %D%/tests.scm					\
>>    %D%/tests/admin.scm				\
>> +  %D%/tests/audio.scm				\
>>    %D%/tests/base.scm				\
>>    %D%/tests/databases.scm			\
>>    %D%/tests/dict.scm				\
>> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
>> new file mode 100644
>> index 000000000..f5c465341
>> --- /dev/null
>> +++ b/gnu/services/audio.scm
>> @@ -0,0 +1,86 @@
>> +;;; GNU Guix --- Functional package management for GNU
>> +;;; Copyright © 2017 Peter Mikkelsen <[email-masked]>
>> +;;;
>> +;;; This file is part of GNU Guix.
>> +;;;
>> +;;; GNU Guix is free software; you can redistribute it and/or modify
>> it +;;; under the terms of the GNU General Public License as
>> published by +;;; the Free Software Foundation; either version 3 of
>> the License, or (at +;;; your option) any later version.
>> +;;;
>> +;;; GNU Guix is distributed in the hope that it will be useful, but
>> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
>> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +;;; GNU General Public License for more details.
>> +;;;
>> +;;; You should have received a copy of the GNU General Public License
>> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
>> +
>> +(define-module (gnu services audio)
>> +  #:use-module (guix gexp)
>> +  #:use-module (gnu services)
>> +  #:use-module (gnu services shepherd)
>> +  #:use-module (gnu packages mpd)
>> +  #:use-module (guix records)
>> +  #:export (mpd-configuration
>> +            mpd-configuration?
>> +            mpd-service-type))
>> +
>> +;;; Commentary:
>> +;;;
>> +;;; Audio related services
>> +;;;
>> +;;; Code:
>> +
>> +(define-record-type* <mpd-configuration>
>> +  mpd-configuration make-mpd-configuration
>> +  mpd-configuration?
>> +  (user         mpd-configuration-user
>> +                (default "mpd"))
>> +  (music-dir    mpd-configuration-music-dir
>> +                (default "~/Music"))
>> +  (playlist-dir mpd-configuration-playlist-dir
>> +                (default "~/.mpd/playlists"))
>> +  (port         mpd-configuration-port
>> +                (default "6600"))
>> +  (address      mpd-configuration-address
>> +                (default "any"))
>> +  (pid-file     mpd-configuration-pid-file
>> +                (default "/var/run/mpd.pid")))
>> +
>> +(define (mpd-config->file config)
>> +  (apply
>> +   mixed-text-file "mpd.conf"
>> +   "audio_output {\n"
>> +   "  type \"pulse\"\n"
>> +   "  name \"MPD\"\n"
>> +   "}\n"
>> +   (map (lambda (config-line)
>> +          (let ((config-name (car config-line))
>> +                (config-val (cadr config-line)))
>> +            (string-append config-name " \"" (config-val config)
>> "\"\n")))
>
> One way of making this a bit more concise and remove the need for car
> and cadr is to use the match module (ice-9 match).
>
>    (map (match-lambda
>          ((config-name config-val)
>           (string-append config-name " \"" (config-val config) "\"\n")))
>         ...
>
>

I didn't know about match-lambda, but it seems pretty neat, so I used
your example here, thanks.

>> +        `(("user" ,mpd-configuration-user)
>> +          ("music_directory" ,mpd-configuration-music-dir)
>> +          ("playlist_directory" ,mpd-configuration-playlist-dir)
>> +          ("port" ,mpd-configuration-port)
>> +          ("bind_to_address" ,mpd-configuration-address)
>> +          ("pid_file" ,mpd-configuration-pid-file)))))
>> +
>
> ...
>From 54850337156401f92b836580a8340c3e250ecc61 Mon Sep 17 00:00:00 2001
From: Peter Mikkelsen <[email-masked]>
Date: Sat, 12 Aug 2017 03:40:25 +0200
Subject: [PATCH] gnu: Add mpd service.

* doc/guix.texi: Add documentation.
* gnu/services/audio.scm (<mpd-configuration>): New record type.
  (mpd-service-type): New service type.
* gnu/tests/audio.scm: New file.
* gnu/local.mk: Add new files.
---
 doc/guix.texi          | 54 +++++++++++++++++++++++++++++++
 gnu/local.mk           |  2 ++
 gnu/services/audio.scm | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++
 gnu/tests/audio.scm    | 78 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 220 insertions(+)
 create mode 100644 gnu/services/audio.scm
 create mode 100644 gnu/tests/audio.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index 8f14ddd50..2f8a2efd0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -227,6 +227,7 @@ Services
 * Network File System::         NFS related services.
 * Continuous Integration::      The Cuirass service.
 * Power management Services::   The TLP tool.
+* Audio Services::              The MPD.
 * Miscellaneous Services::      Other services.
 
 Defining Services
@@ -9035,6 +9036,7 @@ declaration.
 * Network File System::         NFS related services.
 * Continuous Integration::      The Cuirass service.
 * Power management Services::   The TLP tool.
+* Audio Services::              The MPD.
 * Miscellaneous Services::      Other services.
 @end menu
 
@@ -15635,6 +15637,58 @@ Package object of thermald.
 @end table
 @end deftp
 
+@node Audio Services
+@subsubsection Audio Services
+
+The @code{(gnu services audio)} module provides a service to start MPD
+(the Music Player Daemon).
+
+@cindex mpd
+@subsubheading Music Player Daemon
+
+The Music Player Daemon (MPD) is a service that can play music while
+being controlled from the local machine or over the network by a variety
+of clients.
+
+The following example shows how one might run @code{mpd} as user
+@code{"bob"} on port @code{6666}.  It uses pulseaudio for output.
+
+@example
+(service mpd-service-type
+         (mpd-configuration
+          (user "bob")
+          (port "6666")))
+@end example
+
+@defvr {Scheme Variable} mpd-service-type
+The service type for @command{mpd}
+@end defvr
+
+@deftp {Data Type} mpd-configuration
+Data type representing the configuration of @command{mpd}.
+
+@table @asis
+@item @code{user} (default: @code{"mpd"})
+The user to run mpd as.
+
+@item @code{music-dir} (default: @code{"~/Music"})
+The directory to scan for music files.
+
+@item @code{playlist-dir} (default: @code{"~/.mpd/playlists"})
+The directory to store playlists.
+
+@item @code{pid-file} (default: @code{"/var/run/mpd.pid"})
+The file mpd wil store its PID.  This must be an absolute path.
+
+@item @code{port} (default: @code{"6600"})
+The port to run mpd on.
+
+@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.
+
+@end table
+@end deftp
 
 @node Miscellaneous Services
 @subsubsection Miscellaneous Services
diff --git a/gnu/local.mk b/gnu/local.mk
index cffb18d3a..c12fd8559 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -426,6 +426,7 @@ GNU_SYSTEM_MODULES =				\
 						\
   %D%/services.scm				\
   %D%/services/admin.scm			\
+  %D%/services/audio.scm                        \
   %D%/services/avahi.scm			\
   %D%/services/base.scm				\
   %D%/services/configuration.scm		\
@@ -481,6 +482,7 @@ GNU_SYSTEM_MODULES =				\
 						\
   %D%/tests.scm					\
   %D%/tests/admin.scm				\
+  %D%/tests/audio.scm				\
   %D%/tests/base.scm				\
   %D%/tests/databases.scm			\
   %D%/tests/dict.scm				\
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
new file mode 100644
index 000000000..22814a6c0
--- /dev/null
+++ b/gnu/services/audio.scm
@@ -0,0 +1,86 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2017 Peter Mikkelsen <[email-masked]>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services audio)
+  #:use-module (guix gexp)
+  #:use-module (gnu services)
+  #:use-module (gnu services shepherd)
+  #:use-module (gnu packages mpd)
+  #:use-module (guix records)
+  #:use-module (ice-9 match)
+  #:export (mpd-configuration
+            mpd-configuration?
+            mpd-service-type))
+
+;;; Commentary:
+;;;
+;;; Audio related services
+;;;
+;;; Code:
+
+(define-record-type* <mpd-configuration>
+  mpd-configuration make-mpd-configuration
+  mpd-configuration?
+  (user         mpd-configuration-user
+                (default "mpd"))
+  (music-dir    mpd-configuration-music-dir
+                (default "~/Music"))
+  (playlist-dir mpd-configuration-playlist-dir
+                (default "~/.mpd/playlists"))
+  (port         mpd-configuration-port
+                (default "6600"))
+  (address      mpd-configuration-address
+                (default "any"))
+  (pid-file     mpd-configuration-pid-file
+                (default "/var/run/mpd.pid")))
+
+(define (mpd-config->file config)
+  (apply
+   mixed-text-file "mpd.conf"
+   "audio_output {\n"
+   "  type \"pulse\"\n"
+   "  name \"MPD\"\n"
+   "}\n"
+   (map (match-lambda
+          ((config-name config-val)
+           (string-append config-name " \"" (config-val config) "\"\n")))
+        `(("user" ,mpd-configuration-user)
+          ("music_directory" ,mpd-configuration-music-dir)
+          ("playlist_directory" ,mpd-configuration-playlist-dir)
+          ("port" ,mpd-configuration-port)
+          ("bind_to_address" ,mpd-configuration-address)
+          ("pid_file" ,mpd-configuration-pid-file)))))
+
+(define (mpd-service config)
+  (shepherd-service
+   (documentation "Run the MPD (Music Player Daemon)")
+   (provision '(mpd))
+   (start #~(make-forkexec-constructor
+             (list #$(file-append mpd "/bin/mpd")
+                   "--no-daemon"
+                   #$(mpd-config->file config))
+             #:pid-file #$(mpd-configuration-pid-file config)))
+   (stop  #~(make-kill-destructor))))
+
+(define mpd-service-type
+  (service-type
+   (name 'mpd)
+   (extensions
+    (list (service-extension shepherd-root-service-type
+                             (compose list mpd-service))))
+   (default-value (mpd-configuration))))
diff --git a/gnu/tests/audio.scm b/gnu/tests/audio.scm
new file mode 100644
index 000000000..8eadaf02e
--- /dev/null
+++ b/gnu/tests/audio.scm
@@ -0,0 +1,78 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2017 Peter Mikkelsen <[email-masked]>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu tests audio)
+  #:use-module (gnu tests)
+  #:use-module (gnu system)
+  #:use-module (gnu system vm)
+  #:use-module (gnu services)
+  #:use-module (gnu services audio)
+  #:use-module (gnu packages mpd)
+  #:use-module (guix gexp)
+  #:export (%test-mpd))
+
+(define %mpd-os
+  (simple-operating-system
+   (service mpd-service-type
+            (mpd-configuration
+             (user "root")))))
+
+(define (run-mpd-test)
+  "Run tests in %mpd-os, which has mpd running."
+  (define os
+    (marionette-operating-system
+     %mpd-os
+     #:imported-modules '((gnu services herd))))
+
+  (define vm
+    (virtual-machine os))
+
+  (define test
+    (with-imported-modules '((gnu build marionette))
+      #~(begin
+          (use-modules (srfi srfi-64)
+                       (gnu build marionette))
+          (define marionette
+            (make-marionette (list #$vm)))
+
+          (mkdir #$output)
+          (chdir #$output)
+
+          (test-begin "mpd")
+
+          (test-assert "service is running"
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd))
+                (start-service 'mpd))
+             marionette))
+
+          (test-assert "mpc connect"
+            (marionette-eval
+             '(zero? (system #$(file-append mpd-mpc "/bin/mpc")))
+             marionette))
+
+          (test-end)
+          (exit (= (test-runner-fail-count (test-runner-current)) 0)))))
+  (gexp->derivation "mpd-test" test))
+
+(define %test-mpd
+  (system-test
+   (name "mpd")
+   (description "Test that the mpd can run and be connected to.")
+   (value (run-mpd-test))))
-- 
2.14.0