Subject: bug#63082: [PATCH 07/17] services: mpd: Log to syslog by default.

From: Maxim Cournoyer <[email-masked]>
Date: Fri, 28 Apr 2023 10:27:00 -0400

Rationale: the tristate value was awkward to deal with, the default log file
name was odd (/var/log/mpd/log) and it required special attention to create
the 'mpd' parent directory as root and chowning it to the MPD user.  It also
didn't match the default behavior of MPD, which is to log to systemd or syslog
unless a log file is specified.

* gnu/services/audio.scm (mpd-log-file-sanitizer): New procedure.
(mpd-configuration) [log-file]: Remove default maybe value.  Add sanitizer.
(mpd-shepherd-service): Validate the log file parent directory exists and has
the right permissions.
* doc/guix.texi (Audio Services): Update doc.
---
 doc/guix.texi          |  7 +++----
 gnu/services/audio.scm | 29 ++++++++++++++++++++---------
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 1aa8dc2809..a71a05bcf3 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33581,10 +33581,9 @@ Audio Services
 @item @code{environment-variables} (default: @code{("PULSE_CLIENTCONFIG=/etc/pulse/client.conf" "PULSE_CONFIG=/etc/pulse/daemon.conf")}) (type: list-of-strings)
 A list of strings specifying environment variables.
 
-@item @code{log-file} (default: @code{"/var/log/mpd/log"}) (type: maybe-string)
-The location of the log file.  Set to @code{syslog} to use the local
-syslog daemon or @code{%unset-value} to omit this directive from the
-configuration file.
+@item @code{log-file} (type: maybe-string)
+The location of the log file.  Unless specified, the logs are collected
+by the local syslog daemon.
 
 @item @code{log-level} (type: maybe-string)
 Supress any messages below this threshold.  The available values, in
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 0b7a25d9ef..a1d1a3d2fe 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -228,7 +228,17 @@ (define (mpd-group-sanitizer value)
     (warning (G_ "'group' in <mpd-configuration> is obsolete; ignoring~%")))
   #f)
 
-;;;
+(define (mpd-log-file-sanitizer value)
+  (match value
+    (%unset-value
+     ;; XXX: While leaving the 'sys_log' option out of the mpd.conf file is
+     ;; supposed to cause logging to happen via systemd (elogind provides a
+     ;; compatible interface), this doesn't work (nothing gets logged); use
+     ;; syslog instead.
+     "syslog")
+    ((? string?)
+     value)
+    (_ (configuration-field-error #f 'user value))))
 
 ;; Generic MPD plugin record, lists only the most prevalent fields.
 (define-configuration mpd-plugin
@@ -401,10 +411,10 @@ (define-configuration mpd-configuration
    empty-serializer)
 
   (log-file
-   (maybe-string "/var/log/mpd/log")
-   "The location of the log file. Set to @code{syslog} to use the
-local syslog daemon or @code{%unset-value} to omit this directive
-from the configuration file.")
+   maybe-string
+   "The location of the log file.  Unless specified, the logs are collected by
+the local syslog daemon."
+   (sanitizer mpd-log-file-sanitizer))
 
   (log-level
    maybe-string
@@ -563,17 +573,18 @@ (define (mpd-shepherd-service config)
        (requirement `(user-processes loopback ,@shepherd-requirement))
        (provision '(mpd))
        (start #~(begin
-                  (and=> #$(maybe-value log-file)
-                         (compose mkdir-p dirname))
-
                   (let ((user (getpw #$username)))
                     (for-each
                      (lambda (x)
-                       (when (and x (not (file-exists? x)))
+                       ;; Take action on absolute file names, to filter out
+                       ;; the 'syslog' special value.
+                       (when (and x (string-prefix? "/" x)
+                                  (not (file-exists? x)))
                          (mkdir-p x)
                          (chown x (passwd:uid user) (passwd:gid user))))
                      (list #$(maybe-value playlist-directory)
                            (and=> #$(maybe-value db-file) dirname)
+                           (and=> #$(maybe-value log-file) dirname)
                            (and=> #$(maybe-value state-file) dirname)
                            (and=> #$(maybe-value sticker-file) dirname))))
 
-- 
2.39.2