[bug#67842,4/4] services: mympd: Refactor serialization process.
Commit Message
* 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(-)
Comments
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
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.
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
@@ -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)
@@ -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))))