[bug#67842] [PATCH 0/4] Refactor mympd service.

[bug#67842] [PATCH 0/4] Refactor mympd service.

[Bruno Victal]

Cleanups to mympd-service-type.
Notable changes:
* Removed revert leftover in the manual.
* Simplified mympd system test.
* Fixed mympd syslog logging and missing service destructor.
* Refactored mympd serialization process.

Bruno Victal (4):
  doc: Remove missed hunk of revert commit for MPD service.
  tests: mympd: Simplify test.
  services: mympd: Fix syslog logging and missing service destructor.
  services: mympd: Refactor serialization process.

 doc/guix.texi          |  15 +---
 gnu/services/audio.scm | 166 ++++++++++++++++++++---------------------
 gnu/tests/audio.scm    |  35 +++++----
 3 files changed, 102 insertions(+), 114 deletions(-)


base-commit: 93597fc39cbe2d24b41f4054c9656c2dedeabacf
-- 
2.41.0

[bug#67842] [PATCH 1/4] doc: Remove missed hunk of revert commit for MPD service.

[Bruno Victal]

Introduced with e1070ee16036f6dfb84c44aea4119e4db770356b and missed in the
c7e45139faa27b60f2c7d0a4bc140f9793d97d47 revert.

* doc/guix.texi (Audio Services): Remove missed hunk of revert commit
c7e45139faa27b60f2c7d0a4bc140f9793d97d47.

Change-Id: Ibbf0fa4e6a3a378d2981f03ffa5d1ca9c0e3f797
---
 doc/guix.texi | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index e61a893af9..3659d57361 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -34718,13 +34718,6 @@ Audio Services
 by default.
 @end quotation
 
-Most MPD clients will trigger a database update upon connecting, but you
-can also use the @code{update} action do to so:
-
-@example
-herd update mpd
-@end example
-
 All the MPD configuration fields are documented below, and a more
 complex example follows.
 
-- 
2.41.0

[bug#67842] [PATCH 2/4] tests: mympd: Simplify test.

[Bruno Victal]

* gnu/tests/audio.scm: (run-mympd-test): Restyle.
Remove dhcp-client-service-type. Remove port-forwards and refactor http-head
test to happen within the VM instead of the host.
---
 gnu/tests/audio.scm | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/gnu/tests/audio.scm b/gnu/tests/audio.scm
index acb91293e8..a0ab54da2a 100644
--- a/gnu/tests/audio.scm
+++ b/gnu/tests/audio.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017 Peter Mikkelsen <petermikkelsen10@gmail.com>
-;;; Copyright © 2022 Bruno Victal <mirai@makinata.eu>
+;;; Copyright © 2022⁠–⁠2023 Bruno Victal <mirai@makinata.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -81,23 +81,17 @@ (define %test-mpd
    (value (run-mpd-test))))
 
 (define (run-mympd-test)
-  (define os (marionette-operating-system
-              (simple-operating-system (service dhcp-client-service-type)
-                                       (service mympd-service-type))
-              #:imported-modules '((gnu services herd))))
+  (define os
+    (marionette-operating-system
+     (simple-operating-system (service mympd-service-type))
+     #:imported-modules '((gnu services herd))))
 
-  (define vm
-    (virtual-machine
-     (operating-system os)
-     (port-forwardings '((8080 . 80)))))
+  (define vm (virtual-machine os))
 
   (define test
     (with-imported-modules '((gnu build marionette))
       #~(begin
           (use-modules (srfi srfi-64)
-                       (srfi srfi-8)
-                       (web client)
-                       (web response)
                        (gnu build marionette))
 
           (define marionette
@@ -106,18 +100,23 @@ (define (run-mympd-test)
           (test-runner-current (system-test-runner #$output))
           (test-begin "mympd")
           (test-assert "service is running"
-            (marionette-eval '(begin
-                                (use-modules (gnu services herd))
-
-                                (start-service 'mympd))
-                             marionette))
+            (marionette-eval
+             '(begin
+                (use-modules (gnu services herd))
+                (start-service 'mympd))
+             marionette))
 
           (test-assert "HTTP port ready"
             (wait-for-tcp-port 80 marionette))
 
           (test-equal "http-head"
             200
-            (receive (x _) (http-head "http://localhost:8080") (response-code x)))
+            (marionette-eval
+             '(begin
+                (use-modules (web client)
+                             (web response))
+                (response-code (http-head "http://localhost")))
+             marionette))
 
           (test-end))))
   (gexp->derivation "mympd-test" test))
-- 
2.41.0

[bug#67842] [PATCH 3/4] services: mympd: Fix syslog logging and missing service destructor.

[Bruno Victal]

* gnu/services/audio.scm: (mympd-shepherd-service): Fix syslog logging and
missing service destructor. Prefer list over quasiquote.
---
 gnu/services/audio.scm | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index ae991ced4d..4fcfcc13ea 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -938,14 +938,15 @@ (define (mympd-shepherd-service config)
                                                             cache-directory)))))
 
               (make-forkexec-constructor
-               `(#$(file-append package "/bin/mympd")
-                 "--user" #$username
-                 #$@(if (eq? log-to 'syslog) '("--syslog") '())
-                 "--workdir" #$work-directory
-                 "--cachedir" #$cache-directory)
+               (list #$(file-append package "/bin/mympd")
+                     "--user" #$username
+                     #$@(if (maybe-value-set? log-to) '() '("--syslog"))
+                     "--workdir" #$work-directory
+                     "--cachedir" #$cache-directory)
                #:environment-variables
                (list #$(format #f "MYMPD_LOGLEVEL=~a" log-level))
-               #:log-file #$(maybe-value log-to)))))))))
+               #:log-file #$(maybe-value log-to))))))
+     (stop #~(make-kill-destructor)))))
 
 (define (mympd-accounts config)
   (match-record config <mympd-configuration> (user group)
-- 
2.41.0

[bug#67842] [PATCH 4/4] services: mympd: Refactor serialization process.

[Bruno Victal]

* gnu/services/audio.scm: (string-or-symbol?): Remove unused predicate.
<mympd-configuration>[acl, covercache-ttl, http?, host, log-to, uri,
script-acl, ssl?, ssl-port, ssl-cert, ssl-key, pin-hash, save-caches?]: Pass
file-names via custom serializer procedure.
[port, log-level, ssl-port]: Use exact-integer (resp. maybe-exact-integer).
(mympd-field-serializer): New procedure, extracted from …
(mympd-serialize-configuration): … this. Refactor and rename it to
(mympd-configuration->files): … this.
(mympd-log-rotation): Restyle.
(mympd-service-type): Adjust service-extension to renamed procedure.
Use @acronym for description.
* doc/guix.texi: Update it.
---
 doc/guix.texi          |   8 +--
 gnu/services/audio.scm | 153 ++++++++++++++++++++---------------------
 2 files changed, 78 insertions(+), 83 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 3659d57361..b02a2ad498 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -35013,7 +35013,7 @@ Audio Services
 @item @code{acl} (type: maybe-mympd-ip-acl)
 ACL to access the myMPD webserver.
 
-@item @code{covercache-ttl} (default: @code{31}) (type: maybe-integer)
+@item @code{covercache-ttl} (default: @code{31}) (type: maybe-exact-integer)
 How long to keep cached covers, @code{0} disables cover caching.
 
 @item @code{http?} (default: @code{#t}) (type: boolean)
@@ -35022,10 +35022,10 @@ Audio Services
 @item @code{host} (default: @code{"[::]"}) (type: string)
 Host name to listen on.
 
-@item @code{port} (default: @code{80}) (type: maybe-port)
+@item @code{port} (default: @code{80}) (type: maybe-exact-integer)
 HTTP port to listen on.
 
-@item @code{log-level} (default: @code{5}) (type: integer)
+@item @code{log-level} (default: @code{5}) (type: exact-integer)
 How much detail to include in logs, possible values: @code{0} to
 @code{7}.
 
@@ -35048,7 +35048,7 @@ Audio Services
 @item @code{ssl?} (default: @code{#f}) (type: boolean)
 SSL/TLS support.
 
-@item @code{ssl-port} (default: @code{443}) (type: maybe-port)
+@item @code{ssl-port} (default: @code{443}) (type: maybe-exact-integer)
 Port to listen for HTTPS.
 
 @item @code{ssl-cert} (type: maybe-string)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 4fcfcc13ea..2a6e1b90df 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -39,6 +39,7 @@ (define-module (gnu services audio)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-71)
+  #:use-module (srfi srfi-171)
   #:export (mpd-output
             mpd-output?
             mpd-output-name
@@ -686,9 +687,6 @@ (define mpd-service-type
 ;;; myMPD
 ;;;
 
-(define (string-or-symbol? x)
-  (or (symbol? x) (string? x)))
-
 (define-configuration/no-serialization mympd-ip-acl
   (allow
    (list-of-strings '())
@@ -698,7 +696,7 @@ (define-configuration/no-serialization mympd-ip-acl
    (list-of-strings '())
    "Disallowed IP addresses."))
 
-(define-maybe/no-serialization integer)
+(define-maybe/no-serialization exact-integer)
 (define-maybe/no-serialization mympd-ip-acl)
 
 (define %mympd-user
@@ -749,11 +747,28 @@ (define (mympd-log-to-sanitizer value)
      value)
     (_ (configuration-field-error #f 'log-to value))))
 
-;; XXX: The serialization procedures are insufficient since we require
-;; access to multiple fields at once.
-;; Fields marked with empty-serializer are never serialized and are
-;; used for command-line arguments or by the service definition.
-(define-configuration/no-serialization mympd-configuration
+(define (mympd-field-serializer file-name)
+  "Return a procedure that partially serializes the fields of
+mympd-configuration as pairs of file-names and file-like objects whose
+contents are the serialized values of the fields."
+  (define serialize-value
+    (match-lambda
+      ((? boolean? val) (if val "true" "false"))
+      ((? integer? val) (number->string val))
+      ((? mympd-ip-acl? val) (ip-acl-serialize-configuration val))
+      ((? string? val) val)))
+
+  (define (ip-acl-serialize-configuration config)
+    (string-join
+     (append
+      (map (cut string-append "+" <>) (mympd-ip-acl-allow config))
+      (map (cut string-append "-" <>) (mympd-ip-acl-deny config))) ","))
+
+  (lambda (_ field-value)
+    (cons file-name
+          (plain-file file-name (serialize-value field-value)))))
+
+(define-configuration mympd-configuration
   (package
     (file-like mympd)
     "The package object of the myMPD server."
@@ -789,27 +804,33 @@ (define-configuration/no-serialization mympd-configuration
 
   (acl
    maybe-mympd-ip-acl
-   "ACL to access the myMPD webserver.")
+   "ACL to access the myMPD webserver."
+   (serializer (mympd-field-serializer "acl")))
 
   (covercache-ttl
-   (maybe-integer 31)
-   "How long to keep cached covers, @code{0} disables cover caching.")
+   (maybe-exact-integer 31)
+   "How long to keep cached covers, @code{0} disables cover caching."
+   (serializer (mympd-field-serializer "covercache_keep_days")))
 
   (http?
    (boolean #t)
-   "HTTP support.")
+   "HTTP support."
+   (serializer (mympd-field-serializer "http")))
 
   (host
    (string "[::]")
-   "Host name to listen on.")
+   "Host name to listen on."
+   (serializer (mympd-field-serializer "http_host")))
 
   (port
-   (maybe-port 80)
-   "HTTP port to listen on.")
+   (maybe-exact-integer 80)
+   "HTTP port to listen on."
+   (serializer (mympd-field-serializer "http_port")))
 
   (log-level
-   (integer 5)
-   "How much detail to include in logs, possible values: @code{0} to @code{7}.")
+   (exact-integer 5)
+   "How much detail to include in logs, possible values: @code{0} to @code{7}."
+   (serializer (mympd-field-serializer "loglevel")))
 
   (log-to
    maybe-string
@@ -822,89 +843,64 @@ (define-configuration/no-serialization mympd-configuration
   (lualibs
    (maybe-string "all")
    "See
-@url{https://jcorporation.github.io/myMPD/scripting/#lua-standard-libraries}.")
+@url{https://jcorporation.github.io/myMPD/scripting/#lua-standard-libraries}."
+   (serializer (mympd-field-serializer "lualibs")))
 
   (uri
    maybe-string
    "Override URI to myMPD.
-See @url{https://github.com/jcorporation/myMPD/issues/950}.")
+See @url{https://github.com/jcorporation/myMPD/issues/950}."
+   (serializer (mympd-field-serializer "mympd_uri")))
 
   (script-acl
    (maybe-mympd-ip-acl (mympd-ip-acl
                         (allow '("127.0.0.1"))))
-   "ACL to access the myMPD script backend.")
+   "ACL to access the myMPD script backend."
+   (serializer (mympd-field-serializer "scriptacl")))
 
   (ssl?
    (boolean #f)
-   "SSL/TLS support.")
+   "SSL/TLS support."
+   (serializer (mympd-field-serializer "ssl")))
 
   (ssl-port
-   (maybe-port 443)
-   "Port to listen for HTTPS.")
+   (maybe-exact-integer 443)
+   "Port to listen for HTTPS."
+   (serializer (mympd-field-serializer "ssl_port")))
 
   (ssl-cert
    maybe-string
-   "Path to PEM encoded X.509 SSL/TLS certificate (public key).")
+   "Path to PEM encoded X.509 SSL/TLS certificate (public key)."
+   (serializer (mympd-field-serializer "ssl_cert")))
 
   (ssl-key
    maybe-string
-   "Path to PEM encoded SSL/TLS private key.")
+   "Path to PEM encoded SSL/TLS private key."
+   (serializer (mympd-field-serializer "ssl_key")))
 
   (pin-hash
    maybe-string
    "SHA-256 hashed pin used by myMPD to control settings access by
-prompting a pin from the user.")
+prompting a pin from the user."
+   (serializer (mympd-field-serializer "pin_hash")))
 
   (save-caches?
    maybe-boolean
-   "Whether to preserve caches between service restarts."))
-
-(define (mympd-serialize-configuration config)
-  (define serialize-value
-    (match-lambda
-      ((? boolean? val) (if val "true" "false"))
-      ((? integer? val) (number->string val))
-      ((? mympd-ip-acl? val) (ip-acl-serialize-configuration val))
-      ((? string? val) val)))
-
-  (define (ip-acl-serialize-configuration config)
-    (define (serialize-list-of-strings prefix lst)
-      (map (cut format #f "~a~a" prefix <>) lst))
-    (string-join
-     (append
-      (serialize-list-of-strings "+" (mympd-ip-acl-allow config))
-      (serialize-list-of-strings "-" (mympd-ip-acl-deny config))) ","))
-
-  ;; myMPD configuration fields are serialized as individual files under
-  ;; <work-directory>/config/.
-  (match-record config <mympd-configuration> (work-directory acl
-                                              covercache-ttl http? host port
-                                              log-level lualibs uri script-acl
-                                              ssl? ssl-port ssl-cert ssl-key
-                                              pin-hash save-caches?)
-    (define (serialize-field filename value)
-      (when (maybe-value-set? value)
-        (list (format #f "~a/config/~a" work-directory filename)
-              (mixed-text-file filename (serialize-value value)))))
-
-    (let ((filename-to-field `(("acl" . ,acl)
-                               ("covercache_keep_days" . ,covercache-ttl)
-                               ("http"                 . ,http?)
-                               ("http_host"            . ,host)
-                               ("http_port"            . ,port)
-                               ("loglevel"             . ,log-level)
-                               ("lualibs"              . ,lualibs)
-                               ("mympd_uri"            . ,uri)
-                               ("scriptacl"            . ,script-acl)
-                               ("ssl"                  . ,ssl?)
-                               ("ssl_port"             . ,ssl-port)
-                               ("ssl_cert"             . ,ssl-cert)
-                               ("ssl_key"              . ,ssl-key)
-                               ("pin_hash"             . ,pin-hash)
-                               ("save_caches"          . ,save-caches?))))
-      (filter list?
-              (generic-serialize-alist list serialize-field
-                                       filename-to-field)))))
+   "Whether to preserve caches between service restarts."
+   (serializer (mympd-field-serializer "save_caches"))))
+
+(define (mympd-configuration->files config)
+  (match-record config <mympd-configuration> (work-directory)
+    (list-transduce
+     (compose (base-transducer config)
+              (tmap (match-lambda
+                      ((file-name . file)
+                       ;; myMPD configuration fields are serialized as
+                       ;; individual files under <work-directory>/config/….
+                       (list (string-append work-directory "/config/"
+                                            file-name)
+                             file)))))
+     rcons mympd-configuration-fields)))
 
 (define (mympd-shepherd-service config)
   (match-record config <mympd-configuration>
@@ -957,8 +953,7 @@ (define (mympd-accounts config)
       (list user group))))
 
 (define (mympd-log-rotation config)
-  (match-record config <mympd-configuration>
-    (log-to)
+  (match-record config <mympd-configuration> (log-to)
     (if (maybe-value-set? log-to)
         (list (log-rotation
                (files (list log-to))))
@@ -973,8 +968,8 @@ (define mympd-service-type
            (service-extension account-service-type
                               mympd-accounts)
            (service-extension special-files-service-type
-                              mympd-serialize-configuration)
+                              mympd-configuration->files)
            (service-extension rottlog-service-type
                               mympd-log-rotation)))
-   (description "Run myMPD, a frontend for MPD. (Music Player Daemon)")
+   (description "Run myMPD, a frontend for @acronym{MPD, Music Player Daemon}.")
    (default-value (mympd-configuration))))
-- 
2.41.0

[bug#67842] [PATCH 4/4] services: mympd: Refactor serialization process.

[Liliana Marie Prikler]

Am Freitag, dem 15.12.2023 um 21:02 +0000 schrieb Bruno Victal:
> -(define-maybe/no-serialization integer)
> +(define-maybe/no-serialization exact-integer)
At the risk of asking a silly question, what's the difference between
an integer and an exact integer?

>  (define-maybe/no-serialization mympd-ip-acl)
>  
>  (define %mympd-user
> @@ -749,11 +747,28 @@ (define (mympd-log-to-sanitizer value)
>       value)
>      (_ (configuration-field-error #f 'log-to value))))
>  
> -;; XXX: The serialization procedures are insufficient since we
> require
> -;; access to multiple fields at once.
> -;; Fields marked with empty-serializer are never serialized and are
> -;; used for command-line arguments or by the service definition.
> -(define-configuration/no-serialization mympd-configuration
> +(define (mympd-field-serializer file-name)
> +  "Return a procedure that partially serializes the fields of
> +mympd-configuration as pairs of file-names and file-like objects
> whose
> +contents are the serialized values of the fields."
> +  (define serialize-value
> +    (match-lambda
> +      ((? boolean? val) (if val "true" "false"))
> +      ((? integer? val) (number->string val))
> +      ((? mympd-ip-acl? val) (ip-acl-serialize-configuration val))
> +      ((? string? val) val)))
> +
> +  (define (ip-acl-serialize-configuration config)
> +    (string-join
> +     (append
> +      (map (cut string-append "+" <>) (mympd-ip-acl-allow config))
> +      (map (cut string-append "-" <>) (mympd-ip-acl-deny config)))
> ","))
> +
> +  (lambda (_ field-value)
> +    (cons file-name
> +          (plain-file file-name (serialize-value field-value)))))
> +
> +(define-configuration mympd-configuration
>    (package
>      (file-like mympd)
>      "The package object of the myMPD server."
> @@ -789,27 +804,33 @@ (define-configuration/no-serialization mympd-
> configuration
>  
>    (acl
>     maybe-mympd-ip-acl
> -   "ACL to access the myMPD webserver.")
> +   "ACL to access the myMPD webserver."
> +   (serializer (mympd-field-serializer "acl")))
>  
>    (covercache-ttl
> -   (maybe-integer 31)
> -   "How long to keep cached covers, @code{0} disables cover
> caching.")
> +   (maybe-exact-integer 31)
> +   "How long to keep cached covers, @code{0} disables cover
> caching."
> +   (serializer (mympd-field-serializer "covercache_keep_days")))
>  
>    (http?
>     (boolean #t)
> -   "HTTP support.")
> +   "HTTP support."
> +   (serializer (mympd-field-serializer "http")))
>  
>    (host
>     (string "[::]")
> -   "Host name to listen on.")
> +   "Host name to listen on."
> +   (serializer (mympd-field-serializer "http_host")))
>  
>    (port
> -   (maybe-port 80)
> -   "HTTP port to listen on.")
> +   (maybe-exact-integer 80)
Losing the information that this is a port (i.e. only integers that fit
into a uint16 are valid) is imho not great.
> +   "HTTP port to listen on."
> +   (serializer (mympd-field-serializer "http_port")))
>  
>    (log-level
> -   (integer 5)
> -   "How much detail to include in logs, possible values: @code{0} to
> @code{7}.")
> +   (exact-integer 5)
> +   "How much detail to include in logs, possible values: @code{0} to
> @code{7}."
> +   (serializer (mympd-field-serializer "loglevel")))
>  
>    (log-to
>     maybe-string
> @@ -822,89 +843,64 @@ (define-configuration/no-serialization mympd-
> configuration
>    (lualibs
>     (maybe-string "all")
>     "See
> -
> @url{https://jcorporation.github.io/myMPD/scripting/#lua-standard-libr
> aries}.")
> +@url{
> https://jcorporation.github.io/myMPD/scripting/#lua-standard-librarie
> s}."
> +   (serializer (mympd-field-serializer "lualibs")))
>  
>    (uri
>     maybe-string
>     "Override URI to myMPD.
> -See @url{https://github.com/jcorporation/myMPD/issues/950}.")
> +See @url{https://github.com/jcorporation/myMPD/issues/950}."
> +   (serializer (mympd-field-serializer "mympd_uri")))
>  
>    (script-acl
>     (maybe-mympd-ip-acl (mympd-ip-acl
>                          (allow '("127.0.0.1"))))
> -   "ACL to access the myMPD script backend.")
> +   "ACL to access the myMPD script backend."
> +   (serializer (mympd-field-serializer "scriptacl")))
>  
>    (ssl?
>     (boolean #f)
> -   "SSL/TLS support.")
> +   "SSL/TLS support."
> +   (serializer (mympd-field-serializer "ssl")))
>  
>    (ssl-port
> -   (maybe-port 443)
> -   "Port to listen for HTTPS.")
> +   (maybe-exact-integer 443)
> +   "Port to listen for HTTPS."
> +   (serializer (mympd-field-serializer "ssl_port")))
>  
>    (ssl-cert
>     maybe-string
> -   "Path to PEM encoded X.509 SSL/TLS certificate (public key).")
> +   "Path to PEM encoded X.509 SSL/TLS certificate (public key)."
> +   (serializer (mympd-field-serializer "ssl_cert")))
>  
>    (ssl-key
>     maybe-string
> -   "Path to PEM encoded SSL/TLS private key.")
> +   "Path to PEM encoded SSL/TLS private key."
> +   (serializer (mympd-field-serializer "ssl_key")))
>  
>    (pin-hash
>     maybe-string
>     "SHA-256 hashed pin used by myMPD to control settings access by
> -prompting a pin from the user.")
> +prompting a pin from the user."
> +   (serializer (mympd-field-serializer "pin_hash")))
>  
>    (save-caches?
>     maybe-boolean
> -   "Whether to preserve caches between service restarts."))
> -
> -(define (mympd-serialize-configuration config)
> -  (define serialize-value
> -    (match-lambda
> -      ((? boolean? val) (if val "true" "false"))
> -      ((? integer? val) (number->string val))
> -      ((? mympd-ip-acl? val) (ip-acl-serialize-configuration val))
> -      ((? string? val) val)))
> -
> -  (define (ip-acl-serialize-configuration config)
> -    (define (serialize-list-of-strings prefix lst)
> -      (map (cut format #f "~a~a" prefix <>) lst))
> -    (string-join
> -     (append
> -      (serialize-list-of-strings "+" (mympd-ip-acl-allow config))
> -      (serialize-list-of-strings "-" (mympd-ip-acl-deny config)))
> ","))
> -
> -  ;; myMPD configuration fields are serialized as individual files
> under
> -  ;; <work-directory>/config/.
> -  (match-record config <mympd-configuration> (work-directory acl
> -                                              covercache-ttl http?
> host port
> -                                              log-level lualibs uri
> script-acl
> -                                              ssl? ssl-port ssl-cert
> ssl-key
> -                                              pin-hash save-caches?)
> -    (define (serialize-field filename value)
> -      (when (maybe-value-set? value)
> -        (list (format #f "~a/config/~a" work-directory filename)
> -              (mixed-text-file filename (serialize-value value)))))
> -
> -    (let ((filename-to-field `(("acl" . ,acl)
> -                               ("covercache_keep_days" .
> ,covercache-ttl)
> -                               ("http"                 . ,http?)
> -                               ("http_host"            . ,host)
> -                               ("http_port"            . ,port)
> -                               ("loglevel"             . ,log-level)
> -                               ("lualibs"              . ,lualibs)
> -                               ("mympd_uri"            . ,uri)
> -                               ("scriptacl"            . ,script-
> acl)
> -                               ("ssl"                  . ,ssl?)
> -                               ("ssl_port"             . ,ssl-port)
> -                               ("ssl_cert"             . ,ssl-cert)
> -                               ("ssl_key"              . ,ssl-key)
> -                               ("pin_hash"             . ,pin-hash)
> -                               ("save_caches"          . ,save-
> caches?))))
> -      (filter list?
> -              (generic-serialize-alist list serialize-field
> -                                       filename-to-field)))))
> +   "Whether to preserve caches between service restarts."
> +   (serializer (mympd-field-serializer "save_caches"))))
> +
> +(define (mympd-configuration->files config)
> +  (match-record config <mympd-configuration> (work-directory)
> +    (list-transduce
> +     (compose (base-transducer config)
> +              (tmap (match-lambda
> +                      ((file-name . file)
> +                       ;; myMPD configuration fields are serialized
> as
> +                       ;; individual files under <work-
> directory>/config/….
> +                       (list (string-append work-directory
> "/config/"
> +                                            file-name)
> +                             file)))))
> +     rcons mympd-configuration-fields)))
>  
>  (define (mympd-shepherd-service config)
>    (match-record config <mympd-configuration>
> @@ -957,8 +953,7 @@ (define (mympd-accounts config)
>        (list user group))))
>  
>  (define (mympd-log-rotation config)
> -  (match-record config <mympd-configuration>
> -    (log-to)
> +  (match-record config <mympd-configuration> (log-to)
>      (if (maybe-value-set? log-to)
>          (list (log-rotation
>                 (files (list log-to))))
> @@ -973,8 +968,8 @@ (define mympd-service-type
>             (service-extension account-service-type
>                                mympd-accounts)
>             (service-extension special-files-service-type
> -                              mympd-serialize-configuration)
> +                              mympd-configuration->files)
>             (service-extension rottlog-service-type
>                                mympd-log-rotation)))
> -   (description "Run myMPD, a frontend for MPD. (Music Player
> Daemon)")
> +   (description "Run myMPD, a frontend for @acronym{MPD, Music
> Player Daemon}.")
>     (default-value (mympd-configuration))))
Cheers

[bug#67842] [PATCH 4/4] services: mympd: Refactor serialization process.

[Bruno Victal]

[-- Attachment #1.1: Type: text/plain, Size: 2199 bytes --]

Hi Liliana,

On 2023-12-16 01:44, Liliana Marie Prikler wrote:
> Am Freitag, dem 15.12.2023 um 21:02 +0000 schrieb Bruno Victal:
>> -(define-maybe/no-serialization integer)
>> +(define-maybe/no-serialization exact-integer)
> At the risk of asking a silly question, what's the difference between
> an integer and an exact integer?

IIUC it has to do with whether a decimal point is present or not,
which influences the serialization process. (e.g. having port set
to 8080.0 doesn't make much sense even though it is an integer)

--8<---------------cut here---------------start------------->8---
$ cat integer-dem.scm
#!/usr/bin/env -S guile --no-auto-compile
!#

(for-each
 (lambda (s)
   (format #t "Formatted output: ~a~%" s)
   (format #t "number->string: ~a~%" (number->string s))
   (format #t "Integer? ~a~%" (integer? s))
   (format #t "Exact-integer? ~a~%" (exact-integer? s))
   (newline))
 (list 64 128.0))

$ ./integer-dem.scm
Formatted output: 64
number->string: 64
Integer? #t
Exact-integer? #t

Formatted output: 128.0
number->string: 128.0
Integer? #t
Exact-integer? #f
--8<---------------cut here---------------end--------------->8---

>>    (port
>> -   (maybe-port 80)
>> -   "HTTP port to listen on.")
>> +   (maybe-exact-integer 80)
> Losing the information that this is a port (i.e. only integers that fit
> into a uint16 are valid) is imho not great.

I'm not too happy with this either, though in hindsight I think
redefining 'port?' (from Guile Ports) was a bad idea. At the moment
the (re)defined port? predicate only checks whether the value is an
integer, so switching it to exact-integer doesn't seem to change things
much. (other than being stricter in criteria)

Alternatively we could have a proper predicate, perhaps named ip-port?
that would not only perform the exact-integer? check, but also test
whether it fits within a uint16. I'm more inclined to introduce this
kind of change in a separate series that would define it in a reusable
manner and perform a cleanup run across the existing services though.

-- 
Furthermore, I consider that nonfree software must be eradicated.

Cheers,
Bruno.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[bug#67842] [PATCH 4/4] services: mympd: Refactor serialization process.

[Liliana Marie Prikler]

Am Samstag, dem 16.12.2023 um 19:36 +0000 schrieb Bruno Victal:
> Hi Liliana,
> 
> On 2023-12-16 01:44, Liliana Marie Prikler wrote:
> > Am Freitag, dem 15.12.2023 um 21:02 +0000 schrieb Bruno Victal:
> > > -(define-maybe/no-serialization integer)
> > > +(define-maybe/no-serialization exact-integer)
> > At the risk of asking a silly question, what's the difference
> > between an integer and an exact integer?
> 
> IIUC it has to do with whether a decimal point is present or not,
> which influences the serialization process. (e.g. having port set
> to 8080.0 doesn't make much sense even though it is an integer)
I don't think we have to make this distinction that often, though; and
if we do, there are more fitting descriptions like signed-integer and
unsigned-integer.  Even if it's to guard against silly inputs, most
folks wouldn't write 8080.0 there.

> --8<---------------cut here---------------start------------->8---
> $ cat integer-dem.scm
> #!/usr/bin/env -S guile --no-auto-compile
> !#
> 
> (for-each
>  (lambda (s)
>    (format #t "Formatted output: ~a~%" s)
>    (format #t "number->string: ~a~%" (number->string s))
>    (format #t "Integer? ~a~%" (integer? s))
>    (format #t "Exact-integer? ~a~%" (exact-integer? s))
>    (newline))
>  (list 64 128.0))
> 
> $ ./integer-dem.scm
> Formatted output: 64
> number->string: 64
> Integer? #t
> Exact-integer? #t
> 
> Formatted output: 128.0
> number->string: 128.0
> Integer? #t
> Exact-integer? #f
> --8<---------------cut here---------------end--------------->8---
> 
> > >    (port
> > > -   (maybe-port 80)
> > > -   "HTTP port to listen on.")
> > > +   (maybe-exact-integer 80)
> > Losing the information that this is a port (i.e. only integers that
> > fit
> > into a uint16 are valid) is imho not great.
> 
> I'm not too happy with this either, though in hindsight I think
> redefining 'port?' (from Guile Ports) was a bad idea. At the moment
> the (re)defined port? predicate only checks whether the value is an
> integer, so switching it to exact-integer doesn't seem to change
> things much. (other than being stricter in criteria)
Maybe port-number? is clearer?

> Alternatively we could have a proper predicate, perhaps named ip-
> port? that would not only perform the exact-integer? check, but also
> test whether it fits within a uint16. I'm more inclined to introduce
> this kind of change in a separate series that would define it in a
> reusable manner and perform a cleanup run across the existing
> services though.
From my point of view you are already introducing "this kind of change"
as not a separate series :)

Cheers