Subject: [PATCH v3 08/16] services: mpd: Log to syslog by default.

From: Maxim Cournoyer <[email-masked]>
Date: Fri, 5 May 2023 14:29:03 -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.  Conditionally add syslogd to requirements.
(mympd-log-to-sanitizer): New procedure.
(mympd-configuration) [log-to]: Change type to maybe-string.  Update doc and
add sanitizer.
(mympd-shepherd-service) [requirement]: Fix to use syslogd.  Adjust
accordingly.
[start] Adjust accordingly.
(mympd-log-rotation): Check log-to via maybe-value-set?.
* doc/guix.texi (Audio Services): Update doc.
---
 doc/guix.texi          | 17 +++++-----
 gnu/services/audio.scm | 74 ++++++++++++++++++++++++++++--------------
 2 files changed, 57 insertions(+), 34 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 200f6d019c..253b8f113b 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33610,10 +33610,10 @@ 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, logs are sent to the
+local syslog daemon.  Alternatively, a log file name can be specified,
+for example @file{/var/log/mpd.log}.
 
 @item @code{log-level} (type: maybe-string)
 Supress any messages below this threshold.  The available values, in
@@ -33884,11 +33884,10 @@ Audio Services
 How much detail to include in logs, possible values: @code{0} to
 @code{7}.
 
-@item @code{log-to} (default: @code{"/var/log/mympd/log"}) (type: string-or-symbol)
-Where to send logs.  By default, the service logs to
-@file{/var/log/mympd.log}.  The alternative is @code{'syslog}, which
-sends output to the running syslog service under the @samp{daemon}
-facility.
+@item @code{log-to} (type: maybe-string)
+Where to send logs.  Unless specified, the service logs to the local
+syslog service under the @samp{daemon} facility.  Alternatively, a log
+file name can be specified, for example @file{/var/log/mympd.log}.
 
 @item @code{lualibs} (default: @code{"all"}) (type: maybe-string)
 See
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 6e57bf5cba..c1295837b6 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -229,6 +229,18 @@ (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 'log-file value))))
+
 ;;;
 
 ;; Generic MPD plugin record, lists only the most prevalent fields.
@@ -402,10 +414,11 @@ (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, logs are sent to the
+local syslog daemon.  Alternatively, a log file name can be specified, for
+example @file{/var/log/mpd.log}."
+   (sanitizer mpd-log-file-sanitizer))
 
   (log-level
    maybe-string
@@ -562,7 +575,11 @@ (define (mpd-shepherd-service config)
           (username (user-account-name user)))
       (shepherd-service
        (documentation "Run the MPD (Music Player Daemon)")
-       (requirement `(user-processes loopback ,@shepherd-requirement))
+       (requirement `(user-processes loopback
+                                     ,@(if (string=? "syslog" log-file)
+                                           '(syslogd)
+                                           '())
+                                     ,@shepherd-requirement))
        (provision '(mpd))
        (start
         (with-imported-modules (source-module-closure
@@ -683,6 +700,15 @@ (define (mympd-group-sanitizer value)
     (warning (G_ "'group' in <mympd-configuration> is obsolete; ignoring~%")))
   #f)
 
+(define (mympd-log-to-sanitizer value)
+  (match value
+    ('syslog
+     (warning (G_ "syslog symbol value for 'log-to' is deprecated~%"))
+     %unset-value)
+    ((or %unset-value (? string?))
+     value)
+    (_ (configuration-field-error #f 'log-to value))))
+
 ;;;

@@ -749,10 +775,11 @@ (define-configuration/no-serialization mympd-configuration
    "How much detail to include in logs, possible values: @code{0} to @code{7}.")
 
   (log-to
-   (string-or-symbol "/var/log/mympd/log")
-   "Where to send logs. By default, the service logs to
-@file{/var/log/mympd.log}. The alternative is @code{'syslog}, which
-sends output to the running syslog service under the @samp{daemon} facility."
+   maybe-string
+   "Where to send logs.  Unless specified, the service logs to the local
+syslog service under the @samp{daemon} facility.  Alternatively, a log file
+name can be specified, for example @file{/var/log/mympd.log}."
+   (sanitizer mympd-log-to-sanitizer)
    empty-serializer)
 
   (lualibs
@@ -849,9 +876,9 @@ (define (mympd-shepherd-service config)
     (shepherd-service
      (documentation "Run the myMPD daemon.")
      (requirement `(loopback user-processes
-                             ,@(if (eq? log-to 'syslog)
-                                   '(syslog)
-                                   '())
+                             ,@(if (maybe-value-set? log-to)
+                                   '()
+                                   '(syslogd))
                              ,@shepherd-requirement))
      (provision '(mympd))
      (start
@@ -867,16 +894,12 @@ (define (mympd-shepherd-service config)
                   (unless (file-exists? directory)
                     (mkdir-p/perms directory user #o755)))
 
-                (for-each
-                 init-directory
-                 '#$(map dirname
-                         ;; XXX: Delete the potential 'syslog log-file value,
-                         ;; which is not a directory.
-                         (delete 'syslog
-                                 (filter-map maybe-value
-                                             (list log-to
-                                                   work-directory
-                                                   cache-directory))))))
+                (for-each init-directory
+                          '#$(map dirname (filter-map maybe-value
+                                                      (list log-to
+                                                            work-directory
+                                                            cache-directory)))))
+
               (make-forkexec-constructor
                `(#$(file-append package "/bin/mympd")
                  "--user" #$username
@@ -885,7 +908,7 @@ (define (mympd-shepherd-service config)
                  "--cachedir" #$cache-directory)
                #:environment-variables
                (list #$(format #f "MYMPD_LOGLEVEL=~a" log-level))
-               #:log-file #$(if (string? log-to) log-to #f)))))))))
+               #:log-file #$(maybe-value log-to)))))))))
 
 (define (mympd-accounts config)
   (match-record config <mympd-configuration>
@@ -893,8 +916,9 @@ (define (mympd-accounts config)
     (list user)))
 
 (define (mympd-log-rotation config)
-  (match-record config <mympd-configuration> (log-to)
-    (if (string? log-to)
+  (match-record config <mympd-configuration>
+    (log-to)
+    (if (maybe-value-set? log-to)
         (list (log-rotation
                (files (list log-to))))
         '())))
-- 
2.39.2