From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50928) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dgU6M-0000ZN-C6 for guix-patches@gnu.org; Sat, 12 Aug 2017 06:57:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dgU6I-0000Vw-Ce for guix-patches@gnu.org; Sat, 12 Aug 2017 06:57:06 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:47622) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dgU6I-0000Vk-74 for guix-patches@gnu.org; Sat, 12 Aug 2017 06:57:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1dgU6H-0003aU-PH for guix-patches@gnu.org; Sat, 12 Aug 2017 06:57:01 -0400 Subject: [bug#28059] [PATCH] gnu: Add mpd service. Resent-Message-ID: Date: Sat, 12 Aug 2017 10:56:37 +0000 From: ng0 Message-ID: <20170812105637.xzh5ri245i7oxoqb@abyayala> References: <20170812015211.5411-1-petermikkelsen10@gmail.com> <20170812115011.4c076108@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="hl5ldumazpjt3qfi" Content-Disposition: inline In-Reply-To: <20170812115011.4c076108@cbaines.net> 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: Christopher Baines Cc: 28059@debbugs.gnu.org --hl5ldumazpjt3qfi Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Christopher Baines transcribed 13K bytes: > Hey, >=20 > This looks great Peter, awesome job :) I've made some notes about > potential improvements inline below. >=20 > I've succeeded in running the system test locally, but there was some > suspicious output in the log: >=20 > 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. >=20 > On Sat, 12 Aug 2017 03:52:11 +0200 > Peter Mikkelsen wrote: > > * doc/guix.text: Add documentation. >=20 > Typo above, text rather than texi. >=20 > > * 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 >=20 > 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... >=20 > > +@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. >=20 > The style for Guix is to use two spaces between sentences, I always > forget about this too. >=20 > > + > > +@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)) >=20 > 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. >=20 > 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). >=20 > 1: > https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/services/admin.scm#n2= 55 >=20 > > 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)) >=20 > 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. >=20 > 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. >=20 > 2: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D28021 >=20 > > + (test-assert "pid file" > > + (wait-for-file "/root/.mpd-pid" > > + marionette)) >=20 > 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. >=20 > 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. >=20 > > + (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)))) --=20 ng0 GnuPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588 GnuPG: https://n0is.noblogs.org/my-keys https://www.infotropique.org https://krosos.org --hl5ldumazpjt3qfi Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEqIyK3RKYKNfqwC5S4i+bv+40hYgFAlmO3uUACgkQ4i+bv+40 hYhsbg//a68v+UmHV5a+f7yePp9lK00rNZ2Vg/TDcibMd9PyHPTMCL4Z+iAvtklQ EbRzpo/7MnvWygfYfvoZ1HmrsT4eTb1zvSj9KBU9vV4Ur2BSNT2U/zifiYxrKmGd YZVPgDFIcxYPp6zNHyIPtbeCBUT9Gx5uE9vpH6HIl+vGPB+6svQ50CqNNByW+4Ko xxplJG6xcqMPpJCJrC5+i245IdqPw2gaI3E7ffXADKJk/h9ygYroTALIwkD0EQ5W dyvNL0W8eLp2RIZEh97nSDaxF9FAssXgCxNofuXwcLLzEufgD15dvSfQh+uITec2 62epvnSNuCA4OAnjgblqGAUuyY3FGTYIQALmXgWLtrNGwBPku5DwQKZxtIslHSNe WvYcKtwe6qeJDjYqeLysqyBkZgf7QU3172rGe8WP2saUD7XQ6dK/4l9yrRDg2pqf WkqbnrtYeuy1IO1+ZHdakvyjaDmCouGjI/WB0BT5zkKKvVygJeSLAmh/6FDVxg+C BLhMiE6VJ6pIQnycMT6HVUL4mjw+noWghHPItMaBEZ5FHhfSJQppySsBN29CRGwo VjloikhURYqNHVSOftmxjyrFg2oFzH3qujGuK69UEpqE93I+p39Ua2uDzgj1NVC7 OW/KsjdhtLPl7TNWAErkVKKSfw+39ebk34xa1fzw5lNGqSqjc0U= =Gott -----END PGP SIGNATURE----- --hl5ldumazpjt3qfi--