[bug#28208] [PATCH] services: mpd: remove the mpd-file field.

[bug#28208] [PATCH] services: mpd: remove the mpd-file field.

[Peter Mikkelsen]

Since MPD switches user, the pid-file must be in a writable dir.  This
is now always /var/run/mpd/(user)/pid.

* gnu/services/audio.scm (mpd-service-type): Add a activation-service
  extension.
* doc/guix.texi (Audio Services): Document the changes.
---
 doc/guix.texi          |  3 ---
 gnu/services/audio.scm | 35 +++++++++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 77435d897..5a3ba2652 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -15827,9 +15827,6 @@ 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.

diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 22814a6c0..66b23fcca 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -45,9 +45,7 @@
   (port         mpd-configuration-port
                 (default "6600"))
   (address      mpd-configuration-address
-                (default "any"))
-  (pid-file     mpd-configuration-pid-file
-                (default "/var/run/mpd.pid")))
+                (default "any")))

 (define (mpd-config->file config)
   (apply
@@ -56,6 +54,7 @@
    "  type \"pulse\"\n"
    "  name \"MPD\"\n"
    "}\n"
+   "pid_file \"" (mpd-path config "pid") "\"\n"
    (map (match-lambda
           ((config-name config-val)
            (string-append config-name " \"" (config-val config) "\"\n")))
@@ -63,10 +62,16 @@
           ("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)))))
+          ("bind_to_address" ,mpd-configuration-address)))))

-(define (mpd-service config)
+(define (mpd-path config file)
+  "Return a path in /var/run/mpd/ that is writable
+   by @code{user} from @code{config}."
+  (string-append "/var/run/mpd/"
+                 (mpd-configuration-user config)
+                 "/" file))
+
+(define (mpd-shepherd-service config)
   (shepherd-service
    (documentation "Run the MPD (Music Player Daemon)")
    (provision '(mpd))
@@ -74,13 +79,27 @@
              (list #$(file-append mpd "/bin/mpd")
                    "--no-daemon"
                    #$(mpd-config->file config))
-             #:pid-file #$(mpd-configuration-pid-file config)))
+             #:pid-file #$(mpd-path config "pid")
+             #:log-file #$(mpd-path config "log")))
    (stop  #~(make-kill-destructor))))

+(define (mpd-service-activation config)
+  (with-imported-modules '((guix build utils))
+    #~(begin
+        (use-modules (guix build utils))
+        (define %user
+          (getpw #$(mpd-configuration-user config)))
+
+        (let ((directory #$(mpd-path config "")))
+          (mkdir-p directory)
+          (chown directory (passwd:uid %user) (passwd:gid %user))))))
+
 (define mpd-service-type
   (service-type
    (name 'mpd)
    (extensions
     (list (service-extension shepherd-root-service-type
-                             (compose list mpd-service))))
+                             (compose list mpd-shepherd-service))
+          (service-extension activation-service-type
+                             mpd-service-activation)))
    (default-value (mpd-configuration))))
--
2.14.1

[bug#28208] [PATCH] services: mpd: remove the mpd-file field.

[Peter Mikkelsen]

[-- Attachment #1: Type: text/plain, Size: 4032 bytes --]

The commit message is supposed to say pid-file instead of mpd-file. Apart
from that, I wasn't sure about the rest of the commit message, as I have
not written alot of them.

Peter

Den 23. aug. 2017 18.39 skrev "Peter Mikkelsen" <petermikkelsen10@gmail.com
>:

> Since MPD switches user, the pid-file must be in a writable dir.  This
> is now always /var/run/mpd/(user)/pid.
>
> * gnu/services/audio.scm (mpd-service-type): Add a activation-service
>   extension.
> * doc/guix.texi (Audio Services): Document the changes.
> ---
>  doc/guix.texi          |  3 ---
>  gnu/services/audio.scm | 35 +++++++++++++++++++++++++++--------
>  2 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 77435d897..5a3ba2652 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -15827,9 +15827,6 @@ 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.
>
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index 22814a6c0..66b23fcca 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -45,9 +45,7 @@
>    (port         mpd-configuration-port
>                  (default "6600"))
>    (address      mpd-configuration-address
> -                (default "any"))
> -  (pid-file     mpd-configuration-pid-file
> -                (default "/var/run/mpd.pid")))
> +                (default "any")))
>
>  (define (mpd-config->file config)
>    (apply
> @@ -56,6 +54,7 @@
>     "  type \"pulse\"\n"
>     "  name \"MPD\"\n"
>     "}\n"
> +   "pid_file \"" (mpd-path config "pid") "\"\n"
>     (map (match-lambda
>            ((config-name config-val)
>             (string-append config-name " \"" (config-val config) "\"\n")))
> @@ -63,10 +62,16 @@
>            ("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)))))
> +          ("bind_to_address" ,mpd-configuration-address)))))
>
> -(define (mpd-service config)
> +(define (mpd-path config file)
> +  "Return a path in /var/run/mpd/ that is writable
> +   by @code{user} from @code{config}."
> +  (string-append "/var/run/mpd/"
> +                 (mpd-configuration-user config)
> +                 "/" file))
> +
> +(define (mpd-shepherd-service config)
>    (shepherd-service
>     (documentation "Run the MPD (Music Player Daemon)")
>     (provision '(mpd))
> @@ -74,13 +79,27 @@
>               (list #$(file-append mpd "/bin/mpd")
>                     "--no-daemon"
>                     #$(mpd-config->file config))
> -             #:pid-file #$(mpd-configuration-pid-file config)))
> +             #:pid-file #$(mpd-path config "pid")
> +             #:log-file #$(mpd-path config "log")))
>     (stop  #~(make-kill-destructor))))
>
> +(define (mpd-service-activation config)
> +  (with-imported-modules '((guix build utils))
> +    #~(begin
> +        (use-modules (guix build utils))
> +        (define %user
> +          (getpw #$(mpd-configuration-user config)))
> +
> +        (let ((directory #$(mpd-path config "")))
> +          (mkdir-p directory)
> +          (chown directory (passwd:uid %user) (passwd:gid %user))))))
> +
>  (define mpd-service-type
>    (service-type
>     (name 'mpd)
>     (extensions
>      (list (service-extension shepherd-root-service-type
> -                             (compose list mpd-service))))
> +                             (compose list mpd-shepherd-service))
> +          (service-extension activation-service-type
> +                             mpd-service-activation)))
>     (default-value (mpd-configuration))))
> --
> 2.14.1
>

[-- Attachment #2: Type: text/html, Size: 5348 bytes --]

bug#28208: [PATCH] services: mpd: remove the mpd-file field.

[Ludovic Courtès]

Hi Peter,

Peter Mikkelsen <petermikkelsen10@gmail.com> skribis:

> Since MPD switches user, the pid-file must be in a writable dir.  This
> is now always /var/run/mpd/(user)/pid.
>
> * gnu/services/audio.scm (mpd-service-type): Add a activation-service
>   extension.
> * doc/guix.texi (Audio Services): Document the changes.

Thanks, I’ve applied the patch.  I adjusted the commit log to reflect
all the changes made, and also renamed “mpd-path” to “mpd-file-name” (in
GNU “path” refers to “search paths” like $PATH):

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=b6dc69af1de806e8b3a70e9a1afe0e1590da5480

Thanks!

Ludo’.