* [bug#28059] [PATCH] gnu: Add mpd service.
@ 2017-08-12 1:52 Peter Mikkelsen
2017-08-12 10:50 ` Christopher Baines
0 siblings, 1 reply; 7+ messages in thread
From: Peter Mikkelsen @ 2017-08-12 1:52 UTC (permalink / raw)
To: 28059
* doc/guix.text: Add documentation.
* 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
+
+@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.
+
+@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 <petermikkelsen10@gmail.com>
+;;;
+;;; 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))
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 <petermikkelsen10@gmail.com>
+;;;
+;;; 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))
+
+ (test-assert "pid file"
+ (wait-for-file "/root/.mpd-pid"
+ 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [bug#28059] [PATCH] gnu: Add mpd service.
2017-08-12 1:52 [bug#28059] [PATCH] gnu: Add mpd service Peter Mikkelsen
@ 2017-08-12 10:50 ` Christopher Baines
2017-08-12 10:56 ` ng0
2017-08-12 17:10 ` Peter Mikkelsen
0 siblings, 2 replies; 7+ messages in thread
From: Christopher Baines @ 2017-08-12 10:50 UTC (permalink / raw)
To: Peter Mikkelsen; +Cc: 28059
[-- Attachment #1: Type: text/plain, Size: 12801 bytes --]
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
On Sat, 12 Aug 2017 03:52:11 +0200
Peter Mikkelsen <petermikkelsen10@gmail.com> wrote:
> * doc/guix.text: Add documentation.
Typo above, text rather than texi.
> * 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...
> +@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.
> +
> +@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 <petermikkelsen10@gmail.com>
> +;;;
> +;;; 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.
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).
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 <petermikkelsen10@gmail.com>
> +;;;
> +;;; 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.
> + (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))))
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [bug#28059] [PATCH] gnu: Add mpd service.
2017-08-12 10:50 ` Christopher Baines
@ 2017-08-12 10:56 ` ng0
2017-08-12 17:10 ` Peter Mikkelsen
1 sibling, 0 replies; 7+ messages in thread
From: ng0 @ 2017-08-12 10:56 UTC (permalink / raw)
To: Christopher Baines; +Cc: 28059
[-- Attachment #1: Type: text/plain, Size: 13880 bytes --]
Christopher Baines transcribed 13K bytes:
> 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
Isn't this normal for MPD? I haven't used any system service for it
in a very long time and starting mpd as a user process always gives
me a message like this.
>
> On Sat, 12 Aug 2017 03:52:11 +0200
> Peter Mikkelsen <petermikkelsen10@gmail.com> wrote:
> > * doc/guix.text: Add documentation.
>
> Typo above, text rather than texi.
>
> > * 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...
>
> > +@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.
>
> > +
> > +@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 <petermikkelsen10@gmail.com>
> > +;;;
> > +;;; 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.
>
> 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).
>
> 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 <petermikkelsen10@gmail.com>
> > +;;;
> > +;;; 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.
>
> > + (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))))
--
ng0
GnuPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588
GnuPG: https://n0is.noblogs.org/my-keys
https://www.infotropique.org https://krosos.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [bug#28059] [PATCH] gnu: Add mpd service.
2017-08-12 10:50 ` Christopher Baines
2017-08-12 10:56 ` ng0
@ 2017-08-12 17:10 ` Peter Mikkelsen
2017-08-12 20:58 ` Christopher Baines
1 sibling, 1 reply; 7+ messages in thread
From: Peter Mikkelsen @ 2017-08-12 17:10 UTC (permalink / raw)
To: Christopher Baines; +Cc: 28059
[-- Attachment #1: Type: text/plain, Size: 13986 bytes --]
Hi, thanks for the quick review!
Christopher Baines <mail@cbaines.net> 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.
>
> On Sat, 12 Aug 2017 03:52:11 +0200
> Peter Mikkelsen <petermikkelsen10@gmail.com> 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 <petermikkelsen10@gmail.com>
>> +;;;
>> +;;; 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?
> 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 <petermikkelsen10@gmail.com>
>> +;;;
>> +;;; 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.
>> + (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.
[-- Attachment #2: 0001-gnu-Add-mpd-service.patch --]
[-- Type: text/x-patch, Size: 9799 bytes --]
From 419a8df59bc958ee87ece5519393b32cfbef609c Mon Sep 17 00:00:00 2001
From: Peter Mikkelsen <petermikkelsen10@gmail.com>
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
@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 <petermikkelsen10@gmail.com>
+;;;
+;;; 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")))
+ `(("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 <petermikkelsen10@gmail.com>
+;;;
+;;; 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [bug#28059] [PATCH] gnu: Add mpd service.
2017-08-12 17:10 ` Peter Mikkelsen
@ 2017-08-12 20:58 ` Christopher Baines
2017-08-12 22:04 ` Peter Mikkelsen
0 siblings, 1 reply; 7+ messages in thread
From: Christopher Baines @ 2017-08-12 20:58 UTC (permalink / raw)
To: Peter Mikkelsen; +Cc: 28059
[-- Attachment #1: Type: text/plain, Size: 24057 bytes --]
On Sat, 12 Aug 2017 19:10:08 +0200
Peter Mikkelsen <petermikkelsen10@gmail.com> wrote:
> Hi, thanks for the quick review!
>
> Christopher Baines <mail@cbaines.net> 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 <petermikkelsen10@gmail.com> 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 <petermikkelsen10@gmail.com>
> >> +;;;
> >> +;;; 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.
> > 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 <petermikkelsen10@gmail.com>
> >> +;;;
> >> +;;; 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
> >> + (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 <petermikkelsen10@gmail.com>
> 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
> @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 <petermikkelsen10@gmail.com>
> +;;;
> +;;; 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")))
...
> + `(("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)))))
> +
...
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [bug#28059] [PATCH] gnu: Add mpd service.
2017-08-12 20:58 ` Christopher Baines
@ 2017-08-12 22:04 ` Peter Mikkelsen
2017-08-13 7:15 ` bug#28059: " Christopher Baines
0 siblings, 1 reply; 7+ messages in thread
From: Peter Mikkelsen @ 2017-08-12 22:04 UTC (permalink / raw)
To: Christopher Baines; +Cc: 28059
[-- Attachment #1: Type: text/plain, Size: 24980 bytes --]
Christopher Baines <mail@cbaines.net> writes:
> On Sat, 12 Aug 2017 19:10:08 +0200
> Peter Mikkelsen <petermikkelsen10@gmail.com> wrote:
>
>> Hi, thanks for the quick review!
>>
>> Christopher Baines <mail@cbaines.net> 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 <petermikkelsen10@gmail.com> 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 <petermikkelsen10@gmail.com>
>> >> +;;;
>> >> +;;; 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 <petermikkelsen10@gmail.com>
>> >> +;;;
>> >> +;;; 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 <petermikkelsen10@gmail.com>
>> 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 <petermikkelsen10@gmail.com>
>> +;;;
>> +;;; 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)))))
>> +
>
> ...
[-- Attachment #2: 0001-gnu-Add-mpd-service.patch --]
[-- Type: text/x-patch, Size: 9934 bytes --]
From 54850337156401f92b836580a8340c3e250ecc61 Mon Sep 17 00:00:00 2001
From: Peter Mikkelsen <petermikkelsen10@gmail.com>
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 <petermikkelsen10@gmail.com>
+;;;
+;;; 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 <petermikkelsen10@gmail.com>
+;;;
+;;; 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#28059: [PATCH] gnu: Add mpd service.
2017-08-12 22:04 ` Peter Mikkelsen
@ 2017-08-13 7:15 ` Christopher Baines
0 siblings, 0 replies; 7+ messages in thread
From: Christopher Baines @ 2017-08-13 7:15 UTC (permalink / raw)
To: Peter Mikkelsen; +Cc: 28059-done
[-- Attachment #1: Type: text/plain, Size: 26567 bytes --]
On Sun, 13 Aug 2017 00:04:04 +0200
Peter Mikkelsen <petermikkelsen10@gmail.com> wrote:
> Christopher Baines <mail@cbaines.net> writes:
>
> > On Sat, 12 Aug 2017 19:10:08 +0200
> > Peter Mikkelsen <petermikkelsen10@gmail.com> wrote:
> >
> >> Hi, thanks for the quick review!
> >>
> >> Christopher Baines <mail@cbaines.net> 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 <petermikkelsen10@gmail.com> 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
> >> >> <petermikkelsen10@gmail.com> +;;;
> >> >> +;;; 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
> >> >> <petermikkelsen10@gmail.com> +;;;
> >> >> +;;; 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 <petermikkelsen10@gmail.com>
> >> 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 <petermikkelsen10@gmail.com>
> >> +;;;
> >> +;;; 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)))))
> >> +
> >
> > ...
Awesome, I've now pushed this to master :)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-08-13 7:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-12 1:52 [bug#28059] [PATCH] gnu: Add mpd service Peter Mikkelsen
2017-08-12 10:50 ` Christopher Baines
2017-08-12 10:56 ` ng0
2017-08-12 17:10 ` Peter Mikkelsen
2017-08-12 20:58 ` Christopher Baines
2017-08-12 22:04 ` Peter Mikkelsen
2017-08-13 7:15 ` bug#28059: " Christopher Baines
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).