From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49933) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dgU0Z-0006R0-9Q for guix-patches@gnu.org; Sat, 12 Aug 2017 06:51:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dgU0U-0004Ck-Af for guix-patches@gnu.org; Sat, 12 Aug 2017 06:51:07 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:47618) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dgU0U-0004CR-68 for guix-patches@gnu.org; Sat, 12 Aug 2017 06:51:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dgU0T-0003Xz-LX for guix-patches@gnu.org; Sat, 12 Aug 2017 06:51:01 -0400 Subject: [bug#28059] [PATCH] gnu: Add mpd service. Resent-Message-ID: Date: Sat, 12 Aug 2017 11:50:11 +0100 From: Christopher Baines Message-ID: <20170812115011.4c076108@cbaines.net> In-Reply-To: <20170812015211.5411-1-petermikkelsen10@gmail.com> References: <20170812015211.5411-1-petermikkelsen10@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; boundary="Sig_/afMO+hGIgha25Hk2KS4XiSW"; protocol="application/pgp-signature" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Peter Mikkelsen Cc: 28059@debbugs.gnu.org --Sig_/afMO+hGIgha25Hk2KS4XiSW Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 wrote: > * doc/guix.text: Add documentation. Typo above, text rather than texi. > * gnu/services/music.scm (): 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. >=20 > --- > 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 >=20 > 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. > =20 > 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 > =20 > @@ -15635,6 +15637,57 @@ Package object of thermald. > @end table > @end deftp > =20 > +@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{} 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 > =20 > @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 > =3D \ > %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 > =3D \ > %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 =C2=A9 2017 Peter Mikkelsen > +;;; > +;;; 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 . > + > +(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 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 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 =C2=A9 2017 Peter Mikkelsen > +;;; > +;;; 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 . > + > +(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=3D28021 > + (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 (=3D (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)))) --Sig_/afMO+hGIgha25Hk2KS4XiSW Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlmO3WNfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE 9XccJA//d0rxgDfJa2ZQMaebLVS7ypfGadwibnx3ksTWi+xS1OO2ZkybQc/cWaFW HRlSTWvvVue9yjNqdXu4tMBa2XTR43PGmXrZjVi10zJBlZL988n7Abj5Y0jp7c+y f7kc5KHGOEH/CbTNqcKaxxX774z2ADh5lHIXARyLwnnw6nutQqWRWucgD3rvt9wX eGM7Q3YGZdMjojFXspqxocfi67n1PF5FyrqGWPwVFsu9quX+hhBUVL0v9eA58IV2 xrJLNGfLh2EmZqfneWKymFI0Bd0G7dFxJDfr3xzRX5n/IzQfyLc6pCgYJud/ZgLe JRGOzg2LA8/HZZknFOP4kGcCu+z9ykcxcY3IpkFQIqYUroHJD7I/SwLBAoulpDbK tK0wbdQx3edhhtTja6YH35bZzz3EM2Q2BGcKFtIni3Qnrsp65ZDHXazaBv5q+fOS 88g6WHUB3pjXVhCmsvmCKcO9MbuNRA6DTE88LyKqZi7o4g+XbpSLNMw0sBQNCPNQ 4xdg+9tzghoR9kqE8CgHCM1KXjMiEgUvqqhqB+BxZZ2FRtpR6ZRm4+fQ8uTMbPaj YPLZJZeqByHlvgQGM+bHyE4Bzj1n3cesLxO6HVhAKql4YHqfArXzyc3MkAb8lnmw jUp0xxNBlz+LagtQrTX7+CBQ8phCvVhrb1Izxw3B4bCT4Y/8MII= =ojeq -----END PGP SIGNATURE----- --Sig_/afMO+hGIgha25Hk2KS4XiSW--