[PATCH 0/4] Refactor mympd service.

  • Open
  • quality assurance status badge
Details
2 participants
  • Liliana Marie Prikler
  • Bruno Victal
Owner
unassigned
Submitted by
Bruno Victal
Severity
normal
B
B
Bruno Victal wrote on 15 Dec 2023 21:59
(address . guix-patches@gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
cover.1702663151.git.mirai@makinata.eu
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
B
B
Bruno Victal wrote on 15 Dec 2023 22:02
[PATCH 1/4] doc: Remove missed hunk of revert commit for MPD service.
(address . 67842@debbugs.gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
00b00466d2660fec8a141724285c3d0339df7ad0.1702663151.git.mirai@makinata.eu
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(-)

Toggle diff (20 lines)
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
B
B
Bruno Victal wrote on 15 Dec 2023 22:02
[PATCH 2/4] tests: mympd: Simplify test.
(address . 67842@debbugs.gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
f3bc7f8d4223186255f2e874b9939fe2edbc963a.1702663151.git.mirai@makinata.eu
* 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(-)

Toggle diff (73 lines)
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
B
B
Bruno Victal wrote on 15 Dec 2023 22:02
[PATCH 3/4] services: mympd: Fix syslog logging and missing service destructor.
(address . 67842@debbugs.gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
fe43af9dc3db1eedaab859558bf20bda21162eb9.1702663151.git.mirai@makinata.eu
* 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(-)

Toggle diff (28 lines)
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
B
B
Bruno Victal wrote on 15 Dec 2023 22:02
[PATCH 4/4] services: mympd: Refactor serialization process.
(address . 67842@debbugs.gnu.org)(name . Bruno Victal)(address . mirai@makinata.eu)
eb6d6e5a6da0165bc4da411335b327fcac9350b6.1702663151.git.mirai@makinata.eu
* 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(-)

Toggle diff (288 lines)
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
L
L
Liliana Marie Prikler wrote on 16 Dec 2023 02:44
ccbd11dc1758a8a699362ccd017ef3cb3539c97d.camel@gmail.com
Am Freitag, dem 15.12.2023 um 21:02 +0000 schrieb Bruno Victal:
Toggle quote (2 lines)
> -(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?

Toggle quote (74 lines)
>  (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.
Toggle quote (175 lines)
> +   "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
B
B
Bruno Victal wrote on 16 Dec 2023 20:36
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)(address . 67842@debbugs.gnu.org)
9283714a-991e-436a-bb83-fb9254cba5a2@makinata.eu
Hi Liliana,

On 2023-12-16 01:44, Liliana Marie Prikler wrote:
Toggle quote (6 lines)
> 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)

Toggle snippet (25 lines)
$ 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

Toggle quote (7 lines)
>>    (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: OpenPGP_signature.asc
L
L
Liliana Marie Prikler wrote on 16 Dec 2023 23:12
(name . Bruno Victal)(address . mirai@makinata.eu)(address . 67842@debbugs.gnu.org)
c6c6755bb0b9f5f863b9f9ab1905c960c5c53644.camel@gmail.com
Am Samstag, dem 16.12.2023 um 19:36 +0000 schrieb Bruno Victal:
Toggle quote (12 lines)
> 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.

Toggle quote (39 lines)
> --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?

Toggle quote (6 lines)
> 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
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 67842@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 67842
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch