* [bug#62298] [PATCH 1/8] services: configuration: Add user-defined sanitizer support.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
@ 2023-03-20 17:07 ` Bruno Victal
2023-03-20 19:43 ` Liliana Marie Prikler
2023-03-20 17:07 ` [bug#62298] [PATCH 2/8] services: replace bare serializers with (serializer ...) Bruno Victal
` (9 subsequent siblings)
10 siblings, 1 reply; 47+ messages in thread
From: Bruno Victal @ 2023-03-20 17:07 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
This changes the 'custom-serializer' field into a generic
'extra-args' field that can be extended to support new literals.
With this mechanism, the literals 'sanitizer' allow for user-defined
sanitizer procedures while the 'serializer' literal is used for
custom serializer procedures.
The 'empty-serializer' was also added as a 'literal' and can be used
just like it was previously.
With the repurposing of 'custom-serializer' into 'extra-args', to prevent
intolerable confusion, the custom serializer procedures should be
specified using the new 'literals' approach, with the previous “style”
being considered as deprecated.
* gnu/services/configuration.scm (define-configuration-helper):
Rename 'custom-serializer' to 'extra-args'.
Add support for literals 'sanitizer', 'serializer' and 'empty-serializer'.
Rename procedure 'field-sanitizer' to 'default-field-sanitizer' to avoid syntax clash.
Only define default field sanitizers if user-defined ones are absent.
(deprecated-style-serializer?): New predicate.
(get-sanitizer, get-serializer): New procedure.
(<configuration-field>)[sanitizer]: New field.
* doc/guix.texi (Complex Configurations): Document the newly added literals.
* tests/services/configuration.scm: Add tests for the new literals.
---
doc/guix.texi | 30 ++++++-
gnu/services/configuration.scm | 97 ++++++++++++++++-----
tests/services/configuration.scm | 145 ++++++++++++++++++++++++++++++-
3 files changed, 247 insertions(+), 25 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index fa9ea5a6ec..f84cadeeda 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -41127,7 +41127,7 @@ Complex Configurations
(@var{field-name}
(@var{type} @var{default-value})
@var{documentation}
- @var{serializer})
+ (serializer @var{serializer}))
(@var{field-name}
(@var{type})
@@ -41136,7 +41136,18 @@ Complex Configurations
(@var{field-name}
(@var{type})
@var{documentation}
- @var{serializer})
+ (serializer @var{serializer}))
+
+(@var{field-name}
+ (@var{type})
+ @var{documentation}
+ (sanitizer @var{sanitizer})
+
+(@var{field-name}
+ (@var{type})
+ @var{documentation}
+ (sanitizer @var{sanitizer})
+ (serializer @var{serializer}))
@end example
@var{field-name} is an identifier that denotes the name of the field in
@@ -41159,6 +41170,21 @@ Complex Configurations
@var{documentation} is a string formatted with Texinfo syntax which
should provide a description of what setting this field does.
+@var{sanitizer} is the name of a procedure which takes one argument,
+a user-supplied value, and returns a ``sanitized'' value for the field.
+If none is specified, the predicate @code{@var{type}?} is automatically
+used to construct an internal sanitizer that asserts the type correctness
+of the field value.
+
+An example of a sanitizer for a field that accepts both strings and
+symbols looks like this:
+@lisp
+(define (sanitize-foo value)
+ (cond ((string? value) value)
+ ((symbol? value) (symbol->string value))
+ (else (throw 'bad! value))))
+@end lisp
+
@var{serializer} is the name of a procedure which takes two arguments,
the first is the name of the field, and the second is the value
corresponding to the field. The procedure should return a string or
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 02d1aa1796..6ad9ade76d 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -6,6 +6,7 @@
;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -28,7 +29,8 @@ (define-module (gnu services configuration)
#:use-module (guix gexp)
#:use-module ((guix utils) #:select (source-properties->location))
#:use-module ((guix diagnostics)
- #:select (formatted-message location-file &error-location))
+ #:select (formatted-message location-file &error-location
+ warning))
#:use-module ((guix modules) #:select (file-name->module-name))
#:use-module (guix i18n)
#:autoload (texinfo) (texi-fragment->stexi)
@@ -44,6 +46,7 @@ (define-module (gnu services configuration)
configuration-field-type
configuration-missing-field
configuration-field-error
+ configuration-field-sanitizer
configuration-field-serializer
configuration-field-getter
configuration-field-default-value-thunk
@@ -116,6 +119,7 @@ (define-record-type* <configuration-field>
(type configuration-field-type)
(getter configuration-field-getter)
(predicate configuration-field-predicate)
+ (sanitizer configuration-field-sanitizer)
(serializer configuration-field-serializer)
(default-value-thunk configuration-field-default-value-thunk)
(documentation configuration-field-documentation))
@@ -181,8 +185,46 @@ (define (normalize-field-type+def s)
(values #'(field-type %unset-value)))))
(define (define-configuration-helper serialize? serializer-prefix syn)
+
+ (define (deprecated-style-serializer? s)
+ ;; TODO: helper for deprecated style, remove later.
+ (syntax-case s ()
+ ;; Case 1. Nothing after 'doc'.
+ (() #f)
+ ;; Case 2. Bare serializer after 'doc'. [deprecated]
+ ;; Until this case is removed, don't forget to add a check for your
+ ;; newly added field/literal here.
+ (proc
+ (not (or (maybe-value-set? (get-serializer s))
+ (maybe-value-set? (get-sanitizer s))))
+ (begin
+ (warning #f (G_ "specifying serializers after documentation is \
+deprecated, use (serializer ~a) instead~%") (car (syntax->datum #'proc))) #t))
+ ;; Else, something after 'doc' in the updated style.
+ (else #f)))
+
+ ;; The get-… procedures perform scanning to @var{extra-args} to allow for
+ ;; newly added fields to be specified in arbitrary order.
+ (define (get-sanitizer s)
+ (syntax-case s (sanitizer)
+ (((sanitizer proc) _ ...)
+ #'proc)
+ ((_ tail ...)
+ (get-sanitizer #'(tail ...)))
+ (() %unset-value)))
+
+ (define (get-serializer s)
+ (syntax-case s (serializer empty-serializer)
+ (((serializer proc) _ ...)
+ #'proc)
+ ((empty-serializer _ ...)
+ #'empty-serializer)
+ ((_ tail ...)
+ (get-serializer #'(tail ...)))
+ (() %unset-value)))
+
(syntax-case syn ()
- ((_ stem (field field-type+def doc custom-serializer ...) ...)
+ ((_ stem (field field-type+def doc extra-args ...) ...)
(with-syntax
((((field-type def) ...)
(map normalize-field-type+def #'(field-type+def ...))))
@@ -200,21 +242,23 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
((field-type default-value)
default-value))
#'((field-type def) ...)))
+ ((field-sanitizer ...)
+ (map (compose maybe-value get-sanitizer)
+ #'((extra-args ...) ...)))
((field-serializer ...)
- (map (lambda (type custom-serializer)
+ (map (lambda (type extra-args)
(and serialize?
- (match custom-serializer
- ((serializer)
- serializer)
- (()
- (if serializer-prefix
- (id #'stem
- serializer-prefix
- #'serialize- type)
- (id #'stem #'serialize- type))))))
+ (or
+ (if (deprecated-style-serializer? extra-args)
+ (car extra-args) ; strip outer parenthesis
+ #f)
+ (maybe-value (get-serializer extra-args))
+ (if serializer-prefix
+ (id #'stem serializer-prefix #'serialize- type)
+ (id #'stem #'serialize- type)))))
#'(field-type ...)
- #'((custom-serializer ...) ...))))
- (define (field-sanitizer name pred)
+ #'((extra-args ...) ...))))
+ (define (default-field-sanitizer name pred)
;; Define a macro for use as a record field sanitizer, where NAME
;; is the name of the field and PRED is the predicate that tells
;; whether a value is valid for this field.
@@ -235,21 +279,29 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
#`(begin
;; Define field validation macros.
- #,@(map field-sanitizer
- #'(field ...)
- #'(field-predicate ...))
+ #,@(filter-map (lambda (name pred sanitizer)
+ (if sanitizer
+ #f
+ (default-field-sanitizer name pred)))
+ #'(field ...)
+ #'(field-predicate ...)
+ #'(field-sanitizer ...))
(define-record-type* #,(id #'stem #'< #'stem #'>)
stem
#,(id #'stem #'make- #'stem)
#,(id #'stem #'stem #'?)
- #,@(map (lambda (name getter def)
- #`(#,name #,getter (default #,def)
+ #,@(map (lambda (name getter def sanitizer)
+ #`(#,name #,getter
+ (default #,def)
(sanitize
- #,(id #'stem #'validate- #'stem #'- name))))
+ #,(or sanitizer
+ (id #'stem
+ #'validate- #'stem #'- name)))))
#'(field ...)
#'(field-getter ...)
- #'(field-default ...))
+ #'(field-default ...)
+ #'(field-sanitizer ...))
(%location #,(id #'stem #'stem #'-source-location)
(default (and=> (current-source-location)
source-properties->location))
@@ -261,6 +313,9 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
(type 'field-type)
(getter field-getter)
(predicate field-predicate)
+ (sanitizer
+ (or field-sanitizer
+ (id #'stem #'validate- #'stem #'- #'field)))
(serializer field-serializer)
(default-value-thunk
(lambda ()
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 4f8a74dc8a..c5569a9e50 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -22,6 +23,7 @@ (define-module (tests services configuration)
#:use-module (gnu services configuration)
#:use-module (guix diagnostics)
#:use-module (guix gexp)
+ #:autoload (guix i18n) (G_)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-64))
@@ -46,14 +48,14 @@ (define-configuration port-configuration
(port-configuration-port (port-configuration)))
(test-equal "wrong type for a field"
- '("configuration.scm" 57 11) ;error location
+ '("configuration.scm" 59 11) ;error location
(guard (c ((configuration-error? c)
(let ((loc (error-location c)))
(list (basename (location-file loc))
(location-line loc)
(location-column loc)))))
(port-configuration
- ;; This is line 56; the test relies on line/column numbers!
+ ;; This is line 58; the test relies on line/column numbers!
(port "This is not a number!"))))
(define-configuration port-configuration-cs
@@ -109,6 +111,145 @@ (define-configuration configuration-with-prefix
(let ((config (configuration-with-prefix)))
(serialize-configuration config configuration-with-prefix-fields))))
+\f
+;;;
+;;; define-configuration macro, extra-args literals
+;;;
+
+(define (eval-gexp x)
+ "Get serialized config as string."
+ (eval (gexp->approximate-sexp x)
+ (current-module)))
+
+(define (port? value)
+ (or (string? value) (number? value)))
+
+(define (sanitize-port value)
+ (cond ((number? value) value)
+ ((string? value) (string->number value))
+ (else (raise (formatted-message (G_ "Bad value: ~a") value)))))
+
+(let ()
+ ;; Basic sanitizer literal tests
+
+ (define serialize-port serialize-number)
+
+ (define-configuration config-with-sanitizer
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)))
+
+ (test-equal "default value, sanitizer"
+ 80
+ (config-with-sanitizer-port (config-with-sanitizer)))
+
+ (test-equal "string value, sanitized to number"
+ 56
+ (config-with-sanitizer-port (config-with-sanitizer
+ (port "56"))))
+
+
+ (define (custom-serialize-port field-name value)
+ (number->string value))
+
+ (define-configuration config-serializer
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (serializer custom-serialize-port)))
+
+ (test-equal "default value, serializer literal"
+ "80"
+ (eval-gexp
+ (serialize-configuration (config-serializer)
+ config-serializer-fields))))
+
+(let ()
+ ;; empty-serializer as literal/procedure tests
+
+ ;; empty-serializer as literal
+ (define-configuration config-with-literal
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ empty-serializer))
+
+ ;; empty-serializer as procedure
+ (define-configuration config-with-proc
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (serializer empty-serializer)))
+
+ (test-equal "empty-serializer as literal"
+ ""
+ (eval-gexp
+ (serialize-configuration (config-with-literal)
+ config-with-literal-fields)))
+
+ (test-equal "empty-serializer as procedure"
+ ""
+ (eval-gexp
+ (serialize-configuration (config-with-proc)
+ config-with-proc-fields))))
+
+(let ()
+ ;; permutation tests
+
+ (define-configuration config-san+empty-ser
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ empty-serializer))
+
+ (define-configuration config-san+ser
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ (serializer (lambda _ "foo"))))
+
+ (test-equal "default value, sanitizer, permutation"
+ 80
+ (config-san+empty-ser-port (config-san+empty-ser)))
+
+ (test-equal "default value, serializer, permutation"
+ "foo"
+ (eval-gexp
+ (serialize-configuration (config-san+ser) config-san+ser-fields)))
+
+ (test-equal "string value, sanitized to number, permutation"
+ 56
+ (config-san+ser-port (config-san+ser
+ (port "56"))))
+
+ ;; ordering tests
+ (define-configuration config-ser+san
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ (serializer (lambda _ "foo"))))
+
+ (define-configuration config-empty-ser+san
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ empty-serializer
+ (sanitizer sanitize-port)))
+
+ (test-equal "default value, sanitizer, permutation 2"
+ 56
+ (config-empty-ser+san-port (config-empty-ser+san
+ (port "56"))))
+
+ (test-equal "default value, serializer, permutation 2"
+ "foo"
+ (eval-gexp
+ (serialize-configuration (config-ser+san) config-ser+san-fields))))
+
\f
;;;
;;; define-maybe macro.
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH 1/8] services: configuration: Add user-defined sanitizer support.
2023-03-20 17:07 ` [bug#62298] [PATCH 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
@ 2023-03-20 19:43 ` Liliana Marie Prikler
0 siblings, 0 replies; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-20 19:43 UTC (permalink / raw)
To: Bruno Victal, 62298; +Cc: ludo, maxim.cournoyer
Am Montag, dem 20.03.2023 um 17:07 +0000 schrieb Bruno Victal:
> + ;; The get-… procedures perform scanning to @var{extra-args} to
> allow for
> + ;; newly added fields to be specified in arbitrary order.
> + (define (get-sanitizer s)
> + (syntax-case s (sanitizer)
> + (((sanitizer proc) _ ...)
> + #'proc)
> + ((_ tail ...)
> + (get-sanitizer #'(tail ...)))
> + (() %unset-value)))
> +
> + (define (get-serializer s)
> + (syntax-case s (serializer empty-serializer)
> + (((serializer proc) _ ...)
> + #'proc)
> + ((empty-serializer _ ...)
> + #'empty-serializer)
> + ((_ tail ...)
> + (get-serializer #'(tail ...)))
> + (() %unset-value)))
Instead of doing two passes, try using good old named let to loop over
s and get serializer and sanitizer in one go. Use #f for their
defaults so you can do (or serializer #'empty-serializer) and (or
sanitizer %unset-value).
> (syntax-case syn ()
> - ((_ stem (field field-type+def doc custom-serializer ...) ...)
> + ((_ stem (field field-type+def doc extra-args ...) ...)
> (with-syntax
> ((((field-type def) ...)
> (map normalize-field-type+def #'(field-type+def ...))))
> @@ -200,21 +242,23 @@ (define (define-configuration-helper serialize?
> serializer-prefix syn)
> ((field-type default-value)
> default-value))
> #'((field-type def) ...)))
> + ((field-sanitizer ...)
> + (map (compose maybe-value get-sanitizer)
> + #'((extra-args ...) ...)))
> ((field-serializer ...)
> - (map (lambda (type custom-serializer)
> + (map (lambda (type extra-args)
> (and serialize?
> - (match custom-serializer
> - ((serializer)
> - serializer)
> - (()
> - (if serializer-prefix
> - (id #'stem
> - serializer-prefix
> - #'serialize- type)
> - (id #'stem #'serialize- type))))))
> + (or
> + (if (deprecated-style-serializer? extra-
> args)
> + (car extra-args) ; strip outer
> parenthesis
> + #f)
> + (maybe-value (get-serializer extra-args))
> + (if serializer-prefix
> + (id #'stem serializer-prefix
> #'serialize- type)
> + (id #'stem #'serialize- type)))))
> #'(field-type ...)
> - #'((custom-serializer ...) ...))))
> - (define (field-sanitizer name pred)
> + #'((extra-args ...) ...))))
> + (define (default-field-sanitizer name pred)
> ;; Define a macro for use as a record field sanitizer,
> where NAME
> ;; is the name of the field and PRED is the predicate
> that tells
> ;; whether a value is valid for this field.
> @@ -235,21 +279,29 @@ (define (define-configuration-helper serialize?
> serializer-prefix syn)
>
> #`(begin
> ;; Define field validation macros.
> - #,@(map field-sanitizer
> - #'(field ...)
> - #'(field-predicate ...))
> + #,@(filter-map (lambda (name pred sanitizer)
> + (if sanitizer
> + #f
> + (default-field-sanitizer name
> pred)))
> + #'(field ...)
> + #'(field-predicate ...)
> + #'(field-sanitizer ...))
>
> (define-record-type* #,(id #'stem #'< #'stem #'>)
> stem
> #,(id #'stem #'make- #'stem)
> #,(id #'stem #'stem #'?)
> - #,@(map (lambda (name getter def)
> - #`(#,name #,getter (default #,def)
> + #,@(map (lambda (name getter def sanitizer)
> + #`(#,name #,getter
> + (default #,def)
> (sanitize
> - #,(id #'stem #'validate- #'stem
> #'- name))))
> + #,(or sanitizer
> + (id #'stem
> + #'validate- #'stem #'-
> name)))))
> #'(field ...)
> #'(field-getter ...)
> - #'(field-default ...))
> + #'(field-default ...)
> + #'(field-sanitizer ...))
> (%location #,(id #'stem #'stem #'-source-location)
> (default (and=> (current-source-location)
> source-properties-
> >location))
> @@ -261,6 +313,9 @@ (define (define-configuration-helper serialize?
> serializer-prefix syn)
> (type 'field-type)
> (getter field-getter)
> (predicate field-predicate)
> + (sanitizer
> + (or field-sanitizer
> + (id #'stem #'validate- #'stem #'-
> #'field)))
> (serializer field-serializer)
> (default-value-thunk
> (lambda ()
> diff --git a/tests/services/configuration.scm
> b/tests/services/configuration.scm
> index 4f8a74dc8a..c5569a9e50 100644
> --- a/tests/services/configuration.scm
> +++ b/tests/services/configuration.scm
> @@ -2,6 +2,7 @@
> ;;; Copyright © 2021, 2022 Maxim Cournoyer
> <maxim.cournoyer@gmail.com>
> ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
> ;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
> +;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -22,6 +23,7 @@ (define-module (tests services configuration)
> #:use-module (gnu services configuration)
> #:use-module (guix diagnostics)
> #:use-module (guix gexp)
> + #:autoload (guix i18n) (G_)
> #:use-module (srfi srfi-34)
> #:use-module (srfi srfi-64))
>
> @@ -46,14 +48,14 @@ (define-configuration port-configuration
> (port-configuration-port (port-configuration)))
>
> (test-equal "wrong type for a field"
> - '("configuration.scm" 57 11) ;error location
> + '("configuration.scm" 59 11) ;error location
> (guard (c ((configuration-error? c)
> (let ((loc (error-location c)))
> (list (basename (location-file loc))
> (location-line loc)
> (location-column loc)))))
> (port-configuration
> - ;; This is line 56; the test relies on line/column numbers!
> + ;; This is line 58; the test relies on line/column numbers!
> (port "This is not a number!"))))
>
> (define-configuration port-configuration-cs
> @@ -109,6 +111,145 @@ (define-configuration configuration-with-prefix
> (let ((config (configuration-with-prefix)))
> (serialize-configuration config configuration-with-prefix-
> fields))))
>
> +\f
> +;;;
> +;;; define-configuration macro, extra-args literals
> +;;;
> +
> +(define (eval-gexp x)
> + "Get serialized config as string."
> + (eval (gexp->approximate-sexp x)
> + (current-module)))
> +
> +(define (port? value)
> + (or (string? value) (number? value)))
> +
> +(define (sanitize-port value)
> + (cond ((number? value) value)
> + ((string? value) (string->number value))
> + (else (raise (formatted-message (G_ "Bad value: ~a")
> value)))))
> +
> +(let ()
> + ;; Basic sanitizer literal tests
> +
> + (define serialize-port serialize-number)
> +
> + (define-configuration config-with-sanitizer
> + (port
> + (port 80)
> + "Lorem Ipsum."
> + (sanitizer sanitize-port)))
> +
> + (test-equal "default value, sanitizer"
> + 80
> + (config-with-sanitizer-port (config-with-sanitizer)))
> +
> + (test-equal "string value, sanitized to number"
> + 56
> + (config-with-sanitizer-port (config-with-sanitizer
> + (port "56"))))
> +
> +
> + (define (custom-serialize-port field-name value)
> + (number->string value))
> +
> + (define-configuration config-serializer
> + (port
> + (port 80)
> + "Lorem Ipsum."
> + (serializer custom-serialize-port)))
> +
> + (test-equal "default value, serializer literal"
> + "80"
> + (eval-gexp
> + (serialize-configuration (config-serializer)
> + config-serializer-fields))))
> +
> +(let ()
> + ;; empty-serializer as literal/procedure tests
> +
> + ;; empty-serializer as literal
> + (define-configuration config-with-literal
> + (port
> + (port 80)
> + "Lorem Ipsum."
> + empty-serializer))
> +
> + ;; empty-serializer as procedure
> + (define-configuration config-with-proc
> + (port
> + (port 80)
> + "Lorem Ipsum."
> + (serializer empty-serializer)))
> +
> + (test-equal "empty-serializer as literal"
> + ""
> + (eval-gexp
> + (serialize-configuration (config-with-literal)
> + config-with-literal-fields)))
> +
> + (test-equal "empty-serializer as procedure"
> + ""
> + (eval-gexp
> + (serialize-configuration (config-with-proc)
> + config-with-proc-fields))))
> +
> +(let ()
> + ;; permutation tests
> +
> + (define-configuration config-san+empty-ser
> + (port
> + (port 80)
> + "Lorem Ipsum."
> + (sanitizer sanitize-port)
> + empty-serializer))
> +
> + (define-configuration config-san+ser
> + (port
> + (port 80)
> + "Lorem Ipsum."
> + (sanitizer sanitize-port)
> + (serializer (lambda _ "foo"))))
> +
> + (test-equal "default value, sanitizer, permutation"
> + 80
> + (config-san+empty-ser-port (config-san+empty-ser)))
> +
> + (test-equal "default value, serializer, permutation"
> + "foo"
> + (eval-gexp
> + (serialize-configuration (config-san+ser) config-san+ser-
> fields)))
> +
> + (test-equal "string value, sanitized to number, permutation"
> + 56
> + (config-san+ser-port (config-san+ser
> + (port "56"))))
> +
> + ;; ordering tests
> + (define-configuration config-ser+san
> + (port
> + (port 80)
> + "Lorem Ipsum."
> + (sanitizer sanitize-port)
> + (serializer (lambda _ "foo"))))
> +
> + (define-configuration config-empty-ser+san
> + (port
> + (port 80)
> + "Lorem Ipsum."
> + empty-serializer
> + (sanitizer sanitize-port)))
> +
> + (test-equal "default value, sanitizer, permutation 2"
> + 56
> + (config-empty-ser+san-port (config-empty-ser+san
> + (port "56"))))
> +
> + (test-equal "default value, serializer, permutation 2"
> + "foo"
> + (eval-gexp
> + (serialize-configuration (config-ser+san) config-ser+san-
> fields))))
> +
Also add a test case for double serializer and double sanitizer bugs.
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH 2/8] services: replace bare serializers with (serializer ...)
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
2023-03-20 17:07 ` [bug#62298] [PATCH 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
@ 2023-03-20 17:07 ` Bruno Victal
2023-03-20 17:07 ` [bug#62298] [PATCH 3/8] services: audio: remove redundant list-of-string? predicate Bruno Victal
` (8 subsequent siblings)
10 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-20 17:07 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/home/services/shells.scm (home-zsh-configuration)[environment-variables]: Use (serializer ...).
(home-bash-configuration)[aliases, environment-variables]: Ditto.
(home-fish-configuration)[abbreviations, aliases, environment-variables]: Ditto.
* gnu/services/audio.scm (mpd-configuration)[music-dir, playlist-dir, endpoints]
[address, inputs, archive-plugins, input-cache-size, decoders, filters, playlist-plugins]: Ditto.
* gnu/services/security.scm (fail2ban-jail-configuration)[backend, log-encoding, extra-content]: Ditto.
* tests/services/configuration.scm: Update tests. Add test for deprecated bare serializers.
---
gnu/home/services/shells.scm | 12 ++++-----
gnu/services/audio.scm | 44 ++++++++++++++++----------------
gnu/services/security.scm | 6 ++---
tests/services/configuration.scm | 11 +++++++-
4 files changed, 41 insertions(+), 32 deletions(-)
diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index 3326eb37f4..f05f2221d6 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -133,7 +133,7 @@ (define-configuration home-zsh-configuration
(environment-variables
(alist '())
"Association list of environment variables to set for the Zsh session."
- serialize-posix-env-vars)
+ (serializer serialize-posix-env-vars))
(zshenv
(text-config '())
"List of file-like objects, which will be added to @file{.zshenv}.
@@ -334,7 +334,7 @@ (define-configuration home-bash-configuration
rules for the @code{home-environment-variables-service-type} apply
here (@pxref{Essential Home Services}). The contents of this field will be
added after the contents of the @code{bash-profile} field."
- serialize-posix-env-vars)
+ (serializer serialize-posix-env-vars))
(aliases
(alist '())
"Association list of aliases to set for the Bash session. The aliases will be
@@ -351,7 +351,7 @@ (define-configuration home-bash-configuration
@example
alias ls=\"ls -alF\"
@end example"
- bash-serialize-aliases)
+ (serializer bash-serialize-aliases))
(bash-profile
(text-config '())
"List of file-like objects, which will be added to @file{.bash_profile}.
@@ -536,19 +536,19 @@ (define-configuration home-fish-configuration
(environment-variables
(alist '())
"Association list of environment variables to set in Fish."
- serialize-fish-env-vars)
+ (serializer serialize-fish-env-vars))
(aliases
(alist '())
"Association list of aliases for Fish, both the key and the value
should be a string. An alias is just a simple function that wraps a
command, If you want something more akin to @dfn{aliases} in POSIX
shells, see the @code{abbreviations} field."
- serialize-fish-aliases)
+ (serializer serialize-fish-aliases))
(abbreviations
(alist '())
"Association list of abbreviations for Fish. These are words that,
when typed in the shell, will automatically expand to the full text."
- serialize-fish-abbreviations))
+ (serializer serialize-fish-abbreviations)))
(define (fish-files-service config)
`(("fish/config.fish"
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index d55b804ba9..5f341fac0f 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -372,7 +372,7 @@ (define-configuration mpd-configuration
(music-dir ; TODO: deprecated, remove later
maybe-string
"The directory to scan for music files."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(playlist-directory
maybe-string
@@ -381,7 +381,7 @@ (define-configuration mpd-configuration
(playlist-dir ; TODO: deprecated, remove later
maybe-string
"The directory to store playlists."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(db-file
maybe-string
@@ -407,16 +407,16 @@ (define-configuration mpd-configuration
port is used.
To use a Unix domain socket, an absolute path or a path starting with @code{~}
can be specified here."
- (lambda (_ endpoints)
- (if (maybe-value-set? endpoints)
- (mpd-serialize-list-of-string "bind_to_address" endpoints)
- "")))
+ (serializer (lambda (_ endpoints)
+ (if (maybe-value-set? endpoints)
+ (mpd-serialize-list-of-string "bind_to_address" endpoints)
+ ""))))
(address ; TODO: deprecated, remove later
maybe-string
"The address that mpd will bind to.
To use a Unix domain socket, an absolute path can be specified here."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(database
maybe-mpd-plugin
@@ -433,29 +433,29 @@ (define-configuration mpd-configuration
(inputs
(list-of-mpd-plugin '())
"List of MPD input plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "input" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "input" x))))
(archive-plugins
(list-of-mpd-plugin '())
"List of MPD archive plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "archive_plugin" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "archive_plugin" x))))
(input-cache-size
maybe-string
"MPD input cache size."
- (lambda (_ x)
- (if (maybe-value-set? x)
- #~(string-append "\ninput_cache {\n"
- #$(mpd-serialize-string "size" x)
- "}\n") "")))
+ (serializer (lambda (_ x)
+ (if (maybe-value-set? x)
+ #~(string-append "\ninput_cache {\n"
+ #$(mpd-serialize-string "size" x)
+ "}\n") ""))))
(decoders
(list-of-mpd-plugin '())
"List of MPD decoder plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "decoder" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "decoder" x))))
(resampler
maybe-mpd-plugin
@@ -464,8 +464,8 @@ (define-configuration mpd-configuration
(filters
(list-of-mpd-plugin '())
"List of MPD filter plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "filter" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "filter" x))))
(outputs
(list-of-mpd-plugin-or-output (list (mpd-output)))
@@ -475,8 +475,8 @@ (define-configuration mpd-configuration
(playlist-plugins
(list-of-mpd-plugin '())
"List of MPD playlist plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x))))
(extra-options
(alist '())
diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 8116072920..e750bb468b 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -200,7 +200,7 @@ (define-configuration fail2ban-jail-configuration
"Backend to use to detect changes in the @code{log-path}. The default is
'auto. To consult the defaults of the jail configuration, refer to the
@file{/etc/fail2ban/jail.conf} file of the @code{fail2ban} package."
- fail2ban-jail-configuration-serialize-backend)
+ (serializer fail2ban-jail-configuration-serialize-backend))
(max-retry
maybe-integer
"The number of failures before a host get banned
@@ -269,7 +269,7 @@ (define-configuration fail2ban-jail-configuration
maybe-symbol
"The encoding of the log files handled by the jail.
Possible values are: @code{'ascii}, @code{'utf-8} and @code{'auto}."
- fail2ban-jail-configuration-serialize-log-encoding)
+ (serializer fail2ban-jail-configuration-serialize-log-encoding))
(log-path
(list-of-strings '())
"The file names of the log files to be monitored.")
@@ -280,7 +280,7 @@ (define-configuration fail2ban-jail-configuration
(text-config '())
"Extra content for the jail configuration, provided as a list of file-like
objects."
- serialize-text-config)
+ (serializer serialize-text-config))
(prefix fail2ban-jail-configuration-))
(define list-of-fail2ban-jail-configurations?
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index c5569a9e50..6abab2832f 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -82,6 +82,9 @@ (define (custom-number-serializer name value)
(format #f "~a = ~a;" name value))
(define-configuration serializable-configuration
+ (port (number 80) "The port number." (serializer custom-number-serializer)))
+
+(define-configuration serializable-configuration-deprecated
(port (number 80) "The port number." custom-number-serializer))
(test-assert "serialize-configuration"
@@ -89,8 +92,14 @@ (define-configuration serializable-configuration
(let ((config (serializable-configuration)))
(serialize-configuration config serializable-configuration-fields))))
+(test-assert "serialize-configuration [deprecated]"
+ (gexp?
+ (let ((config (serializable-configuration-deprecated)))
+ (serialize-configuration
+ config serializable-configuration-deprecated-fields))))
+
(define-configuration serializable-configuration
- (port (number 80) "The port number." custom-number-serializer)
+ (port (number 80) "The port number." (serializer custom-number-serializer))
(no-serialization))
(test-assert "serialize-configuration with no-serialization"
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH 3/8] services: audio: remove redundant list-of-string? predicate.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
2023-03-20 17:07 ` [bug#62298] [PATCH 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
2023-03-20 17:07 ` [bug#62298] [PATCH 2/8] services: replace bare serializers with (serializer ...) Bruno Victal
@ 2023-03-20 17:07 ` Bruno Victal
2023-03-20 17:07 ` [bug#62298] [PATCH 4/8] services: mympd: Require 'syslog service when configured to log to syslog Bruno Victal
` (7 subsequent siblings)
10 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-20 17:07 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
Use list-of-strings? predicate defined in (gnu services configuration).
* gnu/services/audio.scm (list-of-string?): Remove predicate.
(mpd-serialize-list-of-string): Rename procedure to ...
(mpd-serialize-list-of-strings): ... this.
(mpd-configuration)[environment-variables]: Switch to list-of-strings.
[endpoints]: Switch to maybe-list-of-strings.
(mympd-ip-acl)[allow, deny]: Switch to list-of-strings.
(mympd-serialize-configuration): Rename serialize-list-of-string to serialize-list-of-strings.
* doc/guix.texi (Audio Services): Update it.
---
doc/guix.texi | 8 ++++----
gnu/services/audio.scm | 25 +++++++++++--------------
2 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index f84cadeeda..7927e3166b 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33474,7 +33474,7 @@ Audio Services
This is a list of symbols naming Shepherd services that this service
will depend on.
-@item @code{environment-variables} (default: @code{()}) (type: list-of-string)
+@item @code{environment-variables} (default: @code{()}) (type: list-of-strings)
A list of strings specifying environment variables.
@item @code{log-file} (default: @code{"/var/log/mpd/log"}) (type: maybe-string)
@@ -33505,7 +33505,7 @@ Audio Services
@item @code{default-port} (default: @code{6600}) (type: maybe-integer)
The default port to run mpd on.
-@item @code{endpoints} (type: maybe-list-of-string)
+@item @code{endpoints} (type: maybe-list-of-strings)
The addresses that mpd will bind to. A port different from @var{default-port}
may be specified, e.g. @code{localhost:6602} and IPv6 addresses must be
enclosed in square brackets when a different port is used.
@@ -33781,10 +33781,10 @@ Audio Services
Available @code{mympd-ip-acl} fields are:
@table @asis
-@item @code{allow} (default: @code{()}) (type: list-of-string)
+@item @code{allow} (default: @code{()}) (type: list-of-strings)
Allowed IP addresses.
-@item @code{deny} (default: @code{()}) (type: list-of-string)
+@item @code{deny} (default: @code{()}) (type: list-of-strings)
Disallowed IP addresses.
@end table
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 5f341fac0f..e9ecccd614 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -2,7 +2,7 @@
;;; Copyright © 2017 Peter Mikkelsen <petermikkelsen10@gmail.com>
;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
;;; Copyright © 2020 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2022 Bruno Victal <mirai@makinata.eu>
+;;; Copyright © 2022–2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -137,9 +137,6 @@ (define (uglify-field-name field-name)
str)
#\-) "_")))
-(define list-of-string?
- (list-of string?))
-
(define list-of-symbol?
(list-of symbol?))
@@ -159,11 +156,11 @@ (define (mpd-serialize-alist field-name value)
(define mpd-serialize-string mpd-serialize-field)
(define mpd-serialize-boolean mpd-serialize-field)
-(define (mpd-serialize-list-of-string field-name value)
+(define (mpd-serialize-list-of-strings field-name value)
#~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
(define-maybe string (prefix mpd-))
-(define-maybe list-of-string (prefix mpd-))
+(define-maybe list-of-strings (prefix mpd-))
(define-maybe boolean (prefix mpd-))
;;; TODO: Procedures for deprecated fields, to be removed.
@@ -349,7 +346,7 @@ (define-configuration mpd-configuration
empty-serializer)
(environment-variables
- (list-of-string '())
+ (list-of-strings '())
"A list of strings specifying environment variables."
empty-serializer)
@@ -400,7 +397,7 @@ (define-configuration mpd-configuration
"The default port to run mpd on.")
(endpoints
- maybe-list-of-string
+ maybe-list-of-strings
"The addresses that mpd will bind to. A port different from
@var{default-port} may be specified, e.g. @code{localhost:6602} and
IPv6 addresses must be enclosed in square brackets when a different
@@ -409,7 +406,7 @@ (define-configuration mpd-configuration
can be specified here."
(serializer (lambda (_ endpoints)
(if (maybe-value-set? endpoints)
- (mpd-serialize-list-of-string "bind_to_address" endpoints)
+ (mpd-serialize-list-of-strings "bind_to_address" endpoints)
""))))
(address ; TODO: deprecated, remove later
@@ -581,11 +578,11 @@ (define (string-or-symbol? x)
(define-configuration/no-serialization mympd-ip-acl
(allow
- (list-of-string '())
+ (list-of-strings '())
"Allowed IP addresses.")
(deny
- (list-of-string '())
+ (list-of-strings '())
"Disallowed IP addresses."))
(define-maybe/no-serialization integer)
@@ -707,12 +704,12 @@ (define (mympd-serialize-configuration config)
((? string? val) val)))
(define (ip-acl-serialize-configuration config)
- (define (serialize-list-of-string prefix lst)
+ (define (serialize-list-of-strings prefix lst)
(map (cut format #f "~a~a" prefix <>) lst))
(string-join
(append
- (serialize-list-of-string "+" (mympd-ip-acl-allow config))
- (serialize-list-of-string "-" (mympd-ip-acl-deny config))) ","))
+ (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/.
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH 4/8] services: mympd: Require 'syslog service when configured to log to syslog.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
` (2 preceding siblings ...)
2023-03-20 17:07 ` [bug#62298] [PATCH 3/8] services: audio: remove redundant list-of-string? predicate Bruno Victal
@ 2023-03-20 17:07 ` Bruno Victal
2023-03-20 17:07 ` [bug#62298] [PATCH 5/8] services: mpd: Fix unintentional API breakage for mixer-type field Bruno Victal
` (6 subsequent siblings)
10 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-20 17:07 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (mympd-shepherd-service): Depend on 'syslog when
configured to log to syslog.
---
gnu/services/audio.scm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index e9ecccd614..e5b065a479 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -749,7 +749,9 @@ (define (mympd-shepherd-service config)
(let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level)))
(shepherd-service
(documentation "Run the myMPD daemon.")
- (requirement `(loopback user-processes ,@shepherd-requirement))
+ (requirement `(loopback user-processes
+ ,@(if (eqv? log-to 'syslog) '(syslog) '())
+ ,@shepherd-requirement))
(provision '(mympd))
(start #~(begin
(let* ((pw (getpwnam #$user))
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH 5/8] services: mpd: Fix unintentional API breakage for mixer-type field.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
` (3 preceding siblings ...)
2023-03-20 17:07 ` [bug#62298] [PATCH 4/8] services: mympd: Require 'syslog service when configured to log to syslog Bruno Victal
@ 2023-03-20 17:07 ` Bruno Victal
2023-03-20 17:07 ` [bug#62298] [PATCH 6/8] services: mpd: Set PulseAudio related variables as default value for environment-variables field Bruno Victal
` (5 subsequent siblings)
10 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-20 17:07 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (mpd-output)[mixer-type]: Use sanitizer to
accept both strings and symbols as values.
---
gnu/services/audio.scm | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index e5b065a479..ec6b3c5466 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,6 +140,11 @@ (define (uglify-field-name field-name)
(define list-of-symbol?
(list-of symbol?))
+\f
+;;;
+;;; MPD
+;;;
+
(define (mpd-serialize-field field-name value)
(let ((field (if (string? field-name) field-name
(uglify-field-name field-name)))
@@ -294,7 +299,17 @@ (define-configuration mpd-output
for this audio output: the @code{hardware} mixer, the @code{software}
mixer, the @code{null} mixer (allows setting the volume, but with no
effect; this can be used as a trick to implement an external mixer
-External Mixer) or no mixer (@code{none}).")
+External Mixer) or no mixer (@code{none})."
+ (sanitizer
+ (lambda (x) ; TODO: deprecated, remove me later.
+ (cond
+ ((symbol? x)
+ (warning (G_ "symbol value for 'mixer-type' is deprecated, \
+use string instead~%"))
+ (symbol->string x))
+ ((string? x) x)
+ (else
+ (leave (G_ "invalid input for 'mixer-type'~%")))))))
(replay-gain-handler
maybe-string
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH 6/8] services: mpd: Set PulseAudio related variables as default value for environment-variables field.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
` (4 preceding siblings ...)
2023-03-20 17:07 ` [bug#62298] [PATCH 5/8] services: mpd: Fix unintentional API breakage for mixer-type field Bruno Victal
@ 2023-03-20 17:07 ` Bruno Victal
2023-03-20 17:07 ` [bug#62298] [PATCH 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
` (4 subsequent siblings)
10 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-20 17:07 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
These variables are necessary for PulseAudio to work properly out-of-the-box
for 'non-interactive' users.
* doc/guix.texi (Audio Services): Update environment-variables field description for
mpd-configuration data type.
* gnu/services/audio.scm (mpd-configuration)[environment-variables]: Set
PULSE_CLIENTCONFIG and PULSE_CONFIG environment variables to the system-wide
PulseAudio configuration.
---
doc/guix.texi | 2 +-
gnu/services/audio.scm | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 7927e3166b..df424c561f 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33474,7 +33474,7 @@ Audio Services
This is a list of symbols naming Shepherd services that this service
will depend on.
-@item @code{environment-variables} (default: @code{()}) (type: list-of-strings)
+@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)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index ec6b3c5466..0682367358 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -361,7 +361,8 @@ (define-configuration mpd-configuration
empty-serializer)
(environment-variables
- (list-of-strings '())
+ (list-of-strings '("PULSE_CLIENTCONFIG=/etc/pulse/client.conf"
+ "PULSE_CONFIG=/etc/pulse/daemon.conf"))
"A list of strings specifying environment variables."
empty-serializer)
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
` (5 preceding siblings ...)
2023-03-20 17:07 ` [bug#62298] [PATCH 6/8] services: mpd: Set PulseAudio related variables as default value for environment-variables field Bruno Victal
@ 2023-03-20 17:07 ` Bruno Victal
2023-03-20 19:33 ` Liliana Marie Prikler
2023-03-20 17:07 ` [bug#62298] [PATCH 8/8] services: mympd: " Bruno Victal
` (3 subsequent siblings)
10 siblings, 1 reply; 47+ messages in thread
From: Bruno Victal @ 2023-03-20 17:07 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
Deprecate using strings for these fields and prefer user-account (resp. user-group)
instead to avoid duplication within account-service-type.
If a string value is encountered, it is ignored and a predefined variable
is used instead. This is essentially a rollback to how it used to be before
'5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1'.
Fixes #61570 <https://issues.guix.gnu.org/61570>.
* gnu/services/audio.scm (mpd-serialize-user-account, mpd-serialize-user-group)
(mpd-user-sanitizer, mpd-group-sanitizer): New procedure.
(%mpd-user, %mpd-group): New variable.
(mpd-configuration)[user, group]: Set value type to user-account (resp. user-group).
(mpd-shepherd-service): Adapt for user-account values in user field.
(mpd-accounts): Adapt for user-account (resp. user-group) in user (resp. group) field.
* doc/guix.texi (Audio Services): Update documentation.
---
doc/guix.texi | 4 +--
gnu/services/audio.scm | 70 ++++++++++++++++++++++++++++++++----------
2 files changed, 55 insertions(+), 19 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index df424c561f..ecc520397c 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33464,10 +33464,10 @@ Audio Services
@item @code{package} (default: @code{mpd}) (type: file-like)
The MPD package.
-@item @code{user} (default: @code{"mpd"}) (type: string)
+@item @code{user} (type: maybe-user-account)
The user to run mpd as.
-@item @code{group} (default: @code{"mpd"}) (type: string)
+@item @code{group} (type: maybe-user-group)
The group to run mpd as.
@item @code{shepherd-requirement} (default: @code{()}) (type: list-of-symbol)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 0682367358..eaee9b1536 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -164,9 +164,32 @@ (define mpd-serialize-boolean mpd-serialize-field)
(define (mpd-serialize-list-of-strings field-name value)
#~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
+(define (mpd-serialize-user-account field-name value)
+ (mpd-serialize-string field-name (user-account-name value)))
+
+(define (mpd-serialize-user-group field-name value)
+ (mpd-serialize-string field-name (user-group-name value)))
+
(define-maybe string (prefix mpd-))
(define-maybe list-of-strings (prefix mpd-))
(define-maybe boolean (prefix mpd-))
+(define-maybe user-account (prefix mpd-))
+(define-maybe user-group (prefix mpd-))
+
+(define %mpd-user
+ (user-account
+ (name "mpd")
+ (group "mpd")
+ (system? #t)
+ (comment "Music Player Daemon (MPD) user")
+ ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
+ (home-directory "/var/lib/mpd")
+ (shell (file-append shadow "/sbin/nologin"))))
+
+(define %mpd-group
+ (user-group
+ (name "mpd")
+ (system? #t)))
;;; TODO: Procedures for deprecated fields, to be removed.
@@ -197,6 +220,26 @@ (define (mpd-serialize-port field-name value)
(define-maybe port (prefix mpd-))
+;;; procedures for unsupported value types, to be removed.
+
+(define (mpd-user-sanitizer value)
+ (cond ((user-account? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'user' is not supported, use \
+user-account instead. Ignoring this value~%"))
+ %mpd-user)
+ (else
+ (leave (G_ "'~a' is not a valid value for 'user'~%") value))))
+
+(define (mpd-group-sanitizer value)
+ (cond ((user-group? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'group' is not supported, use \
+user-group instead. Ignoring this value~%"))
+ %mpd-group)
+ (else
+ (leave (G_ "'~a' is not a valid value for 'group'~%") value))))
+
;;;
;; Generic MPD plugin record, lists only the most prevalent fields.
@@ -347,12 +390,14 @@ (define-configuration mpd-configuration
empty-serializer)
(user
- (string "mpd")
- "The user to run mpd as.")
+ (maybe-user-account %mpd-user)
+ "The user to run mpd as."
+ (sanitizer mpd-user-sanitizer))
(group
- (string "mpd")
- "The group to run mpd as.")
+ (maybe-user-group %mpd-group)
+ "The group to run mpd as."
+ (sanitizer mpd-group-sanitizer))
(shepherd-requirement
(list-of-symbol '())
@@ -516,7 +561,8 @@ (define (mpd-shepherd-service config)
log-file playlist-directory
db-file state-file sticker-file
environment-variables)
- (let* ((config-file (mpd-serialize-configuration config)))
+ (let ((config-file (mpd-serialize-configuration config))
+ (username (user-account-name user)))
(shepherd-service
(documentation "Run the MPD (Music Player Daemon)")
(requirement `(user-processes loopback ,@shepherd-requirement))
@@ -525,7 +571,7 @@ (define (mpd-shepherd-service config)
(and=> #$(maybe-value log-file)
(compose mkdir-p dirname))
- (let ((user (getpw #$user)))
+ (let ((user (getpw #$username)))
(for-each
(lambda (x)
(when (and x (not (file-exists? x)))
@@ -559,17 +605,7 @@ (define (mpd-shepherd-service config)
(define (mpd-accounts config)
(match-record config <mpd-configuration> (user group)
- (list (user-group
- (name group)
- (system? #t))
- (user-account
- (name user)
- (group group)
- (system? #t)
- (comment "Music Player Daemon (MPD) user")
- ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
- (home-directory "/var/lib/mpd")
- (shell (file-append shadow "/sbin/nologin"))))))
+ (list user group)))
(define mpd-service-type
(service-type
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-20 17:07 ` [bug#62298] [PATCH 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
@ 2023-03-20 19:33 ` Liliana Marie Prikler
2023-03-21 2:10 ` Bruno Victal
0 siblings, 1 reply; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-20 19:33 UTC (permalink / raw)
To: Bruno Victal, 62298; +Cc: ludo, maxim.cournoyer
Am Montag, dem 20.03.2023 um 17:07 +0000 schrieb Bruno Victal:
> Deprecate using strings for these fields and prefer user-account
> (resp. user-group) instead to avoid duplication within account-
> service-type. If a string value is encountered, it is ignored and a
> predefined variable is used instead. This is essentially a rollback
> to how it used to be before
> '5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1'.
I already wrote this in private in IRC, but falling back to a constant
when a string value is given is very silly. IIUC the reason to do so
is because you would need to sanitize the user account and group at the
same time so that the former has access to the latter.
I think it's possible to use one of the following approaches to get a
better result:
1. In (mpd-accounts), check if the user group equals the group name and
raise a warning (or error) if not.
2. Use a special unique symbol, e.g. (make-symbol "%mpd-group") to
denote the value to be lazily inserted by the serializer.
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-20 19:33 ` Liliana Marie Prikler
@ 2023-03-21 2:10 ` Bruno Victal
2023-03-21 5:30 ` Liliana Marie Prikler
0 siblings, 1 reply; 47+ messages in thread
From: Bruno Victal @ 2023-03-21 2:10 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: ludo, 62298, maxim.cournoyer
Hi Liliana,
On 2023-03-20 19:33, Liliana Marie Prikler wrote:
> Am Montag, dem 20.03.2023 um 17:07 +0000 schrieb Bruno Victal:
>> Deprecate using strings for these fields and prefer user-account
>> (resp. user-group) instead to avoid duplication within account-
>> service-type. If a string value is encountered, it is ignored and a
>> predefined variable is used instead. This is essentially a rollback
>> to how it used to be before
>> '5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1'.
> I already wrote this in private in IRC, but falling back to a constant
> when a string value is given is very silly. IIUC the reason to do so
> is because you would need to sanitize the user account and group at the
> same time so that the former has access to the latter.
>
>
> I think it's possible to use one of the following approaches to get a
> better result:
> 1. In (mpd-accounts), check if the user group equals the group name and
> raise a warning (or error) if not.
> 2. Use a special unique symbol, e.g. (make-symbol "%mpd-group") to
> denote the value to be lazily inserted by the serializer.
After giving some thought to this, IMO I think it's simply uninteresting for these
fields to accept string values.
Prior to the 5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1 refactor, the names were hardcoded
and the refactor allowed them to be changed.
But with the observed interactions in [1], it became clear that:
1. This service isn't meant to be used with a non-interactive user, a home shepherd service
should be used instead. (granted the bug isn't due to this, it merely surfaced up here.
Setting group to “nobody” would have also caused the same kind of problem.)
2. Accepting strings was deemed undesirable since it then results in a “race” between
user-accounts from operating-system and account-service-type extensions (or even among the extensions),
with the winner getting to “build” the account as it sees fit.
In the end, the daemon requires a user-account + user-group to work (unless you're in the
mood for running it as root?), so something still has to be made.
The choices that I think make sense for user/group fields are:
1. Leave it with a default value. The service creates what it needs.
2. Give it a user-account/group. This is considered an _advanced_ use-case and it's highly likely (though not mandatory)
to be used within a let-form. You use this when you need fine control over the account properties.
Accepting strings is simply uninteresting (or bad) since:
* A string doesn't uniquely identify an account and results in buggy behavior [1].
* Since the string values are only used to set the 'name' of the user-account/group records, which
is specific to the service as they're created within the mpd-account procedure, it's simply setting
a vanity value. It's as interesting as allowing the filename in (mixed-text-file "mpd.conf" ...)
to be set by the user.
* It's clearly unsanitizable since it would require accessing other fields.
Monkeying within (mpd-accounts) with special symbols just obfuscates the code with
no clear benefits to be had, in addition to defeating the point of having a sanitizer in the first place.
I fail to see the utility in ever accepting strings here for what amounts to a vanity change in 'ps aux'
output. Maybe my imagination is failing here but I really don't think it's worth the trouble to support strings here.
This vanity change is still achievable with some extra-typing if someone really wishes it.
As such, I think the best course of action is to simply use user-account/group record objects from now on,
with string usages deprecated and properly communicated to the user that they're not supported and are ignored. (a non API breaking change)
[1]: <https://issues.guix.gnu.org/61570>
Cheers,
Bruno
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-21 2:10 ` Bruno Victal
@ 2023-03-21 5:30 ` Liliana Marie Prikler
0 siblings, 0 replies; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-21 5:30 UTC (permalink / raw)
To: Bruno Victal; +Cc: ludo, 62298, maxim.cournoyer
Hi Bruno,
Am Dienstag, dem 21.03.2023 um 02:10 +0000 schrieb Bruno Victal:
> After giving some thought to this, IMO I think it's simply
> uninteresting for these fields to accept string values.
> Prior to the 5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1 refactor, the
> names were hardcoded and the refactor allowed them to be changed.
I think it's a little late to come to this realization. Note how my
prior attempt at fixing 61570 was delayed for more than a month so that
a proper sanitizer can be implemented and would have had a better user
interface than what you are currently proposing.
> Accepting strings is simply uninteresting (or bad) since:
>
> * A string doesn't uniquely identify an account and results in buggy
> behavior [1].
>
> * Since the string values are only used to set the 'name' of the
> user-account/group records, which is specific to the service as
> they're created within the mpd-account procedure, it's simply setting
> a vanity value. It's as interesting as allowing the filename in
> (mixed-text-file "mpd.conf" ...) to be set by the user.
>
> * It's clearly unsanitizable since it would require accessing other
> fields. Monkeying within (mpd-accounts) with special symbols just
> obfuscates the code with no clear benefits to be had, in addition to
> defeating the point of having a sanitizer in the first place.
>
>
> I fail to see the utility in ever accepting strings here for what
> amounts to a vanity change in 'ps aux' output.
Need I remind you that the original concern was about backwards API
compatibility? Yes, accepting strings and doing things with them is
broken for the reasons you mentioned and there should be a deprecation
warning about this. But not heeding the user values is silly and you
should still set those vanity values for the sake of vanity itself.
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
` (6 preceding siblings ...)
2023-03-20 17:07 ` [bug#62298] [PATCH 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
@ 2023-03-20 17:07 ` Bruno Victal
2023-03-20 19:33 ` Liliana Marie Prikler
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
` (2 subsequent siblings)
10 siblings, 1 reply; 47+ messages in thread
From: Bruno Victal @ 2023-03-20 17:07 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (%mympd-user, %mympd-group): New variable.
(mympd-user-sanitizer, mympd-group-sanitizer): New procedure.
(mympd-configuration)[user, group]: Set value type to user-account (resp. user-group).
(mympd-serialize-configuration): Adapt for user-account values in user field.
(mympd-accounts): Adapt for user-account (resp. user-group) in user (resp. group) field.
---
doc/guix.texi | 4 +--
gnu/services/audio.scm | 63 +++++++++++++++++++++++++++++++++---------
2 files changed, 52 insertions(+), 15 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index ecc520397c..7c7e45ec8e 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33705,10 +33705,10 @@ Audio Services
This is a list of symbols naming Shepherd services that this service
will depend on.
-@item @code{user} (default: @code{"mympd"}) (type: string)
+@item @code{user} (type: maybe-user-account)
Owner of the @command{mympd} process.
-@item @code{group} (default: @code{"nogroup"}) (type: string)
+@item @code{group} (type: maybe-user-group)
Owner group of the @command{mympd} process.
@item @code{work-directory} (default: @code{"/var/lib/mympd"}) (type: string)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index eaee9b1536..9211cbcc52 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -640,6 +640,47 @@ (define-configuration/no-serialization mympd-ip-acl
(define-maybe/no-serialization integer)
(define-maybe/no-serialization mympd-ip-acl)
+;; XXX: These will shadow the previous definition used by mpd
+;; and cause warnings to be shown. Maybe split the file
+;; into audio/mpd.scm and audio/mympd.scm ?
+#;(define-maybe/no-serialization user-account)
+#;(define-maybe/no-serialization user-group)
+
+(define %mympd-user
+ (user-account
+ (name "mympd")
+ (group "mympd")
+ (system? #t)
+ (comment "myMPD user")
+ (home-directory "/var/empty")
+ (shell (file-append shadow "/sbin/nologin"))))
+
+(define %mympd-group
+ (user-group
+ (name "mympd")
+ (system? #t)))
+
+;;; TODO: procedures for unsupported value types, to be removed.
+(define (mympd-user-sanitizer value)
+ (cond ((user-account? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'user' is not supported, use \
+user-account instead. Ignoring this value~%"))
+ %mympd-user)
+ (else
+ (leave (G_ "'~a' is not a valid value for 'user'~%") value))))
+
+(define (mympd-group-sanitizer value)
+ (cond ((user-group? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'group' is not supported, use \
+user-group instead. Ignoring this value~%"))
+ %mympd-group)
+ (else
+ (leave (G_ "'~a' is not a valid value for 'group'~%") 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
@@ -657,13 +698,15 @@ (define-configuration/no-serialization mympd-configuration
empty-serializer)
(user
- (string "mympd")
+ (maybe-user-account %mympd-user)
"Owner of the @command{mympd} process."
+ (sanitizer mympd-user-sanitizer)
empty-serializer)
(group
- (string "nogroup")
+ (maybe-user-group %mympd-group)
"Owner group of the @command{mympd} process."
+ (sanitizer mympd-group-sanitizer)
empty-serializer)
(work-directory
@@ -798,7 +841,8 @@ (define (mympd-shepherd-service config)
(match-record config <mympd-configuration> (package shepherd-requirement
user work-directory
cache-directory log-level log-to)
- (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level)))
+ (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level))
+ (username (user-account-name user)))
(shepherd-service
(documentation "Run the myMPD daemon.")
(requirement `(loopback user-processes
@@ -806,7 +850,7 @@ (define (mympd-shepherd-service config)
,@shepherd-requirement))
(provision '(mympd))
(start #~(begin
- (let* ((pw (getpwnam #$user))
+ (let* ((pw (getpwnam #$username))
(uid (passwd:uid pw))
(gid (passwd:gid pw)))
(for-each (lambda (dir)
@@ -816,7 +860,7 @@ (define (mympd-shepherd-service config)
(make-forkexec-constructor
`(#$(file-append package "/bin/mympd")
- "--user" #$user
+ "--user" #$username
#$@(if (eqv? log-to 'syslog) '("--syslog") '())
"--workdir" #$work-directory
"--cachedir" #$cache-directory)
@@ -826,14 +870,7 @@ (define (mympd-shepherd-service config)
(define (mympd-accounts config)
(match-record config <mympd-configuration> (user group)
- (list (user-group (name group)
- (system? #t))
- (user-account (name user)
- (group group)
- (system? #t)
- (comment "myMPD user")
- (home-directory "/var/empty")
- (shell (file-append shadow "/sbin/nologin"))))))
+ (list user group)))
(define (mympd-log-rotation config)
(match-record config <mympd-configuration> (log-to)
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-20 17:07 ` [bug#62298] [PATCH 8/8] services: mympd: " Bruno Victal
@ 2023-03-20 19:33 ` Liliana Marie Prikler
0 siblings, 0 replies; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-20 19:33 UTC (permalink / raw)
To: Bruno Victal, 62298; +Cc: ludo, maxim.cournoyer
Am Montag, dem 20.03.2023 um 17:07 +0000 schrieb Bruno Victal:
> * gnu/services/audio.scm (%mympd-user, %mympd-group): New variable.
> (mympd-user-sanitizer, mympd-group-sanitizer): New procedure.
> (mympd-configuration)[user, group]: Set value type to user-account
> (resp. user-group).
> (mympd-serialize-configuration): Adapt for user-account values in
> user field.
> (mympd-accounts): Adapt for user-account (resp. user-group) in user
> (resp. group) field.
Same as 7/8, don't use glorified constants.
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
` (7 preceding siblings ...)
2023-03-20 17:07 ` [bug#62298] [PATCH 8/8] services: mympd: " Bruno Victal
@ 2023-03-23 15:02 ` Bruno Victal
2023-03-23 15:02 ` [bug#62298] [PATCH v2 2/8] services: replace bare serializers with (serializer ...) Bruno Victal
` (8 more replies)
2023-03-25 0:46 ` [bug#62298] [PATCH v3 1/5] " Bruno Victal
2023-03-26 18:41 ` [bug#62298] [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Bruno Victal
10 siblings, 9 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-23 15:02 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
This changes the 'custom-serializer' field into a generic
'extra-args' field that can be extended to support new literals.
With this mechanism, the literals 'sanitizer' allow for user-defined
sanitizer procedures while the 'serializer' literal is used for
custom serializer procedures.
The 'empty-serializer' was also added as a 'literal' and can be used
just like it was previously.
With the repurposing of 'custom-serializer' into 'extra-args', to prevent
intolerable confusion, the custom serializer procedures should be
specified using the new 'literals' approach, with the previous “style”
being considered deprecated.
* gnu/services/configuration.scm (define-configuration-helper):
Rename 'custom-serializer' to 'extra-args'.
Add support for literals 'sanitizer', 'serializer' and 'empty-serializer'.
Rename procedure 'field-sanitizer' to 'default-field-sanitizer' to avoid syntax clash.
Only define default field sanitizers if user-defined ones are absent.
(normalize-extra-args): New procedure.
(<configuration-field>)[sanitizer]: New field.
* doc/guix.texi (Complex Configurations): Document the newly added literals.
* tests/services/configuration.scm: Add tests for the new literals.
---
doc/guix.texi | 30 ++++-
gnu/services/configuration.scm | 91 +++++++++++----
tests/services/configuration.scm | 185 ++++++++++++++++++++++++++++++-
3 files changed, 280 insertions(+), 26 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index dfdb26103a..1609e508ef 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -41216,7 +41216,7 @@ Complex Configurations
(@var{field-name}
(@var{type} @var{default-value})
@var{documentation}
- @var{serializer})
+ (serializer @var{serializer}))
(@var{field-name}
(@var{type})
@@ -41225,7 +41225,18 @@ Complex Configurations
(@var{field-name}
(@var{type})
@var{documentation}
- @var{serializer})
+ (serializer @var{serializer}))
+
+(@var{field-name}
+ (@var{type})
+ @var{documentation}
+ (sanitizer @var{sanitizer})
+
+(@var{field-name}
+ (@var{type})
+ @var{documentation}
+ (sanitizer @var{sanitizer})
+ (serializer @var{serializer}))
@end example
@var{field-name} is an identifier that denotes the name of the field in
@@ -41248,6 +41259,21 @@ Complex Configurations
@var{documentation} is a string formatted with Texinfo syntax which
should provide a description of what setting this field does.
+@var{sanitizer} is the name of a procedure which takes one argument,
+a user-supplied value, and returns a ``sanitized'' value for the field.
+If none is specified, the predicate @code{@var{type}?} is automatically
+used to construct an internal sanitizer that asserts the type correctness
+of the field value.
+
+An example of a sanitizer for a field that accepts both strings and
+symbols looks like this:
+@lisp
+(define (sanitize-foo value)
+ (cond ((string? value) value)
+ ((symbol? value) (symbol->string value))
+ (else (throw 'bad! value))))
+@end lisp
+
@var{serializer} is the name of a procedure which takes two arguments,
the first is the name of the field, and the second is the value
corresponding to the field. The procedure should return a string or
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 174c2f20d2..409d4cef00 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -6,6 +6,7 @@
;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -28,7 +29,8 @@ (define-module (gnu services configuration)
#:use-module (guix gexp)
#:use-module ((guix utils) #:select (source-properties->location))
#:use-module ((guix diagnostics)
- #:select (formatted-message location-file &error-location))
+ #:select (formatted-message location-file &error-location
+ warning))
#:use-module ((guix modules) #:select (file-name->module-name))
#:use-module (guix i18n)
#:autoload (texinfo) (texi-fragment->stexi)
@@ -37,6 +39,7 @@ (define-module (gnu services configuration)
#:use-module (ice-9 format)
#:use-module (ice-9 match)
#:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-35)
#:export (configuration-field
@@ -44,6 +47,7 @@ (define-module (gnu services configuration)
configuration-field-type
configuration-missing-field
configuration-field-error
+ configuration-field-sanitizer
configuration-field-serializer
configuration-field-getter
configuration-field-default-value-thunk
@@ -116,6 +120,7 @@ (define-record-type* <configuration-field>
(type configuration-field-type)
(getter configuration-field-getter)
(predicate configuration-field-predicate)
+ (sanitizer configuration-field-sanitizer)
(serializer configuration-field-serializer)
(default-value-thunk configuration-field-default-value-thunk)
(documentation configuration-field-documentation))
@@ -181,11 +186,45 @@ (define (normalize-field-type+def s)
(values #'(field-type %unset-value)))))
(define (define-configuration-helper serialize? serializer-prefix syn)
+
+ (define (normalize-extra-args s)
+ (let loop ((s s)
+ (sanitizer* %unset-value)
+ (serializer* %unset-value))
+ (syntax-case s (sanitizer serializer empty-serializer)
+ (((sanitizer proc) tail ...)
+ (if (maybe-value-set? sanitizer*)
+ (syntax-violation 'sanitizer "duplicate entry"
+ #'proc)
+ (loop #'(tail ...) #'proc serializer*)))
+ (((serializer proc) tail ...)
+ (if (maybe-value-set? serializer*)
+ (syntax-violation 'serializer "duplicate or conflicting entry"
+ #'proc)
+ (loop #'(tail ...) sanitizer* #'proc)))
+ ((empty-serializer tail ...)
+ (if (maybe-value-set? serializer*)
+ (syntax-violation 'empty-serializer
+ "duplicate or conflicting entry" #f)
+ (loop #'(tail ...) sanitizer* #'empty-serializer)))
+ (() ; stop condition
+ (values (list sanitizer* serializer*)))
+ ((proc) ; TODO: deprecated, to be removed
+ (every (cut eq? <> #f)
+ (map maybe-value-set?
+ (list sanitizer* serializer*)))
+ (begin
+ (warning #f (G_ "specifying serializers after documentation is \
+deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
+ (values (list %unset-value #'proc)))))))
+
(syntax-case syn ()
- ((_ stem (field field-type+def doc custom-serializer ...) ...)
+ ((_ stem (field field-type+def doc extra-args ...) ...)
(with-syntax
((((field-type def) ...)
- (map normalize-field-type+def #'(field-type+def ...))))
+ (map normalize-field-type+def #'(field-type+def ...)))
+ (((sanitizer* serializer*) ...)
+ (map normalize-extra-args #'((extra-args ...) ...))))
(with-syntax
(((field-getter ...)
(map (lambda (field)
@@ -200,21 +239,18 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
((field-type default-value)
default-value))
#'((field-type def) ...)))
+ ((field-sanitizer ...)
+ (map maybe-value #'(sanitizer* ...)))
((field-serializer ...)
- (map (lambda (type custom-serializer)
+ (map (lambda (type proc)
(and serialize?
- (match custom-serializer
- ((serializer)
- serializer)
- (()
- (if serializer-prefix
- (id #'stem
- serializer-prefix
- #'serialize- type)
- (id #'stem #'serialize- type))))))
+ (or (maybe-value proc)
+ (if serializer-prefix
+ (id #'stem serializer-prefix #'serialize- type)
+ (id #'stem #'serialize- type)))))
#'(field-type ...)
- #'((custom-serializer ...) ...))))
- (define (field-sanitizer name pred)
+ #'(serializer* ...))))
+ (define (default-field-sanitizer name pred)
;; Define a macro for use as a record field sanitizer, where NAME
;; is the name of the field and PRED is the predicate that tells
;; whether a value is valid for this field.
@@ -235,21 +271,29 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
#`(begin
;; Define field validation macros.
- #,@(map field-sanitizer
- #'(field ...)
- #'(field-predicate ...))
+ #,@(filter-map (lambda (name pred sanitizer)
+ (if sanitizer
+ #f
+ (default-field-sanitizer name pred)))
+ #'(field ...)
+ #'(field-predicate ...)
+ #'(field-sanitizer ...))
(define-record-type* #,(id #'stem #'< #'stem #'>)
stem
#,(id #'stem #'make- #'stem)
#,(id #'stem #'stem #'?)
- #,@(map (lambda (name getter def)
- #`(#,name #,getter (default #,def)
+ #,@(map (lambda (name getter def sanitizer)
+ #`(#,name #,getter
+ (default #,def)
(sanitize
- #,(id #'stem #'validate- #'stem #'- name))))
+ #,(or sanitizer
+ (id #'stem
+ #'validate- #'stem #'- name)))))
#'(field ...)
#'(field-getter ...)
- #'(field-default ...))
+ #'(field-default ...)
+ #'(field-sanitizer ...))
(%location #,(id #'stem #'stem #'-source-location)
(default (and=> (current-source-location)
source-properties->location))
@@ -261,6 +305,9 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
(type 'field-type)
(getter field-getter)
(predicate field-predicate)
+ (sanitizer
+ (or field-sanitizer
+ (id #'stem #'validate- #'stem #'- #'field)))
(serializer field-serializer)
(default-value-thunk
(lambda ()
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 4f8a74dc8a..64b7bb1543 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -22,6 +23,7 @@ (define-module (tests services configuration)
#:use-module (gnu services configuration)
#:use-module (guix diagnostics)
#:use-module (guix gexp)
+ #:autoload (guix i18n) (G_)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-64))
@@ -46,14 +48,14 @@ (define-configuration port-configuration
(port-configuration-port (port-configuration)))
(test-equal "wrong type for a field"
- '("configuration.scm" 57 11) ;error location
+ '("configuration.scm" 59 11) ;error location
(guard (c ((configuration-error? c)
(let ((loc (error-location c)))
(list (basename (location-file loc))
(location-line loc)
(location-column loc)))))
(port-configuration
- ;; This is line 56; the test relies on line/column numbers!
+ ;; This is line 58; the test relies on line/column numbers!
(port "This is not a number!"))))
(define-configuration port-configuration-cs
@@ -109,6 +111,185 @@ (define-configuration configuration-with-prefix
(let ((config (configuration-with-prefix)))
(serialize-configuration config configuration-with-prefix-fields))))
+\f
+;;;
+;;; define-configuration macro, extra-args literals
+;;;
+
+(define (eval-gexp x)
+ "Get serialized config as string."
+ (eval (gexp->approximate-sexp x)
+ (current-module)))
+
+(define (port? value)
+ (or (string? value) (number? value)))
+
+(define (sanitize-port value)
+ (cond ((number? value) value)
+ ((string? value) (string->number value))
+ (else (raise (formatted-message (G_ "Bad value: ~a") value)))))
+
+(test-group "Basic sanitizer literal tests"
+ (define serialize-port serialize-number)
+
+ (define-configuration config-with-sanitizer
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)))
+
+ (test-equal "default value, sanitizer"
+ 80
+ (config-with-sanitizer-port (config-with-sanitizer)))
+
+ (test-equal "string value, sanitized to number"
+ 56
+ (config-with-sanitizer-port (config-with-sanitizer
+ (port "56"))))
+
+ (define (custom-serialize-port field-name value)
+ (number->string value))
+
+ (define-configuration config-serializer
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (serializer custom-serialize-port)))
+
+ (test-equal "default value, serializer literal"
+ "80"
+ (eval-gexp
+ (serialize-configuration (config-serializer)
+ config-serializer-fields))))
+
+(test-group "empty-serializer as literal/procedure tests"
+ ;; empty-serializer as literal
+ (define-configuration config-with-literal
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ empty-serializer))
+
+ ;; empty-serializer as procedure
+ (define-configuration config-with-proc
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (serializer empty-serializer)))
+
+ (test-equal "empty-serializer as literal"
+ ""
+ (eval-gexp
+ (serialize-configuration (config-with-literal)
+ config-with-literal-fields)))
+
+ (test-equal "empty-serializer as procedure"
+ ""
+ (eval-gexp
+ (serialize-configuration (config-with-proc)
+ config-with-proc-fields))))
+
+(test-group "permutation tests"
+ (define-configuration config-san+empty-ser
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ empty-serializer))
+
+ (define-configuration config-san+ser
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ (serializer (lambda _ "foo"))))
+
+ (test-equal "default value, sanitizer, permutation"
+ 80
+ (config-san+empty-ser-port (config-san+empty-ser)))
+
+ (test-equal "default value, serializer, permutation"
+ "foo"
+ (eval-gexp
+ (serialize-configuration (config-san+ser) config-san+ser-fields)))
+
+ (test-equal "string value sanitized to number, permutation"
+ 56
+ (config-san+ser-port (config-san+ser
+ (port "56"))))
+
+ ;; ordering tests
+ (define-configuration config-ser+san
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ (serializer (lambda _ "foo"))))
+
+ (define-configuration config-empty-ser+san
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ empty-serializer
+ (sanitizer sanitize-port)))
+
+ (test-equal "default value, sanitizer, permutation 2"
+ 56
+ (config-empty-ser+san-port (config-empty-ser+san
+ (port "56"))))
+
+ (test-equal "default value, serializer, permutation 2"
+ "foo"
+ (eval-gexp
+ (serialize-configuration (config-ser+san) config-ser+san-fields))))
+
+(test-group "duplicated/conflicting entries"
+ (test-error
+ "duplicate sanitizer" #t
+ (macroexpand '(define-configuration dupe-san
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (sanitizer (lambda () #t))
+ (sanitizer (lambda () #t))))))
+
+ (test-error
+ "duplicate serializer" #t
+ (macroexpand '(define-configuration dupe-ser
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (serializer (lambda _ ""))
+ (serializer (lambda _ ""))))))
+
+ (test-error
+ "conflicting use of serializer + empty-serializer" #t
+ (macroexpand '(define-configuration ser+empty-ser
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (serializer (lambda _ "lorem"))
+ empty-serializer)))))
+
+(test-group "Mix of deprecated and new syntax"
+ (test-error
+ "Mix of bare serializer and new syntax" #t
+ (macroexpand '(define-configuration mixed
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (sanitizer (lambda () #t))
+ (lambda _ "lorem")))))
+
+ (test-error
+ "Mix of bare serializer and new syntax, permutation)" #t
+ (macroexpand '(define-configuration mixed
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (lambda _ "lorem")
+ (sanitizer (lambda () #t)))))))
+
\f
;;;
;;; define-maybe macro.
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 2/8] services: replace bare serializers with (serializer ...)
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
@ 2023-03-23 15:02 ` Bruno Victal
2023-03-24 14:28 ` Maxim Cournoyer
2023-03-23 15:02 ` [bug#62298] [PATCH v2 3/8] services: audio: remove redundant list-of-string? predicate Bruno Victal
` (7 subsequent siblings)
8 siblings, 1 reply; 47+ messages in thread
From: Bruno Victal @ 2023-03-23 15:02 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/home/services/shells.scm (home-zsh-configuration)[environment-variables]: Use (serializer ...).
(home-bash-configuration)[aliases, environment-variables]: Ditto.
(home-fish-configuration)[abbreviations, aliases, environment-variables]: Ditto.
* gnu/services/audio.scm (mpd-configuration)[music-dir, playlist-dir, endpoints]
[address, inputs, archive-plugins, input-cache-size, decoders, filters, playlist-plugins]: Ditto.
* gnu/services/linux.scm (fstrim-configuration)[extra-arguments]: Ditto.
* gnu/services/security.scm (fail2ban-jail-configuration)[backend, log-encoding, extra-content]: Ditto.
* tests/services/configuration.scm: Update tests. Add test for deprecated bare serializers.
---
gnu/home/services/shells.scm | 12 ++++-----
gnu/services/audio.scm | 44 ++++++++++++++++----------------
gnu/services/linux.scm | 7 ++---
gnu/services/security.scm | 6 ++---
tests/services/configuration.scm | 11 +++++++-
5 files changed, 45 insertions(+), 35 deletions(-)
diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index 3326eb37f4..f05f2221d6 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -133,7 +133,7 @@ (define-configuration home-zsh-configuration
(environment-variables
(alist '())
"Association list of environment variables to set for the Zsh session."
- serialize-posix-env-vars)
+ (serializer serialize-posix-env-vars))
(zshenv
(text-config '())
"List of file-like objects, which will be added to @file{.zshenv}.
@@ -334,7 +334,7 @@ (define-configuration home-bash-configuration
rules for the @code{home-environment-variables-service-type} apply
here (@pxref{Essential Home Services}). The contents of this field will be
added after the contents of the @code{bash-profile} field."
- serialize-posix-env-vars)
+ (serializer serialize-posix-env-vars))
(aliases
(alist '())
"Association list of aliases to set for the Bash session. The aliases will be
@@ -351,7 +351,7 @@ (define-configuration home-bash-configuration
@example
alias ls=\"ls -alF\"
@end example"
- bash-serialize-aliases)
+ (serializer bash-serialize-aliases))
(bash-profile
(text-config '())
"List of file-like objects, which will be added to @file{.bash_profile}.
@@ -536,19 +536,19 @@ (define-configuration home-fish-configuration
(environment-variables
(alist '())
"Association list of environment variables to set in Fish."
- serialize-fish-env-vars)
+ (serializer serialize-fish-env-vars))
(aliases
(alist '())
"Association list of aliases for Fish, both the key and the value
should be a string. An alias is just a simple function that wraps a
command, If you want something more akin to @dfn{aliases} in POSIX
shells, see the @code{abbreviations} field."
- serialize-fish-aliases)
+ (serializer serialize-fish-aliases))
(abbreviations
(alist '())
"Association list of abbreviations for Fish. These are words that,
when typed in the shell, will automatically expand to the full text."
- serialize-fish-abbreviations))
+ (serializer serialize-fish-abbreviations)))
(define (fish-files-service config)
`(("fish/config.fish"
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index d55b804ba9..5f341fac0f 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -372,7 +372,7 @@ (define-configuration mpd-configuration
(music-dir ; TODO: deprecated, remove later
maybe-string
"The directory to scan for music files."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(playlist-directory
maybe-string
@@ -381,7 +381,7 @@ (define-configuration mpd-configuration
(playlist-dir ; TODO: deprecated, remove later
maybe-string
"The directory to store playlists."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(db-file
maybe-string
@@ -407,16 +407,16 @@ (define-configuration mpd-configuration
port is used.
To use a Unix domain socket, an absolute path or a path starting with @code{~}
can be specified here."
- (lambda (_ endpoints)
- (if (maybe-value-set? endpoints)
- (mpd-serialize-list-of-string "bind_to_address" endpoints)
- "")))
+ (serializer (lambda (_ endpoints)
+ (if (maybe-value-set? endpoints)
+ (mpd-serialize-list-of-string "bind_to_address" endpoints)
+ ""))))
(address ; TODO: deprecated, remove later
maybe-string
"The address that mpd will bind to.
To use a Unix domain socket, an absolute path can be specified here."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(database
maybe-mpd-plugin
@@ -433,29 +433,29 @@ (define-configuration mpd-configuration
(inputs
(list-of-mpd-plugin '())
"List of MPD input plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "input" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "input" x))))
(archive-plugins
(list-of-mpd-plugin '())
"List of MPD archive plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "archive_plugin" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "archive_plugin" x))))
(input-cache-size
maybe-string
"MPD input cache size."
- (lambda (_ x)
- (if (maybe-value-set? x)
- #~(string-append "\ninput_cache {\n"
- #$(mpd-serialize-string "size" x)
- "}\n") "")))
+ (serializer (lambda (_ x)
+ (if (maybe-value-set? x)
+ #~(string-append "\ninput_cache {\n"
+ #$(mpd-serialize-string "size" x)
+ "}\n") ""))))
(decoders
(list-of-mpd-plugin '())
"List of MPD decoder plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "decoder" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "decoder" x))))
(resampler
maybe-mpd-plugin
@@ -464,8 +464,8 @@ (define-configuration mpd-configuration
(filters
(list-of-mpd-plugin '())
"List of MPD filter plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "filter" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "filter" x))))
(outputs
(list-of-mpd-plugin-or-output (list (mpd-output)))
@@ -475,8 +475,8 @@ (define-configuration mpd-configuration
(playlist-plugins
(list-of-mpd-plugin '())
"List of MPD playlist plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x))))
(extra-options
(alist '())
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index d085b375a2..229220eeb1 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -213,9 +213,10 @@ (define-configuration fstrim-configuration
maybe-list-of-strings
"Extra options to append to @command{fstrim} (run @samp{man fstrim} for
more information)."
- (lambda (_ value)
- (if (maybe-value-set? value)
- value '())))
+ (serializer
+ (lambda (_ value)
+ (if (maybe-value-set? value)
+ value '()))))
(prefix fstrim-))
(define (serialize-fstrim-configuration config)
diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 8116072920..e750bb468b 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -200,7 +200,7 @@ (define-configuration fail2ban-jail-configuration
"Backend to use to detect changes in the @code{log-path}. The default is
'auto. To consult the defaults of the jail configuration, refer to the
@file{/etc/fail2ban/jail.conf} file of the @code{fail2ban} package."
- fail2ban-jail-configuration-serialize-backend)
+ (serializer fail2ban-jail-configuration-serialize-backend))
(max-retry
maybe-integer
"The number of failures before a host get banned
@@ -269,7 +269,7 @@ (define-configuration fail2ban-jail-configuration
maybe-symbol
"The encoding of the log files handled by the jail.
Possible values are: @code{'ascii}, @code{'utf-8} and @code{'auto}."
- fail2ban-jail-configuration-serialize-log-encoding)
+ (serializer fail2ban-jail-configuration-serialize-log-encoding))
(log-path
(list-of-strings '())
"The file names of the log files to be monitored.")
@@ -280,7 +280,7 @@ (define-configuration fail2ban-jail-configuration
(text-config '())
"Extra content for the jail configuration, provided as a list of file-like
objects."
- serialize-text-config)
+ (serializer serialize-text-config))
(prefix fail2ban-jail-configuration-))
(define list-of-fail2ban-jail-configurations?
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 64b7bb1543..20952acb79 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -82,6 +82,9 @@ (define (custom-number-serializer name value)
(format #f "~a = ~a;" name value))
(define-configuration serializable-configuration
+ (port (number 80) "The port number." (serializer custom-number-serializer)))
+
+(define-configuration serializable-configuration-deprecated
(port (number 80) "The port number." custom-number-serializer))
(test-assert "serialize-configuration"
@@ -89,8 +92,14 @@ (define-configuration serializable-configuration
(let ((config (serializable-configuration)))
(serialize-configuration config serializable-configuration-fields))))
+(test-assert "serialize-configuration [deprecated]"
+ (gexp?
+ (let ((config (serializable-configuration-deprecated)))
+ (serialize-configuration
+ config serializable-configuration-deprecated-fields))))
+
(define-configuration serializable-configuration
- (port (number 80) "The port number." custom-number-serializer)
+ (port (number 80) "The port number." (serializer custom-number-serializer))
(no-serialization))
(test-assert "serialize-configuration with no-serialization"
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 2/8] services: replace bare serializers with (serializer ...)
2023-03-23 15:02 ` [bug#62298] [PATCH v2 2/8] services: replace bare serializers with (serializer ...) Bruno Victal
@ 2023-03-24 14:28 ` Maxim Cournoyer
0 siblings, 0 replies; 47+ messages in thread
From: Maxim Cournoyer @ 2023-03-24 14:28 UTC (permalink / raw)
To: Bruno Victal; +Cc: 62298, ludo, liliana.prikler
Bruno Victal <mirai@makinata.eu> writes:
> * gnu/home/services/shells.scm (home-zsh-configuration)[environment-variables]: Use (serializer ...).
> (home-bash-configuration)[aliases, environment-variables]: Ditto.
> (home-fish-configuration)[abbreviations, aliases, environment-variables]: Ditto.
> * gnu/services/audio.scm (mpd-configuration)[music-dir, playlist-dir, endpoints]
> [address, inputs, archive-plugins, input-cache-size, decoders, filters, playlist-plugins]: Ditto.
> * gnu/services/linux.scm (fstrim-configuration)[extra-arguments]: Ditto.
> * gnu/services/security.scm (fail2ban-jail-configuration)[backend, log-encoding, extra-content]: Ditto.
> * tests/services/configuration.scm: Update tests. Add test for deprecated bare serializers.
The commit message should be line-wrapped around 80 chars and the title
should be puncutated like: "services: Replace [...] ."
The rest LGTM.
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 3/8] services: audio: remove redundant list-of-string? predicate.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
2023-03-23 15:02 ` [bug#62298] [PATCH v2 2/8] services: replace bare serializers with (serializer ...) Bruno Victal
@ 2023-03-23 15:02 ` Bruno Victal
2023-03-23 15:02 ` [bug#62298] [PATCH v2 4/8] services: mympd: Require 'syslog service when configured to log to syslog Bruno Victal
` (6 subsequent siblings)
8 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-23 15:02 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
Use list-of-strings? predicate defined in (gnu services configuration).
* gnu/services/audio.scm (list-of-string?): Remove predicate.
(mpd-serialize-list-of-string): Rename procedure to ...
(mpd-serialize-list-of-strings): ... this.
(mpd-configuration)[environment-variables]: Switch to list-of-strings.
[endpoints]: Switch to maybe-list-of-strings.
(mympd-ip-acl)[allow, deny]: Switch to list-of-strings.
(mympd-serialize-configuration): Rename serialize-list-of-string to serialize-list-of-strings.
* doc/guix.texi (Audio Services): Update it.
---
doc/guix.texi | 8 ++++----
gnu/services/audio.scm | 25 +++++++++++--------------
2 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 1609e508ef..2b62605b51 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33501,7 +33501,7 @@ Audio Services
This is a list of symbols naming Shepherd services that this service
will depend on.
-@item @code{environment-variables} (default: @code{()}) (type: list-of-string)
+@item @code{environment-variables} (default: @code{()}) (type: list-of-strings)
A list of strings specifying environment variables.
@item @code{log-file} (default: @code{"/var/log/mpd/log"}) (type: maybe-string)
@@ -33532,7 +33532,7 @@ Audio Services
@item @code{default-port} (default: @code{6600}) (type: maybe-integer)
The default port to run mpd on.
-@item @code{endpoints} (type: maybe-list-of-string)
+@item @code{endpoints} (type: maybe-list-of-strings)
The addresses that mpd will bind to. A port different from @var{default-port}
may be specified, e.g. @code{localhost:6602} and IPv6 addresses must be
enclosed in square brackets when a different port is used.
@@ -33808,10 +33808,10 @@ Audio Services
Available @code{mympd-ip-acl} fields are:
@table @asis
-@item @code{allow} (default: @code{()}) (type: list-of-string)
+@item @code{allow} (default: @code{()}) (type: list-of-strings)
Allowed IP addresses.
-@item @code{deny} (default: @code{()}) (type: list-of-string)
+@item @code{deny} (default: @code{()}) (type: list-of-strings)
Disallowed IP addresses.
@end table
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 5f341fac0f..e9ecccd614 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -2,7 +2,7 @@
;;; Copyright © 2017 Peter Mikkelsen <petermikkelsen10@gmail.com>
;;; Copyright © 2019 Ricardo Wurmus <rekado@elephly.net>
;;; Copyright © 2020 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2022 Bruno Victal <mirai@makinata.eu>
+;;; Copyright © 2022–2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -137,9 +137,6 @@ (define (uglify-field-name field-name)
str)
#\-) "_")))
-(define list-of-string?
- (list-of string?))
-
(define list-of-symbol?
(list-of symbol?))
@@ -159,11 +156,11 @@ (define (mpd-serialize-alist field-name value)
(define mpd-serialize-string mpd-serialize-field)
(define mpd-serialize-boolean mpd-serialize-field)
-(define (mpd-serialize-list-of-string field-name value)
+(define (mpd-serialize-list-of-strings field-name value)
#~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
(define-maybe string (prefix mpd-))
-(define-maybe list-of-string (prefix mpd-))
+(define-maybe list-of-strings (prefix mpd-))
(define-maybe boolean (prefix mpd-))
;;; TODO: Procedures for deprecated fields, to be removed.
@@ -349,7 +346,7 @@ (define-configuration mpd-configuration
empty-serializer)
(environment-variables
- (list-of-string '())
+ (list-of-strings '())
"A list of strings specifying environment variables."
empty-serializer)
@@ -400,7 +397,7 @@ (define-configuration mpd-configuration
"The default port to run mpd on.")
(endpoints
- maybe-list-of-string
+ maybe-list-of-strings
"The addresses that mpd will bind to. A port different from
@var{default-port} may be specified, e.g. @code{localhost:6602} and
IPv6 addresses must be enclosed in square brackets when a different
@@ -409,7 +406,7 @@ (define-configuration mpd-configuration
can be specified here."
(serializer (lambda (_ endpoints)
(if (maybe-value-set? endpoints)
- (mpd-serialize-list-of-string "bind_to_address" endpoints)
+ (mpd-serialize-list-of-strings "bind_to_address" endpoints)
""))))
(address ; TODO: deprecated, remove later
@@ -581,11 +578,11 @@ (define (string-or-symbol? x)
(define-configuration/no-serialization mympd-ip-acl
(allow
- (list-of-string '())
+ (list-of-strings '())
"Allowed IP addresses.")
(deny
- (list-of-string '())
+ (list-of-strings '())
"Disallowed IP addresses."))
(define-maybe/no-serialization integer)
@@ -707,12 +704,12 @@ (define (mympd-serialize-configuration config)
((? string? val) val)))
(define (ip-acl-serialize-configuration config)
- (define (serialize-list-of-string prefix lst)
+ (define (serialize-list-of-strings prefix lst)
(map (cut format #f "~a~a" prefix <>) lst))
(string-join
(append
- (serialize-list-of-string "+" (mympd-ip-acl-allow config))
- (serialize-list-of-string "-" (mympd-ip-acl-deny config))) ","))
+ (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/.
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 4/8] services: mympd: Require 'syslog service when configured to log to syslog.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
2023-03-23 15:02 ` [bug#62298] [PATCH v2 2/8] services: replace bare serializers with (serializer ...) Bruno Victal
2023-03-23 15:02 ` [bug#62298] [PATCH v2 3/8] services: audio: remove redundant list-of-string? predicate Bruno Victal
@ 2023-03-23 15:02 ` Bruno Victal
2023-03-24 14:32 ` Maxim Cournoyer
2023-03-23 15:02 ` [bug#62298] [PATCH v2 5/8] services: mpd: Fix unintentional API breakage for mixer-type field Bruno Victal
` (5 subsequent siblings)
8 siblings, 1 reply; 47+ messages in thread
From: Bruno Victal @ 2023-03-23 15:02 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (mympd-shepherd-service): Depend on 'syslog when
configured to log to syslog.
---
gnu/services/audio.scm | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index e9ecccd614..e5b065a479 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -749,7 +749,9 @@ (define (mympd-shepherd-service config)
(let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level)))
(shepherd-service
(documentation "Run the myMPD daemon.")
- (requirement `(loopback user-processes ,@shepherd-requirement))
+ (requirement `(loopback user-processes
+ ,@(if (eqv? log-to 'syslog) '(syslog) '())
+ ,@shepherd-requirement))
(provision '(mympd))
(start #~(begin
(let* ((pw (getpwnam #$user))
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 4/8] services: mympd: Require 'syslog service when configured to log to syslog.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 4/8] services: mympd: Require 'syslog service when configured to log to syslog Bruno Victal
@ 2023-03-24 14:32 ` Maxim Cournoyer
0 siblings, 0 replies; 47+ messages in thread
From: Maxim Cournoyer @ 2023-03-24 14:32 UTC (permalink / raw)
To: Bruno Victal; +Cc: 62298, ludo, liliana.prikler
Hi,
Bruno Victal <mirai@makinata.eu> writes:
> * gnu/services/audio.scm (mympd-shepherd-service): Depend on 'syslog when
> configured to log to syslog.
> ---
> gnu/services/audio.scm | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index e9ecccd614..e5b065a479 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -749,7 +749,9 @@ (define (mympd-shepherd-service config)
> (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level)))
> (shepherd-service
> (documentation "Run the myMPD daemon.")
> - (requirement `(loopback user-processes ,@shepherd-requirement))
> + (requirement `(loopback user-processes
> + ,@(if (eqv? log-to 'syslog) '(syslog) '())
eq? is sufficient to compare symbols. Otherwise, LGTM.
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 5/8] services: mpd: Fix unintentional API breakage for mixer-type field.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
` (2 preceding siblings ...)
2023-03-23 15:02 ` [bug#62298] [PATCH v2 4/8] services: mympd: Require 'syslog service when configured to log to syslog Bruno Victal
@ 2023-03-23 15:02 ` Bruno Victal
2023-03-23 15:02 ` [bug#62298] [PATCH v2 6/8] services: mpd: Set PulseAudio related variables as default value for environment-variables field Bruno Victal
` (4 subsequent siblings)
8 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-23 15:02 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (mpd-output)[mixer-type]: Use sanitizer to
accept both strings and symbols as values.
---
gnu/services/audio.scm | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index e5b065a479..56ea2f8638 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,6 +140,11 @@ (define (uglify-field-name field-name)
(define list-of-symbol?
(list-of symbol?))
+\f
+;;;
+;;; MPD
+;;;
+
(define (mpd-serialize-field field-name value)
(let ((field (if (string? field-name) field-name
(uglify-field-name field-name)))
@@ -294,7 +299,17 @@ (define-configuration mpd-output
for this audio output: the @code{hardware} mixer, the @code{software}
mixer, the @code{null} mixer (allows setting the volume, but with no
effect; this can be used as a trick to implement an external mixer
-External Mixer) or no mixer (@code{none}).")
+External Mixer) or no mixer (@code{none})."
+ (sanitizer
+ (lambda (x) ; TODO: deprecated, remove me later.
+ (cond
+ ((symbol? x)
+ (warning (G_ "symbol value for 'mixer-type' is deprecated, \
+use string instead~%"))
+ (symbol->string x))
+ ((string? x) x)
+ (else
+ (configuration-field-error #f 'mixer-type x))))))
(replay-gain-handler
maybe-string
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 6/8] services: mpd: Set PulseAudio related variables as default value for environment-variables field.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
` (3 preceding siblings ...)
2023-03-23 15:02 ` [bug#62298] [PATCH v2 5/8] services: mpd: Fix unintentional API breakage for mixer-type field Bruno Victal
@ 2023-03-23 15:02 ` Bruno Victal
2023-03-24 18:10 ` bug#62298: " Maxim Cournoyer
2023-03-23 15:02 ` [bug#62298] [PATCH v2 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
` (3 subsequent siblings)
8 siblings, 1 reply; 47+ messages in thread
From: Bruno Victal @ 2023-03-23 15:02 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
These variables are necessary for PulseAudio to work properly out-of-the-box
for 'non-interactive' users.
* doc/guix.texi (Audio Services): Update environment-variables field description for
mpd-configuration data type.
* gnu/services/audio.scm (mpd-configuration)[environment-variables]: Set
PULSE_CLIENTCONFIG and PULSE_CONFIG environment variables to the system-wide
PulseAudio configuration.
---
doc/guix.texi | 2 +-
gnu/services/audio.scm | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 2b62605b51..af9f7d78c0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33501,7 +33501,7 @@ Audio Services
This is a list of symbols naming Shepherd services that this service
will depend on.
-@item @code{environment-variables} (default: @code{()}) (type: list-of-strings)
+@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)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 56ea2f8638..198157a83b 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -361,7 +361,8 @@ (define-configuration mpd-configuration
empty-serializer)
(environment-variables
- (list-of-strings '())
+ (list-of-strings '("PULSE_CLIENTCONFIG=/etc/pulse/client.conf"
+ "PULSE_CONFIG=/etc/pulse/daemon.conf"))
"A list of strings specifying environment variables."
empty-serializer)
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* bug#62298: [PATCH v2 6/8] services: mpd: Set PulseAudio related variables as default value for environment-variables field.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 6/8] services: mpd: Set PulseAudio related variables as default value for environment-variables field Bruno Victal
@ 2023-03-24 18:10 ` Maxim Cournoyer
0 siblings, 0 replies; 47+ messages in thread
From: Maxim Cournoyer @ 2023-03-24 18:10 UTC (permalink / raw)
To: Bruno Victal; +Cc: 62298-done, ludo, liliana.prikler
Hi,
Bruno Victal <mirai@makinata.eu> writes:
> These variables are necessary for PulseAudio to work properly out-of-the-box
> for 'non-interactive' users.
> * doc/guix.texi (Audio Services): Update environment-variables field description for
> mpd-configuration data type.
> * gnu/services/audio.scm (mpd-configuration)[environment-variables]: Set
> PULSE_CLIENTCONFIG and PULSE_CONFIG environment variables to the system-wide
> PulseAudio configuration.
> ---
> doc/guix.texi | 2 +-
> gnu/services/audio.scm | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 2b62605b51..af9f7d78c0 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -33501,7 +33501,7 @@ Audio Services
> This is a list of symbols naming Shepherd services that this service
> will depend on.
>
> -@item @code{environment-variables} (default: @code{()}) (type: list-of-strings)
> +@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)
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index 56ea2f8638..198157a83b 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -361,7 +361,8 @@ (define-configuration mpd-configuration
> empty-serializer)
>
> (environment-variables
> - (list-of-strings '())
> + (list-of-strings '("PULSE_CLIENTCONFIG=/etc/pulse/client.conf"
> + "PULSE_CONFIG=/etc/pulse/daemon.conf"))
> "A list of strings specifying environment variables."
> empty-serializer)
Installed, thank you!
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
` (4 preceding siblings ...)
2023-03-23 15:02 ` [bug#62298] [PATCH v2 6/8] services: mpd: Set PulseAudio related variables as default value for environment-variables field Bruno Victal
@ 2023-03-23 15:02 ` Bruno Victal
2023-03-23 18:03 ` Liliana Marie Prikler
2023-03-24 15:31 ` Maxim Cournoyer
2023-03-23 15:02 ` [bug#62298] [PATCH v2 8/8] services: mympd: " Bruno Victal
` (2 subsequent siblings)
8 siblings, 2 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-23 15:02 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
Deprecate using strings for these fields and prefer user-account (resp. user-group)
instead to avoid duplication within account-service-type.
If a string value is encountered, it is ignored and a predefined variable
is used instead. This is essentially a rollback to how it used to be before
'5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1'.
Fixes #61570 <https://issues.guix.gnu.org/61570>.
* gnu/services/audio.scm (mpd-serialize-user-account, mpd-serialize-user-group)
(mpd-user-sanitizer, mpd-group-sanitizer): New procedure.
(%mpd-user, %mpd-group): New variable.
(mpd-configuration)[user, group]: Set value type to user-account (resp. user-group).
(mpd-shepherd-service): Adapt for user-account values in user field.
(mpd-accounts): Adapt for user-account (resp. user-group) in user (resp. group) field.
* doc/guix.texi (Audio Services): Update documentation.
---
doc/guix.texi | 4 +-
gnu/services/audio.scm | 89 ++++++++++++++++++++++++++++++++++--------
2 files changed, 74 insertions(+), 19 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index af9f7d78c0..520a65b0b1 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33491,10 +33491,10 @@ Audio Services
@item @code{package} (default: @code{mpd}) (type: file-like)
The MPD package.
-@item @code{user} (default: @code{"mpd"}) (type: string)
+@item @code{user} (type: maybe-user-account)
The user to run mpd as.
-@item @code{group} (default: @code{"mpd"}) (type: string)
+@item @code{group} (type: maybe-user-group)
The group to run mpd as.
@item @code{shepherd-requirement} (default: @code{()}) (type: list-of-symbol)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 198157a83b..c168d1481c 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,6 +140,14 @@ (define (uglify-field-name field-name)
(define list-of-symbol?
(list-of symbol?))
+;; helpers for deprecated field types, to be removed later
+(define %lazy-group (make-symbol "%lazy-group"))
+
+(define (inject-group-into-user user group)
+ (user-account
+ (inherit user)
+ (group (user-group-name group))))
+
\f
;;;
;;; MPD
@@ -164,9 +172,32 @@ (define mpd-serialize-boolean mpd-serialize-field)
(define (mpd-serialize-list-of-strings field-name value)
#~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
+(define (mpd-serialize-user-account field-name value)
+ (mpd-serialize-string field-name (user-account-name value)))
+
+(define (mpd-serialize-user-group field-name value)
+ (mpd-serialize-string field-name (user-group-name value)))
+
(define-maybe string (prefix mpd-))
(define-maybe list-of-strings (prefix mpd-))
(define-maybe boolean (prefix mpd-))
+(define-maybe user-account (prefix mpd-))
+(define-maybe user-group (prefix mpd-))
+
+(define %mpd-user
+ (user-account
+ (name "mpd")
+ (group "mpd")
+ (system? #t)
+ (comment "Music Player Daemon (MPD) user")
+ ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
+ (home-directory "/var/lib/mpd")
+ (shell (file-append shadow "/sbin/nologin"))))
+
+(define %mpd-group
+ (user-group
+ (name "mpd")
+ (system? #t)))
;;; TODO: Procedures for deprecated fields, to be removed.
@@ -197,6 +228,33 @@ (define (mpd-serialize-port field-name value)
(define-maybe port (prefix mpd-))
+;;; procedures for unsupported value types, to be removed.
+
+(define (mpd-user-sanitizer value)
+ (cond ((user-account? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'user' is deprecated, use \
+user-account instead~%"))
+ (user-account
+ (inherit %mpd-user)
+ (name value)
+ ;; XXX: this is to be lazily substituted in (mpd-accounts)
+ ;; with the value from 'group'.
+ (group %lazy-group)))
+ (else
+ (configuration-field-error #f 'user value))))
+
+(define (mpd-group-sanitizer value)
+ (cond ((user-group? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'group' is deprecated, use \
+user-group instead~%"))
+ (user-group
+ (inherit %mpd-group)
+ (name value)))
+ (else
+ (configuration-field-error #f 'group value))))
+
;;;
;; Generic MPD plugin record, lists only the most prevalent fields.
@@ -347,12 +405,14 @@ (define-configuration mpd-configuration
empty-serializer)
(user
- (string "mpd")
- "The user to run mpd as.")
+ (maybe-user-account %mpd-user)
+ "The user to run mpd as."
+ (sanitizer mpd-user-sanitizer))
(group
- (string "mpd")
- "The group to run mpd as.")
+ (maybe-user-group %mpd-group)
+ "The group to run mpd as."
+ (sanitizer mpd-group-sanitizer))
(shepherd-requirement
(list-of-symbol '())
@@ -516,7 +576,8 @@ (define (mpd-shepherd-service config)
log-file playlist-directory
db-file state-file sticker-file
environment-variables)
- (let* ((config-file (mpd-serialize-configuration config)))
+ (let ((config-file (mpd-serialize-configuration config))
+ (username (user-account-name user)))
(shepherd-service
(documentation "Run the MPD (Music Player Daemon)")
(requirement `(user-processes loopback ,@shepherd-requirement))
@@ -525,7 +586,7 @@ (define (mpd-shepherd-service config)
(and=> #$(maybe-value log-file)
(compose mkdir-p dirname))
- (let ((user (getpw #$user)))
+ (let ((user (getpw #$username)))
(for-each
(lambda (x)
(when (and x (not (file-exists? x)))
@@ -559,17 +620,11 @@ (define (mpd-shepherd-service config)
(define (mpd-accounts config)
(match-record config <mpd-configuration> (user group)
- (list (user-group
- (name group)
- (system? #t))
- (user-account
- (name user)
- (group group)
- (system? #t)
- (comment "Music Player Daemon (MPD) user")
- ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
- (home-directory "/var/lib/mpd")
- (shell (file-append shadow "/sbin/nologin"))))))
+ ;; TODO: deprecation code, to be removed
+ (let ((user (if (eq? (user-account-group user) %lazy-group)
+ (inject-group-into-user user group)
+ user)))
+ (list user group))))
(define mpd-service-type
(service-type
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
@ 2023-03-23 18:03 ` Liliana Marie Prikler
2023-03-24 15:31 ` Maxim Cournoyer
1 sibling, 0 replies; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-23 18:03 UTC (permalink / raw)
To: Bruno Victal, 62298; +Cc: ludo, maxim.cournoyer
Am Donnerstag, dem 23.03.2023 um 15:02 +0000 schrieb Bruno Victal:
> If a string value is encountered, it is ignored and a predefined
> variable is used instead. This is essentially a rollback to how it
> used to be before '5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1'.
As far as I can see, this is not actually what happens. Don't forget
to update your commit messages :)
> Fixes #61570 <https://issues.guix.gnu.org/61570>.
You only need one newline after this one imho.
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -140,6 +140,14 @@ (define (uglify-field-name field-name)
> (define list-of-symbol?
> (list-of symbol?))
>
> +;; helpers for deprecated field types, to be removed later
> +(define %lazy-group (make-symbol "%lazy-group"))
> +
> +(define (inject-group-into-user user group)
> + (user-account
> + (inherit user)
> + (group (user-group-name group))))
> +
> \f
> ;;;
> ;;; MPD
> @@ -559,17 +620,11 @@ (define (mpd-shepherd-service config)
>
> (define (mpd-accounts config)
> (match-record config <mpd-configuration> (user group)
> - (list (user-group
> - (name group)
> - (system? #t))
> - (user-account
> - (name user)
> - (group group)
> - (system? #t)
> - (comment "Music Player Daemon (MPD) user")
> - ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its
> data
> - (home-directory "/var/lib/mpd")
> - (shell (file-append shadow "/sbin/nologin"))))))
> + ;; TODO: deprecation code, to be removed
> + (let ((user (if (eq? (user-account-group user) %lazy-group)
> + (inject-group-into-user user group)
> + user)))
> + (list user group))))
A little over-engineered, but works for me :)
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
2023-03-23 18:03 ` Liliana Marie Prikler
@ 2023-03-24 15:31 ` Maxim Cournoyer
1 sibling, 0 replies; 47+ messages in thread
From: Maxim Cournoyer @ 2023-03-24 15:31 UTC (permalink / raw)
To: Bruno Victal; +Cc: 62298, ludo, liliana.prikler
Hello!
Bruno Victal <mirai@makinata.eu> writes:
> services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
Please keep your git summary lines under 80 chars.
> Deprecate using strings for these fields and prefer user-account (resp. user-group)
> instead to avoid duplication within account-service-type.
> If a string value is encountered, it is ignored and a predefined variable
> is used instead. This is essentially a rollback to how it used to be before
> '5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1'.
>
> Fixes #61570 <https://issues.guix.gnu.org/61570>.
>
> * gnu/services/audio.scm (mpd-serialize-user-account, mpd-serialize-user-group)
> (mpd-user-sanitizer, mpd-group-sanitizer): New procedure.
> (%mpd-user, %mpd-group): New variable.
> (mpd-configuration)[user, group]: Set value type to user-account (resp. user-group).
> (mpd-shepherd-service): Adapt for user-account values in user field.
> (mpd-accounts): Adapt for user-account (resp. user-group) in user (resp. group) field.
Watch the 80 characters mark :-). Probably a good idea to configure
your editor to automatically wrap lines at that mark.
>
> * doc/guix.texi (Audio Services): Update documentation.
> ---
> doc/guix.texi | 4 +-
> gnu/services/audio.scm | 89 ++++++++++++++++++++++++++++++++++--------
> 2 files changed, 74 insertions(+), 19 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index af9f7d78c0..520a65b0b1 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -33491,10 +33491,10 @@ Audio Services
> @item @code{package} (default: @code{mpd}) (type: file-like)
> The MPD package.
>
> -@item @code{user} (default: @code{"mpd"}) (type: string)
> +@item @code{user} (type: maybe-user-account)
> The user to run mpd as.
>
> -@item @code{group} (default: @code{"mpd"}) (type: string)
> +@item @code{group} (type: maybe-user-group)
> The group to run mpd as.
>
> @item @code{shepherd-requirement} (default: @code{()}) (type: list-of-symbol)
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index 198157a83b..c168d1481c 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -140,6 +140,14 @@ (define (uglify-field-name field-name)
> (define list-of-symbol?
> (list-of symbol?))
>
> +;; helpers for deprecated field types, to be removed later
> +(define %lazy-group (make-symbol "%lazy-group"))
> +
> +(define (inject-group-into-user user group)
> + (user-account
> + (inherit user)
> + (group (user-group-name group))))
nitpick: I'd prefer the plainer language 'set-user-group'.
> \f
> ;;;
> ;;; MPD
> @@ -164,9 +172,32 @@ (define mpd-serialize-boolean mpd-serialize-field)
> (define (mpd-serialize-list-of-strings field-name value)
> #~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
>
> +(define (mpd-serialize-user-account field-name value)
> + (mpd-serialize-string field-name (user-account-name value)))
> +
> +(define (mpd-serialize-user-group field-name value)
> + (mpd-serialize-string field-name (user-group-name value)))
> +
> (define-maybe string (prefix mpd-))
> (define-maybe list-of-strings (prefix mpd-))
> (define-maybe boolean (prefix mpd-))
> +(define-maybe user-account (prefix mpd-))
> +(define-maybe user-group (prefix mpd-))
> +
> +(define %mpd-user
> + (user-account
> + (name "mpd")
> + (group "mpd")
> + (system? #t)
> + (comment "Music Player Daemon (MPD) user")
> + ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
> + (home-directory "/var/lib/mpd")
> + (shell (file-append shadow "/sbin/nologin"))))
> +
> +(define %mpd-group
> + (user-group
> + (name "mpd")
> + (system? #t)))
>
> ;;; TODO: Procedures for deprecated fields, to be removed.
>
> @@ -197,6 +228,33 @@ (define (mpd-serialize-port field-name value)
>
> (define-maybe port (prefix mpd-))
>
> +;;; procedures for unsupported value types, to be removed.
Please make this a complete sentence, and clarify this is related to
a deprecated usage?
> +(define (mpd-user-sanitizer value)
> + (cond ((user-account? value) value)
> + ((string? value)
> + (warning (G_ "string value for 'user' is deprecated, use \
> +user-account instead~%"))
> + (user-account
> + (inherit %mpd-user)
> + (name value)
> + ;; XXX: this is to be lazily substituted in (mpd-accounts)
> + ;; with the value from 'group'.
Nitpick: XXX: This (with capitalized first letter), and no hanging
indent for continued line.
> + (group %lazy-group)))
> + (else
> + (configuration-field-error #f 'user value))))
> +
> +(define (mpd-group-sanitizer value)
> + (cond ((user-group? value) value)
> + ((string? value)
> + (warning (G_ "string value for 'group' is deprecated, use \
> +user-group instead~%"))
> + (user-group
> + (inherit %mpd-group)
> + (name value)))
> + (else
> + (configuration-field-error #f 'group value))))
> +
> ;;;
>
> ;; Generic MPD plugin record, lists only the most prevalent fields.
> @@ -347,12 +405,14 @@ (define-configuration mpd-configuration
> empty-serializer)
>
> (user
> - (string "mpd")
> - "The user to run mpd as.")
> + (maybe-user-account %mpd-user)
> + "The user to run mpd as."
> + (sanitizer mpd-user-sanitizer))
>
> (group
> - (string "mpd")
> - "The group to run mpd as.")
> + (maybe-user-group %mpd-group)
> + "The group to run mpd as."
> + (sanitizer mpd-group-sanitizer))
>
> (shepherd-requirement
> (list-of-symbol '())
> @@ -516,7 +576,8 @@ (define (mpd-shepherd-service config)
> log-file playlist-directory
> db-file state-file sticker-file
> environment-variables)
> - (let* ((config-file (mpd-serialize-configuration config)))
> + (let ((config-file (mpd-serialize-configuration config))
> + (username (user-account-name user)))
> (shepherd-service
> (documentation "Run the MPD (Music Player Daemon)")
> (requirement `(user-processes loopback ,@shepherd-requirement))
> @@ -525,7 +586,7 @@ (define (mpd-shepherd-service config)
> (and=> #$(maybe-value log-file)
> (compose mkdir-p dirname))
>
> - (let ((user (getpw #$user)))
> + (let ((user (getpw #$username)))
> (for-each
> (lambda (x)
> (when (and x (not (file-exists? x)))
> @@ -559,17 +620,11 @@ (define (mpd-shepherd-service config)
>
> (define (mpd-accounts config)
> (match-record config <mpd-configuration> (user group)
> - (list (user-group
> - (name group)
> - (system? #t))
> - (user-account
> - (name user)
> - (group group)
> - (system? #t)
> - (comment "Music Player Daemon (MPD) user")
> - ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
> - (home-directory "/var/lib/mpd")
> - (shell (file-append shadow "/sbin/nologin"))))))
> + ;; TODO: deprecation code, to be removed
nitpick: Please use a complete sentence for this stand-alone comment.
> + (let ((user (if (eq? (user-account-group user) %lazy-group)
> + (inject-group-into-user user group)
> + user)))
> + (list user group))))
>
> (define mpd-service-type
> (service-type
The rest LGTM, with consideration to Liliana's remarks.
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
` (5 preceding siblings ...)
2023-03-23 15:02 ` [bug#62298] [PATCH v2 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
@ 2023-03-23 15:02 ` Bruno Victal
2023-03-23 19:19 ` Liliana Marie Prikler
2023-03-24 16:03 ` Maxim Cournoyer
2023-03-23 19:47 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Liliana Marie Prikler
2023-03-24 14:25 ` Maxim Cournoyer
8 siblings, 2 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-23 15:02 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (%mympd-user, %mympd-group): New variable.
(mympd-user-sanitizer, mympd-group-sanitizer): New procedure.
(mympd-configuration)[user, group]: Set value type to user-account (resp. user-group).
(mympd-serialize-configuration): Adapt for user-account values in user field.
(mympd-accounts): Adapt for user-account (resp. user-group) in user (resp. group) field.
---
doc/guix.texi | 4 +--
gnu/services/audio.scm | 74 ++++++++++++++++++++++++++++++++++--------
2 files changed, 63 insertions(+), 15 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 520a65b0b1..ee1e66b3ff 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33732,10 +33732,10 @@ Audio Services
This is a list of symbols naming Shepherd services that this service
will depend on.
-@item @code{user} (default: @code{"mympd"}) (type: string)
+@item @code{user} (type: maybe-user-account)
Owner of the @command{mympd} process.
-@item @code{group} (default: @code{"nogroup"}) (type: string)
+@item @code{group} (type: maybe-user-group)
Owner group of the @command{mympd} process.
@item @code{work-directory} (default: @code{"/var/lib/mympd"}) (type: string)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index c168d1481c..f7f430039e 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -659,6 +659,54 @@ (define-configuration/no-serialization mympd-ip-acl
(define-maybe/no-serialization integer)
(define-maybe/no-serialization mympd-ip-acl)
+;; XXX: These will shadow the previous definition used by mpd
+;; and cause warnings to be shown. Maybe split the file
+;; into audio/mpd.scm and audio/mympd.scm ?
+#;(define-maybe/no-serialization user-account)
+#;(define-maybe/no-serialization user-group)
+
+(define %mympd-user
+ (user-account
+ (name "mympd")
+ (group "mympd")
+ (system? #t)
+ (comment "myMPD user")
+ (home-directory "/var/empty")
+ (shell (file-append shadow "/sbin/nologin"))))
+
+(define %mympd-group
+ (user-group
+ (name "mympd")
+ (system? #t)))
+
+;;; TODO: procedures for unsupported value types, to be removed.
+(define (mympd-user-sanitizer value)
+ (cond ((user-account? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'user' is not supported, use \
+user-account instead~%"))
+ (user-account
+ (inherit %mympd-user)
+ (name value)
+ ;; XXX: this is to be lazily substituted in (…-accounts)
+ ;; with the value from 'group'.
+ (group %lazy-group)))
+ (else
+ (configuration-field-error #f 'user value))))
+
+(define (mympd-group-sanitizer value)
+ (cond ((user-group? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'group' is not supported, use \
+user-group instead~%"))
+ (user-group
+ (inherit %mympd-group)
+ (name value)))
+ (else
+ (configuration-field-error #f 'group 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
@@ -676,13 +724,15 @@ (define-configuration/no-serialization mympd-configuration
empty-serializer)
(user
- (string "mympd")
+ (maybe-user-account %mympd-user)
"Owner of the @command{mympd} process."
+ (sanitizer mympd-user-sanitizer)
empty-serializer)
(group
- (string "nogroup")
+ (maybe-user-group %mympd-group)
"Owner group of the @command{mympd} process."
+ (sanitizer mympd-group-sanitizer)
empty-serializer)
(work-directory
@@ -817,7 +867,8 @@ (define (mympd-shepherd-service config)
(match-record config <mympd-configuration> (package shepherd-requirement
user work-directory
cache-directory log-level log-to)
- (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level)))
+ (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level))
+ (username (user-account-name user)))
(shepherd-service
(documentation "Run the myMPD daemon.")
(requirement `(loopback user-processes
@@ -825,7 +876,7 @@ (define (mympd-shepherd-service config)
,@shepherd-requirement))
(provision '(mympd))
(start #~(begin
- (let* ((pw (getpwnam #$user))
+ (let* ((pw (getpwnam #$username))
(uid (passwd:uid pw))
(gid (passwd:gid pw)))
(for-each (lambda (dir)
@@ -835,7 +886,7 @@ (define (mympd-shepherd-service config)
(make-forkexec-constructor
`(#$(file-append package "/bin/mympd")
- "--user" #$user
+ "--user" #$username
#$@(if (eqv? log-to 'syslog) '("--syslog") '())
"--workdir" #$work-directory
"--cachedir" #$cache-directory)
@@ -845,14 +896,11 @@ (define (mympd-shepherd-service config)
(define (mympd-accounts config)
(match-record config <mympd-configuration> (user group)
- (list (user-group (name group)
- (system? #t))
- (user-account (name user)
- (group group)
- (system? #t)
- (comment "myMPD user")
- (home-directory "/var/empty")
- (shell (file-append shadow "/sbin/nologin"))))))
+ ;; TODO: deprecation code, to be removed
+ (let ((user (if (eq? (user-account-group user) %lazy-group)
+ (inject-group-into-user user group)
+ user)))
+ (list user group))))
(define (mympd-log-rotation config)
(match-record config <mympd-configuration> (log-to)
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 8/8] services: mympd: " Bruno Victal
@ 2023-03-23 19:19 ` Liliana Marie Prikler
2023-03-25 0:39 ` Bruno Victal
2023-03-24 16:03 ` Maxim Cournoyer
1 sibling, 1 reply; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-23 19:19 UTC (permalink / raw)
To: Bruno Victal, 62298; +Cc: ludo, maxim.cournoyer
Am Donnerstag, dem 23.03.2023 um 15:02 +0000 schrieb Bruno Victal:
> +(define %mympd-user
> + (user-account
> + (name "mympd")
> + (group "mympd")
> + (system? #t)
> + (comment "myMPD user")
> + (home-directory "/var/empty")
> + (shell (file-append shadow "/sbin/nologin"))))
> +
> +(define %mympd-group
> + (user-group
> + (name "mympd")
> + (system? #t)))
> +
> +;;; TODO: procedures for unsupported value types, to be removed.
> +(define (mympd-user-sanitizer value)
> + (cond ((user-account? value) value)
> + ((string? value)
> + (warning (G_ "string value for 'user' is not supported, use
> \
> +user-account instead~%"))
> + (user-account
> + (inherit %mympd-user)
> + (name value)
> + ;; XXX: this is to be lazily substituted in (…-accounts)
> + ;; with the value from 'group'.
> + (group %lazy-group)))
> + (else
> + (configuration-field-error #f 'user value))))
I think an in-place creation of the user and group might make more
sense than defining a dummy value for inheritance purposes. Same
probably also applies to the MPD service patch.
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-23 19:19 ` Liliana Marie Prikler
@ 2023-03-25 0:39 ` Bruno Victal
0 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-25 0:39 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: ludo, 62298, maxim.cournoyer
On 2023-03-23 19:19, Liliana Marie Prikler wrote:
> Am Donnerstag, dem 23.03.2023 um 15:02 +0000 schrieb Bruno Victal:
>> +(define %mympd-user
>> + (user-account
>> + (name "mympd")
>> + (group "mympd")
>> + (system? #t)
>> + (comment "myMPD user")
>> + (home-directory "/var/empty")
>> + (shell (file-append shadow "/sbin/nologin"))))
>> +
>> +(define %mympd-group
>> + (user-group
>> + (name "mympd")
>> + (system? #t)))
>> +
>> +;;; TODO: procedures for unsupported value types, to be removed.
>> +(define (mympd-user-sanitizer value)
>> + (cond ((user-account? value) value)
>> + ((string? value)
>> + (warning (G_ "string value for 'user' is not supported, use
>> \
>> +user-account instead~%"))
>> + (user-account
>> + (inherit %mympd-user)
>> + (name value)
>> + ;; XXX: this is to be lazily substituted in (…-accounts)
>> + ;; with the value from 'group'.
>> + (group %lazy-group)))
>> + (else
>> + (configuration-field-error #f 'user value))))
> I think an in-place creation of the user and group might make more
> sense than defining a dummy value for inheritance purposes. Same
> probably also applies to the MPD service patch.
Though already replied to in private via IRC, to leave this clarified for other reviewers,
these are not dummy values. They're the default values that the service will use
if none are specified. The inheritance happened to be a bonus here.
Cheers,
Bruno
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 8/8] services: mympd: " Bruno Victal
2023-03-23 19:19 ` Liliana Marie Prikler
@ 2023-03-24 16:03 ` Maxim Cournoyer
2023-03-25 0:33 ` Bruno Victal
1 sibling, 1 reply; 47+ messages in thread
From: Maxim Cournoyer @ 2023-03-24 16:03 UTC (permalink / raw)
To: Bruno Victal; +Cc: 62298, ludo, liliana.prikler
Hello,
Bruno Victal <mirai@makinata.eu> writes:
> services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
>
> * gnu/services/audio.scm (%mympd-user, %mympd-group): New variable.
> (mympd-user-sanitizer, mympd-group-sanitizer): New procedure.
> (mympd-configuration)[user, group]: Set value type to user-account (resp. user-group).
> (mympd-serialize-configuration): Adapt for user-account values in user field.
> (mympd-accounts): Adapt for user-account (resp. user-group) in user (resp. group) field.
Please configure your editor for the 80 characters mark.
> ---
> doc/guix.texi | 4 +--
> gnu/services/audio.scm | 74 ++++++++++++++++++++++++++++++++++--------
> 2 files changed, 63 insertions(+), 15 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 520a65b0b1..ee1e66b3ff 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -33732,10 +33732,10 @@ Audio Services
> This is a list of symbols naming Shepherd services that this service
> will depend on.
>
> -@item @code{user} (default: @code{"mympd"}) (type: string)
> +@item @code{user} (type: maybe-user-account)
> Owner of the @command{mympd} process.
>
> -@item @code{group} (default: @code{"nogroup"}) (type: string)
> +@item @code{group} (type: maybe-user-group)
> Owner group of the @command{mympd} process.
>
> @item @code{work-directory} (default: @code{"/var/lib/mympd"}) (type: string)
> diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
> index c168d1481c..f7f430039e 100644
> --- a/gnu/services/audio.scm
> +++ b/gnu/services/audio.scm
> @@ -659,6 +659,54 @@ (define-configuration/no-serialization mympd-ip-acl
> (define-maybe/no-serialization integer)
> (define-maybe/no-serialization mympd-ip-acl)
>
> +;; XXX: These will shadow the previous definition used by mpd
> +;; and cause warnings to be shown. Maybe split the file
> +;; into audio/mpd.scm and audio/mympd.scm ?
> +#;(define-maybe/no-serialization user-account)
> +#;(define-maybe/no-serialization user-group)
I'd rather keeping them together if possible; could the prefix trick be
used with them? No need for a hanging indent for continued text, here
and for the other occurrences.
The expressions commented; should they be? On another note, '#;'
appears undocumented, I'd avoid it until it is (and it's not necessary
here).
> +(define %mympd-user
> + (user-account
> + (name "mympd")
> + (group "mympd")
> + (system? #t)
> + (comment "myMPD user")
> + (home-directory "/var/empty")
> + (shell (file-append shadow "/sbin/nologin"))))
> +
> +(define %mympd-group
> + (user-group
> + (name "mympd")
> + (system? #t)))
> +
> +;;; TODO: procedures for unsupported value types, to be removed.
^ Procedures
> +(define (mympd-user-sanitizer value)
> + (cond ((user-account? value) value)
> + ((string? value)
> + (warning (G_ "string value for 'user' is not supported, use \
> +user-account instead~%"))
> + (user-account
> + (inherit %mympd-user)
> + (name value)
> + ;; XXX: this is to be lazily substituted in (…-accounts)
> + ;; with the value from 'group'.
Extraneous hanging indent :-).
> + (group %lazy-group)))
> + (else
> + (configuration-field-error #f 'user value))))
> +
> +(define (mympd-group-sanitizer value)
> + (cond ((user-group? value) value)
> + ((string? value)
> + (warning (G_ "string value for 'group' is not supported, use \
> +user-group instead~%"))
> + (user-group
> + (inherit %mympd-group)
> + (name value)))
> + (else
> + (configuration-field-error #f 'group value))))
> +;;;
Was this ';;;' added by mistake?
> ;; 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
> @@ -676,13 +724,15 @@ (define-configuration/no-serialization mympd-configuration
> empty-serializer)
>
> (user
> - (string "mympd")
> + (maybe-user-account %mympd-user)
> "Owner of the @command{mympd} process."
> + (sanitizer mympd-user-sanitizer)
> empty-serializer)
>
> (group
> - (string "nogroup")
> + (maybe-user-group %mympd-group)
> "Owner group of the @command{mympd} process."
> + (sanitizer mympd-group-sanitizer)
> empty-serializer)
>
> (work-directory
> @@ -817,7 +867,8 @@ (define (mympd-shepherd-service config)
> (match-record config <mympd-configuration> (package shepherd-requirement
> user work-directory
> cache-directory log-level log-to)
> - (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level)))
> + (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level))
> + (username (user-account-name user)))
> (shepherd-service
> (documentation "Run the myMPD daemon.")
> (requirement `(loopback user-processes
> @@ -825,7 +876,7 @@ (define (mympd-shepherd-service config)
> ,@shepherd-requirement))
> (provision '(mympd))
> (start #~(begin
> - (let* ((pw (getpwnam #$user))
> + (let* ((pw (getpwnam #$username))
> (uid (passwd:uid pw))
> (gid (passwd:gid pw)))
> (for-each (lambda (dir)
> @@ -835,7 +886,7 @@ (define (mympd-shepherd-service config)
>
> (make-forkexec-constructor
> `(#$(file-append package "/bin/mympd")
> - "--user" #$user
> + "--user" #$username
> #$@(if (eqv? log-to 'syslog) '("--syslog") '())
> "--workdir" #$work-directory
> "--cachedir" #$cache-directory)
> @@ -845,14 +896,11 @@ (define (mympd-shepherd-service config)
>
> (define (mympd-accounts config)
> (match-record config <mympd-configuration> (user group)
> - (list (user-group (name group)
> - (system? #t))
> - (user-account (name user)
> - (group group)
> - (system? #t)
> - (comment "myMPD user")
> - (home-directory "/var/empty")
> - (shell (file-append shadow "/sbin/nologin"))))))
> + ;; TODO: deprecation code, to be removed
Please use a full sentence.
> + (let ((user (if (eq? (user-account-group user) %lazy-group)
> + (inject-group-into-user user group)
> + user)))
> + (list user group))))
>
> (define (mympd-log-rotation config)
> (match-record config <mympd-configuration> (log-to)
LGTM, with the comments from Liliana taken into account.
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-24 16:03 ` Maxim Cournoyer
@ 2023-03-25 0:33 ` Bruno Victal
2023-03-25 5:21 ` Liliana Marie Prikler
0 siblings, 1 reply; 47+ messages in thread
From: Bruno Victal @ 2023-03-25 0:33 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 62298, ludo, liliana.prikler
On 2023-03-24 16:03, Maxim Cournoyer wrote:> Bruno Victal <mirai@makinata.eu> writes:
>>
>> +;; XXX: These will shadow the previous definition used by mpd
>> +;; and cause warnings to be shown. Maybe split the file
>> +;; into audio/mpd.scm and audio/mympd.scm ?
>> +#;(define-maybe/no-serialization user-account)
>> +#;(define-maybe/no-serialization user-group)
>
> I'd rather keeping them together if possible; could the prefix trick be
> used with them? No need for a hanging indent for continued text, here
> and for the other occurrences.
The prefix trick is unlikely to help since the previous ones already use it.
--8<---------------cut here---------------start------------->8---
(define-maybe user-account (prefix mpd-))
(define-maybe user-group (prefix mpd-))
--8<---------------cut here---------------end--------------->8---
I'm using define-maybe as a “cheat” here for prettier documentation generation.
Without define-maybe, the documentation is rendered as:
--8<---------------cut here---------------start------------->8---
@item @code{user} (type: user-account)
The user to run mpd as.
--8<---------------cut here---------------end--------------->8---
… which is the documentation format for a field that requires an explicit value,
even though we are setting a default one using %mpd-account.
On the other hand, using define-maybe yields:
--8<---------------cut here---------------start------------->8---
@item @code{user} (type: maybe-user-account)
The user to run mpd as.
--8<---------------cut here---------------end--------------->8---
… which is the format for fields that do not require any manual intervention.
> The expressions commented; should they be? On another note, '#;'
> appears undocumented, I'd avoid it until it is (and it's not necessary
> here).
They're commented because the “right way of things” would have that the first
maybe-user-account is mpd-configuration specific due to (prefix mpd-) and that
we should have another maybe-user-account that is unrelated.
Here, we're actually reusing the one from mpd and it happens to be “ok” since
we do no serialization due to define-configuration/no-serialization.
Regarding #;, it is documented, though perhaps in a indirect manner. It's SRFI-62,
which is listed as supported in Guile manual.
>> + (inherit %mympd-group)
>> + (name value)))
>> + (else
>> + (configuration-field-error #f 'group value))))
>> +;;;
>
> Was this ';;;' added by mistake?
It serves as an aid to demarcate the deprecation helpers to be erased when the
time arrives. The helpers for MPD are also similarly demarcated.
^ permalink raw reply [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 8/8] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-25 0:33 ` Bruno Victal
@ 2023-03-25 5:21 ` Liliana Marie Prikler
0 siblings, 0 replies; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-25 5:21 UTC (permalink / raw)
To: Bruno Victal, Maxim Cournoyer; +Cc: 62298, ludo
Am Samstag, dem 25.03.2023 um 00:33 +0000 schrieb Bruno Victal:
> On 2023-03-24 16:03, Maxim Cournoyer wrote:> Bruno Victal
> <mirai@makinata.eu> writes:
> > >
> > > +;; XXX: These will shadow the previous definition used by mpd
> > > +;; and cause warnings to be shown. Maybe split the file
> > > +;; into audio/mpd.scm and audio/mympd.scm ?
> > > +#;(define-maybe/no-serialization user-account)
> > > +#;(define-maybe/no-serialization user-group)
> >
> > I'd rather keeping them together if possible; could the prefix
> > trick be
> > used with them? No need for a hanging indent for continued text,
> > here
> > and for the other occurrences.
>
> The prefix trick is unlikely to help since the previous ones already
> use it.
> --8<---------------cut here---------------start------------->8---
> (define-maybe user-account (prefix mpd-))
> (define-maybe user-group (prefix mpd-))
> --8<---------------cut here---------------end--------------->8---
>
> I'm using define-maybe as a “cheat” here for prettier documentation
> generation.
> Without define-maybe, the documentation is rendered as:
>
> --8<---------------cut here---------------start------------->8---
> @item @code{user} (type: user-account)
> The user to run mpd as.
> --8<---------------cut here---------------end--------------->8---
>
> … which is the documentation format for a field that requires an
> explicit value, even though we are setting a default one using %mpd-
> account.
>
> On the other hand, using define-maybe yields:
>
> --8<---------------cut here---------------start------------->8---
> @item @code{user} (type: maybe-user-account)
> The user to run mpd as.
> --8<---------------cut here---------------end--------------->8---
>
> … which is the format for fields that do not require any manual
> intervention.
This is a slight abuse of define-maybe, though. define-maybe
communicates, that the field having no value, i.e. not even a default
value, is acceptable. Since we always need a user to run MPD with,
this makes no sense semantically.
> > The expressions commented; should they be? On another note, '#;'
> > appears undocumented, I'd avoid it until it is (and it's not
> > necessary
> > here).
>
> They're commented because the “right way of things” would have that
> the first maybe-user-account is mpd-configuration specific due to
> (prefix mpd-) and that we should have another maybe-user-account that
> is unrelated.
Since we require "explicit" users in both cases, I think we can work
around this by dropping the maybe, no?
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
` (6 preceding siblings ...)
2023-03-23 15:02 ` [bug#62298] [PATCH v2 8/8] services: mympd: " Bruno Victal
@ 2023-03-23 19:47 ` Liliana Marie Prikler
2023-03-24 14:25 ` Maxim Cournoyer
8 siblings, 0 replies; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-23 19:47 UTC (permalink / raw)
To: Bruno Victal, 62298; +Cc: ludo, maxim.cournoyer
Am Donnerstag, dem 23.03.2023 um 15:02 +0000 schrieb Bruno Victal:
> +@var{sanitizer} is the name of a procedure which takes one argument,
@var{sanitizer} is a procedure
> +a user-supplied value, and returns a ``sanitized'' value for the
> field.
> +If none is specified, the predicate @code{@var{type}?} is
> automatically
> +used to construct an internal sanitizer that asserts the type
> correctness
> +of the field value.
If no sanitizer is specified, a default sanitizer is used, which raises
an error if the value is not of @var{type}.
> @var{serializer} is the name of a procedure which takes two
> arguments,
@var{serializer} is a procedure, which takes two arguments, a name of a
field and the value of said field.
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support.
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
` (7 preceding siblings ...)
2023-03-23 19:47 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Liliana Marie Prikler
@ 2023-03-24 14:25 ` Maxim Cournoyer
2023-03-24 18:03 ` Liliana Marie Prikler
8 siblings, 1 reply; 47+ messages in thread
From: Maxim Cournoyer @ 2023-03-24 14:25 UTC (permalink / raw)
To: Bruno Victal; +Cc: 62298, ludo, liliana.prikler
Hello!
Bruno Victal <mirai@makinata.eu> writes:
> This changes the 'custom-serializer' field into a generic
> 'extra-args' field that can be extended to support new literals.
> With this mechanism, the literals 'sanitizer' allow for user-defined
> sanitizer procedures while the 'serializer' literal is used for
> custom serializer procedures.
> The 'empty-serializer' was also added as a 'literal' and can be used
> just like it was previously.
>
> With the repurposing of 'custom-serializer' into 'extra-args', to prevent
> intolerable confusion, the custom serializer procedures should be
> specified using the new 'literals' approach, with the previous “style”
> being considered deprecated.
>
> * gnu/services/configuration.scm (define-configuration-helper):
> Rename 'custom-serializer' to 'extra-args'.
> Add support for literals 'sanitizer', 'serializer' and 'empty-serializer'.
> Rename procedure 'field-sanitizer' to 'default-field-sanitizer' to avoid syntax clash.
> Only define default field sanitizers if user-defined ones are absent.
> (normalize-extra-args): New procedure.
> (<configuration-field>)[sanitizer]: New field.
>
> * doc/guix.texi (Complex Configurations): Document the newly added literals.
>
> * tests/services/configuration.scm: Add tests for the new literals.
Interesting work!
> ---
> doc/guix.texi | 30 ++++-
> gnu/services/configuration.scm | 91 +++++++++++----
> tests/services/configuration.scm | 185 ++++++++++++++++++++++++++++++-
> 3 files changed, 280 insertions(+), 26 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index dfdb26103a..1609e508ef 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -41216,7 +41216,7 @@ Complex Configurations
> (@var{field-name}
> (@var{type} @var{default-value})
> @var{documentation}
> - @var{serializer})
> + (serializer @var{serializer}))
>
> (@var{field-name}
> (@var{type})
> @@ -41225,7 +41225,18 @@ Complex Configurations
> (@var{field-name}
> (@var{type})
> @var{documentation}
> - @var{serializer})
> + (serializer @var{serializer}))
> +
> +(@var{field-name}
> + (@var{type})
> + @var{documentation}
> + (sanitizer @var{sanitizer})
> +
> +(@var{field-name}
> + (@var{type})
> + @var{documentation}
> + (sanitizer @var{sanitizer})
> + (serializer @var{serializer}))
> @end example
>
> @var{field-name} is an identifier that denotes the name of the field in
> @@ -41248,6 +41259,21 @@ Complex Configurations
> @var{documentation} is a string formatted with Texinfo syntax which
> should provide a description of what setting this field does.
>
> +@var{sanitizer} is the name of a procedure which takes one argument,
> +a user-supplied value, and returns a ``sanitized'' value for the field.
> +If none is specified, the predicate @code{@var{type}?} is automatically
> +used to construct an internal sanitizer that asserts the type correctness
> +of the field value.
> +
> +An example of a sanitizer for a field that accepts both strings and
> +symbols looks like this:
> +@lisp
> +(define (sanitize-foo value)
> + (cond ((string? value) value)
> + ((symbol? value) (symbol->string value))
> + (else (throw 'bad! value))))
> +@end lisp
I'd use the more conventional (error "bad value" value) in the example.
> @var{serializer} is the name of a procedure which takes two arguments,
> the first is the name of the field, and the second is the value
> corresponding to the field. The procedure should return a string or
> diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
> index 174c2f20d2..409d4cef00 100644
> --- a/gnu/services/configuration.scm
> +++ b/gnu/services/configuration.scm
> @@ -6,6 +6,7 @@
> ;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
> ;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
> ;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
> +;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -28,7 +29,8 @@ (define-module (gnu services configuration)
> #:use-module (guix gexp)
> #:use-module ((guix utils) #:select (source-properties->location))
> #:use-module ((guix diagnostics)
> - #:select (formatted-message location-file &error-location))
> + #:select (formatted-message location-file &error-location
> + warning))
> #:use-module ((guix modules) #:select (file-name->module-name))
> #:use-module (guix i18n)
> #:autoload (texinfo) (texi-fragment->stexi)
> @@ -37,6 +39,7 @@ (define-module (gnu services configuration)
> #:use-module (ice-9 format)
> #:use-module (ice-9 match)
> #:use-module (srfi srfi-1)
> + #:use-module (srfi srfi-26)
> #:use-module (srfi srfi-34)
> #:use-module (srfi srfi-35)
> #:export (configuration-field
> @@ -44,6 +47,7 @@ (define-module (gnu services configuration)
> configuration-field-type
> configuration-missing-field
> configuration-field-error
> + configuration-field-sanitizer
> configuration-field-serializer
> configuration-field-getter
> configuration-field-default-value-thunk
> @@ -116,6 +120,7 @@ (define-record-type* <configuration-field>
> (type configuration-field-type)
> (getter configuration-field-getter)
> (predicate configuration-field-predicate)
> + (sanitizer configuration-field-sanitizer)
> (serializer configuration-field-serializer)
> (default-value-thunk configuration-field-default-value-thunk)
> (documentation configuration-field-documentation))
> @@ -181,11 +186,45 @@ (define (normalize-field-type+def s)
> (values #'(field-type %unset-value)))))
>
> (define (define-configuration-helper serialize? serializer-prefix syn)
> +
> + (define (normalize-extra-args s)
A docstring would help here, explaining what this helper is for.
> + (let loop ((s s)
> + (sanitizer* %unset-value)
> + (serializer* %unset-value))
> + (syntax-case s (sanitizer serializer empty-serializer)
> + (((sanitizer proc) tail ...)
> + (if (maybe-value-set? sanitizer*)
> + (syntax-violation 'sanitizer "duplicate entry"
> + #'proc)
> + (loop #'(tail ...) #'proc serializer*)))
> + (((serializer proc) tail ...)
> + (if (maybe-value-set? serializer*)
> + (syntax-violation 'serializer "duplicate or conflicting entry"
> + #'proc)
> + (loop #'(tail ...) sanitizer* #'proc)))
> + ((empty-serializer tail ...)
> + (if (maybe-value-set? serializer*)
> + (syntax-violation 'empty-serializer
> + "duplicate or conflicting entry" #f)
> + (loop #'(tail ...) sanitizer* #'empty-serializer)))
> + (() ; stop condition
> + (values (list sanitizer* serializer*)))
> + ((proc) ; TODO: deprecated, to be removed
> + (every (cut eq? <> #f)
> + (map maybe-value-set?
> + (list sanitizer* serializer*)))
This 'every' call result is not acted upon. Was it supposed to raise a
syntax violation? If it's useful to keep it (and act on it), I'd use
something like:
--8<---------------cut here---------------start------------->8---
(unless (null? (filter-map maybe-value-set? (list sanitizer* serializer*)))
(syntax-violation ...))
--8<---------------cut here---------------end--------------->8---
> + (begin
I think the 'begin' is unnecessary.
> + (warning #f (G_ "specifying serializers after documentation is \
> +deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
I wonder, instead of #f, would it be possible to fill in the correct
source location? That'd be useful, and it's done elsewhere, though I'm
not too familiar with the mechanism (I think it relies on Guile's source
properties).
[...]
> diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
> index 4f8a74dc8a..64b7bb1543 100644
> --- a/tests/services/configuration.scm
> +++ b/tests/services/configuration.scm
[...]
> +(test-group "empty-serializer as literal/procedure tests"
> + ;; empty-serializer as literal
Nitpick; all stand-alone comments starting with ;; should be properly
punctuated, complete sentences.
The rest LGTM, but I'll leave it on the tracker for another week or so
to allow for others to comment since it's a close-to-core change.
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support.
2023-03-24 14:25 ` Maxim Cournoyer
@ 2023-03-24 18:03 ` Liliana Marie Prikler
2023-03-26 2:01 ` Maxim Cournoyer
0 siblings, 1 reply; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-03-24 18:03 UTC (permalink / raw)
To: Maxim Cournoyer, Bruno Victal; +Cc: 62298, ludo
Am Freitag, dem 24.03.2023 um 10:25 -0400 schrieb Maxim Cournoyer:
> > + ((proc) ; TODO: deprecated, to be removed
> > + (every (cut eq? <> #f)
> > + (map maybe-value-set?
> > + (list sanitizer* serializer*)))
>
> This 'every' call result is not acted upon. Was it supposed to raise
> a syntax violation? If it's useful to keep it (and act on it), I'd
> use something like:
If I read this correctly, this 'every' call is actually in a guard
position, that is the syntax-case will ignore proc unless
> --8<---------------cut here---------------start------------->8---
> (null? (filter-map maybe-value-set? (list sanitizer* serializer*)))
> --8<---------------cut here---------------end--------------->8---
I think your guard is the fancier one, though, and should be preferred.
>
Cheers
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support.
2023-03-24 18:03 ` Liliana Marie Prikler
@ 2023-03-26 2:01 ` Maxim Cournoyer
0 siblings, 0 replies; 47+ messages in thread
From: Maxim Cournoyer @ 2023-03-26 2:01 UTC (permalink / raw)
To: Liliana Marie Prikler; +Cc: 62298, Bruno Victal, ludo
Hi,
Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
> Am Freitag, dem 24.03.2023 um 10:25 -0400 schrieb Maxim Cournoyer:
>> > + ((proc) ; TODO: deprecated, to be removed
>> > + (every (cut eq? <> #f)
>> > + (map maybe-value-set?
>> > + (list sanitizer* serializer*)))
>>
>> This 'every' call result is not acted upon. Was it supposed to raise
>> a syntax violation? If it's useful to keep it (and act on it), I'd
>> use something like:
> If I read this correctly, this 'every' call is actually in a guard
> position, that is the syntax-case will ignore proc unless
Thanks for explaining -- Bruno had also hinted at this was a syntax-case
guard on IRC, which I had forgotten about :-).
--
Thanks,
Maxim
^ permalink raw reply [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v3 1/5] services: configuration: Add user-defined sanitizer support.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
` (8 preceding siblings ...)
2023-03-23 15:02 ` [bug#62298] [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support Bruno Victal
@ 2023-03-25 0:46 ` Bruno Victal
2023-03-25 0:46 ` [bug#62298] [PATCH v3 2/5] services: replace bare serializers with (serializer ...) Bruno Victal
` (3 more replies)
2023-03-26 18:41 ` [bug#62298] [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Bruno Victal
10 siblings, 4 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-25 0:46 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
This changes the 'custom-serializer' field into a generic
'extra-args' field that can be extended to support new literals.
With this mechanism, the literals 'sanitizer' allow for user-defined
sanitizer procedures while the 'serializer' literal is used for
custom serializer procedures.
The 'empty-serializer' was also added as a 'literal' and can be used
just like it was previously.
With the repurposing of 'custom-serializer' into 'extra-args', to prevent
intolerable confusion, the custom serializer procedures should be
specified using the new 'literals' approach, with the previous “style”
being considered deprecated.
* gnu/services/configuration.scm (define-configuration-helper):
Rename 'custom-serializer' to 'extra-args'. Add support for literals
'sanitizer', 'serializer' and 'empty-serializer'. Rename procedure
'field-sanitizer' to 'default-field-sanitizer' to avoid syntax clash.
Only define default field sanitizers if user-defined ones are absent.
(normalize-extra-args): New procedure.
(<configuration-field>)[sanitizer]: New field.
* doc/guix.texi (Complex Configurations): Document the newly added literals.
* tests/services/configuration.scm: Add tests for the new literals.
---
doc/guix.texi | 29 ++++-
gnu/services/configuration.scm | 90 +++++++++++----
tests/services/configuration.scm | 183 ++++++++++++++++++++++++++++++-
3 files changed, 276 insertions(+), 26 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 3e335306f1..8604b95f94 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -41216,7 +41216,7 @@ Complex Configurations
(@var{field-name}
(@var{type} @var{default-value})
@var{documentation}
- @var{serializer})
+ (serializer @var{serializer}))
(@var{field-name}
(@var{type})
@@ -41225,7 +41225,18 @@ Complex Configurations
(@var{field-name}
(@var{type})
@var{documentation}
- @var{serializer})
+ (serializer @var{serializer}))
+
+(@var{field-name}
+ (@var{type})
+ @var{documentation}
+ (sanitizer @var{sanitizer})
+
+(@var{field-name}
+ (@var{type})
+ @var{documentation}
+ (sanitizer @var{sanitizer})
+ (serializer @var{serializer}))
@end example
@var{field-name} is an identifier that denotes the name of the field in
@@ -41248,6 +41259,20 @@ Complex Configurations
@var{documentation} is a string formatted with Texinfo syntax which
should provide a description of what setting this field does.
+@var{sanitizer} is a procedure which takes one argument,
+a user-supplied value, and returns a ``sanitized'' value for the field.
+If no sanitizer is specified, a default sanitizer is used, which raises
+an error if the value is not of type @var{type}.
+
+An example of a sanitizer for a field that accepts both strings and
+symbols looks like this:
+@lisp
+(define (sanitize-foo value)
+ (cond ((string? value) value)
+ ((symbol? value) (symbol->string value))
+ (else (error "bad value"))))
+@end lisp
+
@var{serializer} is the name of a procedure which takes two arguments,
the first is the name of the field, and the second is the value
corresponding to the field. The procedure should return a string or
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 174c2f20d2..4f3c6e2331 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -6,6 +6,7 @@
;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -28,7 +29,8 @@ (define-module (gnu services configuration)
#:use-module (guix gexp)
#:use-module ((guix utils) #:select (source-properties->location))
#:use-module ((guix diagnostics)
- #:select (formatted-message location-file &error-location))
+ #:select (formatted-message location-file &error-location
+ warning))
#:use-module ((guix modules) #:select (file-name->module-name))
#:use-module (guix i18n)
#:autoload (texinfo) (texi-fragment->stexi)
@@ -37,6 +39,7 @@ (define-module (gnu services configuration)
#:use-module (ice-9 format)
#:use-module (ice-9 match)
#:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-35)
#:export (configuration-field
@@ -44,6 +47,7 @@ (define-module (gnu services configuration)
configuration-field-type
configuration-missing-field
configuration-field-error
+ configuration-field-sanitizer
configuration-field-serializer
configuration-field-getter
configuration-field-default-value-thunk
@@ -116,6 +120,7 @@ (define-record-type* <configuration-field>
(type configuration-field-type)
(getter configuration-field-getter)
(predicate configuration-field-predicate)
+ (sanitizer configuration-field-sanitizer)
(serializer configuration-field-serializer)
(default-value-thunk configuration-field-default-value-thunk)
(documentation configuration-field-documentation))
@@ -181,11 +186,44 @@ (define (normalize-field-type+def s)
(values #'(field-type %unset-value)))))
(define (define-configuration-helper serialize? serializer-prefix syn)
+
+ (define (normalize-extra-args s)
+ "Extract and normalize arguments following @var{doc}."
+ (let loop ((s s)
+ (sanitizer* %unset-value)
+ (serializer* %unset-value))
+ (syntax-case s (sanitizer serializer empty-serializer)
+ (((sanitizer proc) tail ...)
+ (if (maybe-value-set? sanitizer*)
+ (syntax-violation 'sanitizer "duplicate entry"
+ #'proc)
+ (loop #'(tail ...) #'proc serializer*)))
+ (((serializer proc) tail ...)
+ (if (maybe-value-set? serializer*)
+ (syntax-violation 'serializer "duplicate or conflicting entry"
+ #'proc)
+ (loop #'(tail ...) sanitizer* #'proc)))
+ ((empty-serializer tail ...)
+ (if (maybe-value-set? serializer*)
+ (syntax-violation 'empty-serializer
+ "duplicate or conflicting entry" #f)
+ (loop #'(tail ...) sanitizer* #'empty-serializer)))
+ (() ; stop condition
+ (values (list sanitizer* serializer*)))
+ ((proc) ; TODO: deprecated, to be removed
+ (null? (filter-map maybe-value-set? (list sanitizer* serializer*)))
+ (begin
+ (warning #f (G_ "specifying serializers after documentation is \
+deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
+ (values (list %unset-value #'proc)))))))
+
(syntax-case syn ()
- ((_ stem (field field-type+def doc custom-serializer ...) ...)
+ ((_ stem (field field-type+def doc extra-args ...) ...)
(with-syntax
((((field-type def) ...)
- (map normalize-field-type+def #'(field-type+def ...))))
+ (map normalize-field-type+def #'(field-type+def ...)))
+ (((sanitizer* serializer*) ...)
+ (map normalize-extra-args #'((extra-args ...) ...))))
(with-syntax
(((field-getter ...)
(map (lambda (field)
@@ -200,21 +238,18 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
((field-type default-value)
default-value))
#'((field-type def) ...)))
+ ((field-sanitizer ...)
+ (map maybe-value #'(sanitizer* ...)))
((field-serializer ...)
- (map (lambda (type custom-serializer)
+ (map (lambda (type proc)
(and serialize?
- (match custom-serializer
- ((serializer)
- serializer)
- (()
- (if serializer-prefix
- (id #'stem
- serializer-prefix
- #'serialize- type)
- (id #'stem #'serialize- type))))))
+ (or (maybe-value proc)
+ (if serializer-prefix
+ (id #'stem serializer-prefix #'serialize- type)
+ (id #'stem #'serialize- type)))))
#'(field-type ...)
- #'((custom-serializer ...) ...))))
- (define (field-sanitizer name pred)
+ #'(serializer* ...))))
+ (define (default-field-sanitizer name pred)
;; Define a macro for use as a record field sanitizer, where NAME
;; is the name of the field and PRED is the predicate that tells
;; whether a value is valid for this field.
@@ -235,21 +270,29 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
#`(begin
;; Define field validation macros.
- #,@(map field-sanitizer
- #'(field ...)
- #'(field-predicate ...))
+ #,@(filter-map (lambda (name pred sanitizer)
+ (if sanitizer
+ #f
+ (default-field-sanitizer name pred)))
+ #'(field ...)
+ #'(field-predicate ...)
+ #'(field-sanitizer ...))
(define-record-type* #,(id #'stem #'< #'stem #'>)
stem
#,(id #'stem #'make- #'stem)
#,(id #'stem #'stem #'?)
- #,@(map (lambda (name getter def)
- #`(#,name #,getter (default #,def)
+ #,@(map (lambda (name getter def sanitizer)
+ #`(#,name #,getter
+ (default #,def)
(sanitize
- #,(id #'stem #'validate- #'stem #'- name))))
+ #,(or sanitizer
+ (id #'stem
+ #'validate- #'stem #'- name)))))
#'(field ...)
#'(field-getter ...)
- #'(field-default ...))
+ #'(field-default ...)
+ #'(field-sanitizer ...))
(%location #,(id #'stem #'stem #'-source-location)
(default (and=> (current-source-location)
source-properties->location))
@@ -261,6 +304,9 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
(type 'field-type)
(getter field-getter)
(predicate field-predicate)
+ (sanitizer
+ (or field-sanitizer
+ (id #'stem #'validate- #'stem #'- #'field)))
(serializer field-serializer)
(default-value-thunk
(lambda ()
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 4f8a74dc8a..0392cce927 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -22,6 +23,7 @@ (define-module (tests services configuration)
#:use-module (gnu services configuration)
#:use-module (guix diagnostics)
#:use-module (guix gexp)
+ #:autoload (guix i18n) (G_)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-64))
@@ -46,14 +48,14 @@ (define-configuration port-configuration
(port-configuration-port (port-configuration)))
(test-equal "wrong type for a field"
- '("configuration.scm" 57 11) ;error location
+ '("configuration.scm" 59 11) ;error location
(guard (c ((configuration-error? c)
(let ((loc (error-location c)))
(list (basename (location-file loc))
(location-line loc)
(location-column loc)))))
(port-configuration
- ;; This is line 56; the test relies on line/column numbers!
+ ;; This is line 58; the test relies on line/column numbers!
(port "This is not a number!"))))
(define-configuration port-configuration-cs
@@ -109,6 +111,183 @@ (define-configuration configuration-with-prefix
(let ((config (configuration-with-prefix)))
(serialize-configuration config configuration-with-prefix-fields))))
+\f
+;;;
+;;; define-configuration macro, extra-args literals
+;;;
+
+(define (eval-gexp x)
+ "Get serialized config as string."
+ (eval (gexp->approximate-sexp x)
+ (current-module)))
+
+(define (port? value)
+ (or (string? value) (number? value)))
+
+(define (sanitize-port value)
+ (cond ((number? value) value)
+ ((string? value) (string->number value))
+ (else (raise (formatted-message (G_ "Bad value: ~a") value)))))
+
+(test-group "Basic sanitizer literal tests"
+ (define serialize-port serialize-number)
+
+ (define-configuration config-with-sanitizer
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)))
+
+ (test-equal "default value, sanitizer"
+ 80
+ (config-with-sanitizer-port (config-with-sanitizer)))
+
+ (test-equal "string value, sanitized to number"
+ 56
+ (config-with-sanitizer-port (config-with-sanitizer
+ (port "56"))))
+
+ (define (custom-serialize-port field-name value)
+ (number->string value))
+
+ (define-configuration config-serializer
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (serializer custom-serialize-port)))
+
+ (test-equal "default value, serializer literal"
+ "80"
+ (eval-gexp
+ (serialize-configuration (config-serializer)
+ config-serializer-fields))))
+
+(test-group "empty-serializer as literal/procedure tests"
+ (define-configuration config-with-literal
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ empty-serializer))
+
+ (define-configuration config-with-proc
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (serializer empty-serializer)))
+
+ (test-equal "empty-serializer as literal"
+ ""
+ (eval-gexp
+ (serialize-configuration (config-with-literal)
+ config-with-literal-fields)))
+
+ (test-equal "empty-serializer as procedure"
+ ""
+ (eval-gexp
+ (serialize-configuration (config-with-proc)
+ config-with-proc-fields))))
+
+(test-group "permutation tests"
+ (define-configuration config-san+empty-ser
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ empty-serializer))
+
+ (define-configuration config-san+ser
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ (serializer (lambda _ "foo"))))
+
+ (test-equal "default value, sanitizer, permutation"
+ 80
+ (config-san+empty-ser-port (config-san+empty-ser)))
+
+ (test-equal "default value, serializer, permutation"
+ "foo"
+ (eval-gexp
+ (serialize-configuration (config-san+ser) config-san+ser-fields)))
+
+ (test-equal "string value sanitized to number, permutation"
+ 56
+ (config-san+ser-port (config-san+ser
+ (port "56"))))
+
+ ;; Ordering tests.
+ (define-configuration config-ser+san
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ (serializer (lambda _ "foo"))))
+
+ (define-configuration config-empty-ser+san
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ empty-serializer
+ (sanitizer sanitize-port)))
+
+ (test-equal "default value, sanitizer, permutation 2"
+ 56
+ (config-empty-ser+san-port (config-empty-ser+san
+ (port "56"))))
+
+ (test-equal "default value, serializer, permutation 2"
+ "foo"
+ (eval-gexp
+ (serialize-configuration (config-ser+san) config-ser+san-fields))))
+
+(test-group "duplicated/conflicting entries"
+ (test-error
+ "duplicate sanitizer" #t
+ (macroexpand '(define-configuration dupe-san
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (sanitizer (lambda () #t))
+ (sanitizer (lambda () #t))))))
+
+ (test-error
+ "duplicate serializer" #t
+ (macroexpand '(define-configuration dupe-ser
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (serializer (lambda _ ""))
+ (serializer (lambda _ ""))))))
+
+ (test-error
+ "conflicting use of serializer + empty-serializer" #t
+ (macroexpand '(define-configuration ser+empty-ser
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (serializer (lambda _ "lorem"))
+ empty-serializer)))))
+
+(test-group "Mix of deprecated and new syntax"
+ (test-error
+ "Mix of bare serializer and new syntax" #t
+ (macroexpand '(define-configuration mixed
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (sanitizer (lambda () #t))
+ (lambda _ "lorem")))))
+
+ (test-error
+ "Mix of bare serializer and new syntax, permutation)" #t
+ (macroexpand '(define-configuration mixed
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (lambda _ "lorem")
+ (sanitizer (lambda () #t)))))))
+
\f
;;;
;;; define-maybe macro.
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v3 2/5] services: replace bare serializers with (serializer ...)
2023-03-25 0:46 ` [bug#62298] [PATCH v3 1/5] " Bruno Victal
@ 2023-03-25 0:46 ` Bruno Victal
2023-03-25 0:46 ` [bug#62298] [PATCH v3 3/5] services: mpd: Fix unintentional API breakage for mixer-type field Bruno Victal
` (2 subsequent siblings)
3 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-25 0:46 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/home/services/shells.scm
(home-zsh-configuration)[environment-variables]: Use (serializer ...).
(home-bash-configuration)[aliases, environment-variables]: Ditto.
(home-fish-configuration)[abbreviations, aliases, environment-variables]: Ditto.
* gnu/services/audio.scm (mpd-configuration)[music-dir, playlist-dir, endpoints]
[address, inputs, archive-plugins, input-cache-size, decoders, filters]
[playlist-plugins]: Ditto.
* gnu/services/linux.scm (fstrim-configuration)[extra-arguments]: Ditto.
* gnu/services/security.scm (fail2ban-jail-configuration)[backend, log-encoding]
[extra-content]: Ditto.
* tests/services/configuration.scm: Update tests.
Add test for deprecated bare serializers.
---
gnu/home/services/shells.scm | 12 ++++-----
gnu/services/audio.scm | 45 ++++++++++++++++----------------
gnu/services/linux.scm | 7 ++---
gnu/services/security.scm | 6 ++---
tests/services/configuration.scm | 11 +++++++-
5 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index 3326eb37f4..f05f2221d6 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -133,7 +133,7 @@ (define-configuration home-zsh-configuration
(environment-variables
(alist '())
"Association list of environment variables to set for the Zsh session."
- serialize-posix-env-vars)
+ (serializer serialize-posix-env-vars))
(zshenv
(text-config '())
"List of file-like objects, which will be added to @file{.zshenv}.
@@ -334,7 +334,7 @@ (define-configuration home-bash-configuration
rules for the @code{home-environment-variables-service-type} apply
here (@pxref{Essential Home Services}). The contents of this field will be
added after the contents of the @code{bash-profile} field."
- serialize-posix-env-vars)
+ (serializer serialize-posix-env-vars))
(aliases
(alist '())
"Association list of aliases to set for the Bash session. The aliases will be
@@ -351,7 +351,7 @@ (define-configuration home-bash-configuration
@example
alias ls=\"ls -alF\"
@end example"
- bash-serialize-aliases)
+ (serializer bash-serialize-aliases))
(bash-profile
(text-config '())
"List of file-like objects, which will be added to @file{.bash_profile}.
@@ -536,19 +536,19 @@ (define-configuration home-fish-configuration
(environment-variables
(alist '())
"Association list of environment variables to set in Fish."
- serialize-fish-env-vars)
+ (serializer serialize-fish-env-vars))
(aliases
(alist '())
"Association list of aliases for Fish, both the key and the value
should be a string. An alias is just a simple function that wraps a
command, If you want something more akin to @dfn{aliases} in POSIX
shells, see the @code{abbreviations} field."
- serialize-fish-aliases)
+ (serializer serialize-fish-aliases))
(abbreviations
(alist '())
"Association list of abbreviations for Fish. These are words that,
when typed in the shell, will automatically expand to the full text."
- serialize-fish-abbreviations))
+ (serializer serialize-fish-abbreviations)))
(define (fish-files-service config)
`(("fish/config.fish"
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 4885fb8424..c073b85a32 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -370,7 +370,7 @@ (define-configuration mpd-configuration
(music-dir ; TODO: deprecated, remove later
maybe-string
"The directory to scan for music files."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(playlist-directory
maybe-string
@@ -379,7 +379,7 @@ (define-configuration mpd-configuration
(playlist-dir ; TODO: deprecated, remove later
maybe-string
"The directory to store playlists."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(db-file
maybe-string
@@ -405,16 +405,17 @@ (define-configuration mpd-configuration
port is used.
To use a Unix domain socket, an absolute path or a path starting with @code{~}
can be specified here."
- (lambda (_ endpoints)
- (if (maybe-value-set? endpoints)
- (mpd-serialize-list-of-strings "bind_to_address" endpoints)
- "")))
+ (serializer
+ (lambda (_ endpoints)
+ (if (maybe-value-set? endpoints)
+ (mpd-serialize-list-of-strings "bind_to_address" endpoints)
+ ""))))
(address ; TODO: deprecated, remove later
maybe-string
"The address that mpd will bind to.
To use a Unix domain socket, an absolute path can be specified here."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(database
maybe-mpd-plugin
@@ -431,29 +432,29 @@ (define-configuration mpd-configuration
(inputs
(list-of-mpd-plugin '())
"List of MPD input plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "input" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "input" x))))
(archive-plugins
(list-of-mpd-plugin '())
"List of MPD archive plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "archive_plugin" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "archive_plugin" x))))
(input-cache-size
maybe-string
"MPD input cache size."
- (lambda (_ x)
- (if (maybe-value-set? x)
- #~(string-append "\ninput_cache {\n"
- #$(mpd-serialize-string "size" x)
- "}\n") "")))
+ (serializer (lambda (_ x)
+ (if (maybe-value-set? x)
+ #~(string-append "\ninput_cache {\n"
+ #$(mpd-serialize-string "size" x)
+ "}\n") ""))))
(decoders
(list-of-mpd-plugin '())
"List of MPD decoder plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "decoder" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "decoder" x))))
(resampler
maybe-mpd-plugin
@@ -462,8 +463,8 @@ (define-configuration mpd-configuration
(filters
(list-of-mpd-plugin '())
"List of MPD filter plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "filter" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "filter" x))))
(outputs
(list-of-mpd-plugin-or-output (list (mpd-output)))
@@ -473,8 +474,8 @@ (define-configuration mpd-configuration
(playlist-plugins
(list-of-mpd-plugin '())
"List of MPD playlist plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x))))
(extra-options
(alist '())
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index d085b375a2..229220eeb1 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -213,9 +213,10 @@ (define-configuration fstrim-configuration
maybe-list-of-strings
"Extra options to append to @command{fstrim} (run @samp{man fstrim} for
more information)."
- (lambda (_ value)
- (if (maybe-value-set? value)
- value '())))
+ (serializer
+ (lambda (_ value)
+ (if (maybe-value-set? value)
+ value '()))))
(prefix fstrim-))
(define (serialize-fstrim-configuration config)
diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 8116072920..e750bb468b 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -200,7 +200,7 @@ (define-configuration fail2ban-jail-configuration
"Backend to use to detect changes in the @code{log-path}. The default is
'auto. To consult the defaults of the jail configuration, refer to the
@file{/etc/fail2ban/jail.conf} file of the @code{fail2ban} package."
- fail2ban-jail-configuration-serialize-backend)
+ (serializer fail2ban-jail-configuration-serialize-backend))
(max-retry
maybe-integer
"The number of failures before a host get banned
@@ -269,7 +269,7 @@ (define-configuration fail2ban-jail-configuration
maybe-symbol
"The encoding of the log files handled by the jail.
Possible values are: @code{'ascii}, @code{'utf-8} and @code{'auto}."
- fail2ban-jail-configuration-serialize-log-encoding)
+ (serializer fail2ban-jail-configuration-serialize-log-encoding))
(log-path
(list-of-strings '())
"The file names of the log files to be monitored.")
@@ -280,7 +280,7 @@ (define-configuration fail2ban-jail-configuration
(text-config '())
"Extra content for the jail configuration, provided as a list of file-like
objects."
- serialize-text-config)
+ (serializer serialize-text-config))
(prefix fail2ban-jail-configuration-))
(define list-of-fail2ban-jail-configurations?
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 0392cce927..8ad5907f37 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -82,6 +82,9 @@ (define (custom-number-serializer name value)
(format #f "~a = ~a;" name value))
(define-configuration serializable-configuration
+ (port (number 80) "The port number." (serializer custom-number-serializer)))
+
+(define-configuration serializable-configuration-deprecated
(port (number 80) "The port number." custom-number-serializer))
(test-assert "serialize-configuration"
@@ -89,8 +92,14 @@ (define-configuration serializable-configuration
(let ((config (serializable-configuration)))
(serialize-configuration config serializable-configuration-fields))))
+(test-assert "serialize-configuration [deprecated]"
+ (gexp?
+ (let ((config (serializable-configuration-deprecated)))
+ (serialize-configuration
+ config serializable-configuration-deprecated-fields))))
+
(define-configuration serializable-configuration
- (port (number 80) "The port number." custom-number-serializer)
+ (port (number 80) "The port number." (serializer custom-number-serializer))
(no-serialization))
(test-assert "serialize-configuration with no-serialization"
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v3 3/5] services: mpd: Fix unintentional API breakage for mixer-type field.
2023-03-25 0:46 ` [bug#62298] [PATCH v3 1/5] " Bruno Victal
2023-03-25 0:46 ` [bug#62298] [PATCH v3 2/5] services: replace bare serializers with (serializer ...) Bruno Victal
@ 2023-03-25 0:46 ` Bruno Victal
2023-03-25 0:46 ` [bug#62298] [PATCH v3 4/5] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
2023-03-25 0:46 ` [bug#62298] [PATCH v3 5/5] services: mympd: " Bruno Victal
3 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-25 0:46 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (mpd-output)[mixer-type]: Use sanitizer to
accept both strings and symbols as values.
---
gnu/services/audio.scm | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index c073b85a32..bc4aed71dc 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,6 +140,11 @@ (define (uglify-field-name field-name)
(define list-of-symbol?
(list-of symbol?))
+\f
+;;;
+;;; MPD
+;;;
+
(define (mpd-serialize-field field-name value)
(let ((field (if (string? field-name) field-name
(uglify-field-name field-name)))
@@ -294,7 +299,17 @@ (define-configuration mpd-output
for this audio output: the @code{hardware} mixer, the @code{software}
mixer, the @code{null} mixer (allows setting the volume, but with no
effect; this can be used as a trick to implement an external mixer
-External Mixer) or no mixer (@code{none}).")
+External Mixer) or no mixer (@code{none})."
+ (sanitizer
+ (lambda (x) ; TODO: deprecated, remove me later.
+ (cond
+ ((symbol? x)
+ (warning (G_ "symbol value for 'mixer-type' is deprecated, \
+use string instead~%"))
+ (symbol->string x))
+ ((string? x) x)
+ (else
+ (configuration-field-error #f 'mixer-type x))))))
(replay-gain-handler
maybe-string
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v3 4/5] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-25 0:46 ` [bug#62298] [PATCH v3 1/5] " Bruno Victal
2023-03-25 0:46 ` [bug#62298] [PATCH v3 2/5] services: replace bare serializers with (serializer ...) Bruno Victal
2023-03-25 0:46 ` [bug#62298] [PATCH v3 3/5] services: mpd: Fix unintentional API breakage for mixer-type field Bruno Victal
@ 2023-03-25 0:46 ` Bruno Victal
2023-03-25 0:46 ` [bug#62298] [PATCH v3 5/5] services: mympd: " Bruno Victal
3 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-25 0:46 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
Deprecate using strings for these fields and prefer user-account
(resp. user-group) instead to avoid duplication within account-service-type.
Fixes #61570 <https://issues.guix.gnu.org/61570>.
* gnu/services/audio.scm (mpd-serialize-user-account, mpd-serialize-user-group)
(mpd-user-sanitizer, mpd-group-sanitizer): New procedure.
(%mpd-user, %mpd-group): New variable.
(mpd-configuration)[user, group]: Set value type to
user-account (resp. user-group).
(mpd-shepherd-service): Adapt for user-account values in user field.
(mpd-accounts): Adapt for user-account (resp. user-group) in
user (resp. group) field.
* doc/guix.texi (Audio Services): Update documentation.
---
doc/guix.texi | 4 +-
gnu/services/audio.scm | 89 ++++++++++++++++++++++++++++++++++--------
2 files changed, 74 insertions(+), 19 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 8604b95f94..10ce098a21 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33491,10 +33491,10 @@ Audio Services
@item @code{package} (default: @code{mpd}) (type: file-like)
The MPD package.
-@item @code{user} (default: @code{"mpd"}) (type: string)
+@item @code{user} (type: maybe-user-account)
The user to run mpd as.
-@item @code{group} (default: @code{"mpd"}) (type: string)
+@item @code{group} (type: maybe-user-group)
The group to run mpd as.
@item @code{shepherd-requirement} (default: @code{()}) (type: list-of-symbol)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index bc4aed71dc..55749111ab 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,6 +140,14 @@ (define (uglify-field-name field-name)
(define list-of-symbol?
(list-of symbol?))
+;; Helpers for deprecated field types, to be removed later.
+(define %lazy-group (make-symbol "%lazy-group"))
+
+(define (inject-group-into-user user group)
+ (user-account
+ (inherit user)
+ (group (user-group-name group))))
+
\f
;;;
;;; MPD
@@ -164,9 +172,32 @@ (define mpd-serialize-boolean mpd-serialize-field)
(define (mpd-serialize-list-of-strings field-name value)
#~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
+(define (mpd-serialize-user-account field-name value)
+ (mpd-serialize-string field-name (user-account-name value)))
+
+(define (mpd-serialize-user-group field-name value)
+ (mpd-serialize-string field-name (user-group-name value)))
+
(define-maybe string (prefix mpd-))
(define-maybe list-of-strings (prefix mpd-))
(define-maybe boolean (prefix mpd-))
+(define-maybe user-account (prefix mpd-))
+(define-maybe user-group (prefix mpd-))
+
+(define %mpd-user
+ (user-account
+ (name "mpd")
+ (group "mpd")
+ (system? #t)
+ (comment "Music Player Daemon (MPD) user")
+ ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
+ (home-directory "/var/lib/mpd")
+ (shell (file-append shadow "/sbin/nologin"))))
+
+(define %mpd-group
+ (user-group
+ (name "mpd")
+ (system? #t)))
;;; TODO: Procedures for deprecated fields, to be removed.
@@ -197,6 +228,33 @@ (define (mpd-serialize-port field-name value)
(define-maybe port (prefix mpd-))
+;;; Procedures for unsupported value types, to be removed.
+
+(define (mpd-user-sanitizer value)
+ (cond ((user-account? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'user' is deprecated, use \
+user-account instead~%"))
+ (user-account
+ (inherit %mpd-user)
+ (name value)
+ ;; XXX: This is to be lazily substituted in (…-accounts)
+ ;; with the value from 'group'.
+ (group %lazy-group)))
+ (else
+ (configuration-field-error #f 'user value))))
+
+(define (mpd-group-sanitizer value)
+ (cond ((user-group? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'group' is deprecated, use \
+user-group instead~%"))
+ (user-group
+ (inherit %mpd-group)
+ (name value)))
+ (else
+ (configuration-field-error #f 'group value))))
+
;;;
;; Generic MPD plugin record, lists only the most prevalent fields.
@@ -347,12 +405,14 @@ (define-configuration mpd-configuration
empty-serializer)
(user
- (string "mpd")
- "The user to run mpd as.")
+ (maybe-user-account %mpd-user)
+ "The user to run mpd as."
+ (sanitizer mpd-user-sanitizer))
(group
- (string "mpd")
- "The group to run mpd as.")
+ (maybe-user-group %mpd-group)
+ "The group to run mpd as."
+ (sanitizer mpd-group-sanitizer))
(shepherd-requirement
(list-of-symbol '())
@@ -517,7 +577,8 @@ (define (mpd-shepherd-service config)
log-file playlist-directory
db-file state-file sticker-file
environment-variables)
- (let* ((config-file (mpd-serialize-configuration config)))
+ (let ((config-file (mpd-serialize-configuration config))
+ (username (user-account-name user)))
(shepherd-service
(documentation "Run the MPD (Music Player Daemon)")
(requirement `(user-processes loopback ,@shepherd-requirement))
@@ -526,7 +587,7 @@ (define (mpd-shepherd-service config)
(and=> #$(maybe-value log-file)
(compose mkdir-p dirname))
- (let ((user (getpw #$user)))
+ (let ((user (getpw #$username)))
(for-each
(lambda (x)
(when (and x (not (file-exists? x)))
@@ -560,17 +621,11 @@ (define (mpd-shepherd-service config)
(define (mpd-accounts config)
(match-record config <mpd-configuration> (user group)
- (list (user-group
- (name group)
- (system? #t))
- (user-account
- (name user)
- (group group)
- (system? #t)
- (comment "Music Player Daemon (MPD) user")
- ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
- (home-directory "/var/lib/mpd")
- (shell (file-append shadow "/sbin/nologin"))))))
+ ;; TODO: Deprecation code, to be removed.
+ (let ((user (if (eq? (user-account-group user) %lazy-group)
+ (inject-group-into-user user group)
+ user)))
+ (list user group))))
(define mpd-service-type
(service-type
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v3 5/5] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-25 0:46 ` [bug#62298] [PATCH v3 1/5] " Bruno Victal
` (2 preceding siblings ...)
2023-03-25 0:46 ` [bug#62298] [PATCH v3 4/5] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
@ 2023-03-25 0:46 ` Bruno Victal
3 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-25 0:46 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (%mympd-user, %mympd-group): New variable.
(mympd-user-sanitizer, mympd-group-sanitizer): New procedure.
(mympd-configuration)[user, group]: Set value type to
user-account (resp. user-group).
(mympd-serialize-configuration): Adapt for user-account values in user field.
(mympd-accounts): Adapt for user-account (resp. user-group) in
user (resp. group) field.
---
doc/guix.texi | 4 +--
gnu/services/audio.scm | 75 ++++++++++++++++++++++++++++++++++--------
2 files changed, 63 insertions(+), 16 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 10ce098a21..1e36ea5183 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33732,10 +33732,10 @@ Audio Services
This is a list of symbols naming Shepherd services that this service
will depend on.
-@item @code{user} (default: @code{"mympd"}) (type: string)
+@item @code{user} (type: maybe-user-account)
Owner of the @command{mympd} process.
-@item @code{group} (default: @code{"nogroup"}) (type: string)
+@item @code{group} (type: maybe-user-group)
Owner group of the @command{mympd} process.
@item @code{work-directory} (default: @code{"/var/lib/mympd"}) (type: string)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 55749111ab..5faec5a0df 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -659,6 +659,53 @@ (define-configuration/no-serialization mympd-ip-acl
(define-maybe/no-serialization integer)
(define-maybe/no-serialization mympd-ip-acl)
+;; XXX: These will shadow the previous definition used by mpd
+;; and cause warnings to be shown. Maybe split the file
+;; into audio/mpd.scm and audio/mympd.scm ?
+#;(define-maybe/no-serialization user-account)
+#;(define-maybe/no-serialization user-group)
+
+(define %mympd-user
+ (user-account
+ (name "mympd")
+ (group "mympd")
+ (system? #t)
+ (comment "myMPD user")
+ (home-directory "/var/empty")
+ (shell (file-append shadow "/sbin/nologin"))))
+
+(define %mympd-group
+ (user-group
+ (name "mympd")
+ (system? #t)))
+
+;;; TODO: Procedures for unsupported value types, to be removed.
+(define (mympd-user-sanitizer value)
+ (cond ((user-account? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'user' is not supported, use \
+user-account instead~%"))
+ (user-account
+ (inherit %mympd-user)
+ (name value)
+ ;; XXX: this is to be lazily substituted in (…-accounts)
+ ;; with the value from 'group'.
+ (group %lazy-group)))
+ (else
+ (configuration-field-error #f 'user value))))
+
+(define (mympd-group-sanitizer value)
+ (cond ((user-group? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'group' is not supported, use \
+user-group instead~%"))
+ (user-group
+ (inherit %mympd-group)
+ (name value)))
+ (else
+ (configuration-field-error #f 'group value))))
+;;;
+
;; XXX: The serialization procedures are insufficient since we require
;; access to multiple fields at once.
@@ -677,13 +724,15 @@ (define-configuration/no-serialization mympd-configuration
empty-serializer)
(user
- (string "mympd")
+ (maybe-user-account %mympd-user)
"Owner of the @command{mympd} process."
+ (sanitizer mympd-user-sanitizer)
empty-serializer)
(group
- (string "nogroup")
+ (maybe-user-group %mympd-group)
"Owner group of the @command{mympd} process."
+ (sanitizer mympd-group-sanitizer)
empty-serializer)
(work-directory
@@ -818,7 +867,8 @@ (define (mympd-shepherd-service config)
(match-record config <mympd-configuration> (package shepherd-requirement
user work-directory
cache-directory log-level log-to)
- (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level)))
+ (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level))
+ (username (user-account-name user)))
(shepherd-service
(documentation "Run the myMPD daemon.")
(requirement `(loopback user-processes
@@ -828,7 +878,7 @@ (define (mympd-shepherd-service config)
,@shepherd-requirement))
(provision '(mympd))
(start #~(begin
- (let* ((pw (getpwnam #$user))
+ (let* ((pw (getpwnam #$username))
(uid (passwd:uid pw))
(gid (passwd:gid pw)))
(for-each (lambda (dir)
@@ -838,8 +888,8 @@ (define (mympd-shepherd-service config)
(make-forkexec-constructor
`(#$(file-append package "/bin/mympd")
- "--user" #$user
- #$@(if (eqv? log-to 'syslog) '("--syslog") '())
+ "--user" #$username
+ #$@(if (eq? log-to 'syslog) '("--syslog") '())
"--workdir" #$work-directory
"--cachedir" #$cache-directory)
#:environment-variables (list #$log-level*)
@@ -848,14 +898,11 @@ (define (mympd-shepherd-service config)
(define (mympd-accounts config)
(match-record config <mympd-configuration> (user group)
- (list (user-group (name group)
- (system? #t))
- (user-account (name user)
- (group group)
- (system? #t)
- (comment "myMPD user")
- (home-directory "/var/empty")
- (shell (file-append shadow "/sbin/nologin"))))))
+ ;; TODO: Deprecation code, to be removed.
+ (let ((user (if (eq? (user-account-group user) %lazy-group)
+ (inject-group-into-user user group)
+ user)))
+ (list user group))))
(define (mympd-log-rotation config)
(match-record config <mympd-configuration> (log-to)
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [bug#62298] [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support.
2023-03-20 16:45 [bug#62298] [PATCH 0/8] Extensible define-configuration & mpd/mympd service fixes Bruno Victal
` (9 preceding siblings ...)
2023-03-25 0:46 ` [bug#62298] [PATCH v3 1/5] " Bruno Victal
@ 2023-03-26 18:41 ` Bruno Victal
2023-03-26 18:41 ` [bug#62298] [PATCH v4 2/5] services: replace bare serializers with (serializer ...) Bruno Victal
` (4 more replies)
10 siblings, 5 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-26 18:41 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
This changes the 'custom-serializer' field into a generic
'extra-args' field that can be extended to support new literals.
With this mechanism, the literals 'sanitizer' allow for user-defined
sanitizer procedures while the 'serializer' literal is used for
custom serializer procedures.
The 'empty-serializer' was also added as a 'literal' and can be used
just like it was previously.
With the repurposing of 'custom-serializer' into 'extra-args', to prevent
intolerable confusion, the custom serializer procedures should be
specified using the new 'literals' approach, with the previous “style”
being considered deprecated.
* gnu/services/configuration.scm (define-configuration-helper):
Rename 'custom-serializer' to 'extra-args'. Add support for literals
'sanitizer', 'serializer' and 'empty-serializer'. Rename procedure
'field-sanitizer' to 'default-field-sanitizer' to avoid syntax clash.
Only define default field sanitizers if user-defined ones are absent.
(normalize-extra-args): New procedure.
(<configuration-field>)[sanitizer]: New field.
* doc/guix.texi (Complex Configurations): Document the newly added literals.
* tests/services/configuration.scm: Add tests for the new literals.
---
Notable changes from v3 to v4:
* Removed define-maybe usage for user-account and user-group.
doc/guix.texi | 29 ++++-
gnu/services/configuration.scm | 90 +++++++++++----
tests/services/configuration.scm | 183 ++++++++++++++++++++++++++++++-
3 files changed, 276 insertions(+), 26 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 3e335306f1..8604b95f94 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -41216,7 +41216,7 @@ Complex Configurations
(@var{field-name}
(@var{type} @var{default-value})
@var{documentation}
- @var{serializer})
+ (serializer @var{serializer}))
(@var{field-name}
(@var{type})
@@ -41225,7 +41225,18 @@ Complex Configurations
(@var{field-name}
(@var{type})
@var{documentation}
- @var{serializer})
+ (serializer @var{serializer}))
+
+(@var{field-name}
+ (@var{type})
+ @var{documentation}
+ (sanitizer @var{sanitizer})
+
+(@var{field-name}
+ (@var{type})
+ @var{documentation}
+ (sanitizer @var{sanitizer})
+ (serializer @var{serializer}))
@end example
@var{field-name} is an identifier that denotes the name of the field in
@@ -41248,6 +41259,20 @@ Complex Configurations
@var{documentation} is a string formatted with Texinfo syntax which
should provide a description of what setting this field does.
+@var{sanitizer} is a procedure which takes one argument,
+a user-supplied value, and returns a ``sanitized'' value for the field.
+If no sanitizer is specified, a default sanitizer is used, which raises
+an error if the value is not of type @var{type}.
+
+An example of a sanitizer for a field that accepts both strings and
+symbols looks like this:
+@lisp
+(define (sanitize-foo value)
+ (cond ((string? value) value)
+ ((symbol? value) (symbol->string value))
+ (else (error "bad value"))))
+@end lisp
+
@var{serializer} is the name of a procedure which takes two arguments,
the first is the name of the field, and the second is the value
corresponding to the field. The procedure should return a string or
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 174c2f20d2..880eba8138 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -6,6 +6,7 @@
;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2021 Andrew Tropin <andrew@trop.in>
;;; Copyright © 2022 Maxime Devos <maximedevos@telenet.be>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -28,7 +29,8 @@ (define-module (gnu services configuration)
#:use-module (guix gexp)
#:use-module ((guix utils) #:select (source-properties->location))
#:use-module ((guix diagnostics)
- #:select (formatted-message location-file &error-location))
+ #:select (formatted-message location-file &error-location
+ warning))
#:use-module ((guix modules) #:select (file-name->module-name))
#:use-module (guix i18n)
#:autoload (texinfo) (texi-fragment->stexi)
@@ -37,6 +39,7 @@ (define-module (gnu services configuration)
#:use-module (ice-9 format)
#:use-module (ice-9 match)
#:use-module (srfi srfi-1)
+ #:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-35)
#:export (configuration-field
@@ -44,6 +47,7 @@ (define-module (gnu services configuration)
configuration-field-type
configuration-missing-field
configuration-field-error
+ configuration-field-sanitizer
configuration-field-serializer
configuration-field-getter
configuration-field-default-value-thunk
@@ -116,6 +120,7 @@ (define-record-type* <configuration-field>
(type configuration-field-type)
(getter configuration-field-getter)
(predicate configuration-field-predicate)
+ (sanitizer configuration-field-sanitizer)
(serializer configuration-field-serializer)
(default-value-thunk configuration-field-default-value-thunk)
(documentation configuration-field-documentation))
@@ -181,11 +186,44 @@ (define (normalize-field-type+def s)
(values #'(field-type %unset-value)))))
(define (define-configuration-helper serialize? serializer-prefix syn)
+
+ (define (normalize-extra-args s)
+ "Extract and normalize arguments following @var{doc}."
+ (let loop ((s s)
+ (sanitizer* %unset-value)
+ (serializer* %unset-value))
+ (syntax-case s (sanitizer serializer empty-serializer)
+ (((sanitizer proc) tail ...)
+ (if (maybe-value-set? sanitizer*)
+ (syntax-violation 'sanitizer "duplicate entry"
+ #'proc)
+ (loop #'(tail ...) #'proc serializer*)))
+ (((serializer proc) tail ...)
+ (if (maybe-value-set? serializer*)
+ (syntax-violation 'serializer "duplicate or conflicting entry"
+ #'proc)
+ (loop #'(tail ...) sanitizer* #'proc)))
+ ((empty-serializer tail ...)
+ (if (maybe-value-set? serializer*)
+ (syntax-violation 'empty-serializer
+ "duplicate or conflicting entry" #f)
+ (loop #'(tail ...) sanitizer* #'empty-serializer)))
+ (() ; stop condition
+ (values (list sanitizer* serializer*)))
+ ((proc) ; TODO: deprecated, to be removed.
+ (null? (filter-map maybe-value-set? (list sanitizer* serializer*)))
+ (begin
+ (warning #f (G_ "specifying serializers after documentation is \
+deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
+ (values (list %unset-value #'proc)))))))
+
(syntax-case syn ()
- ((_ stem (field field-type+def doc custom-serializer ...) ...)
+ ((_ stem (field field-type+def doc extra-args ...) ...)
(with-syntax
((((field-type def) ...)
- (map normalize-field-type+def #'(field-type+def ...))))
+ (map normalize-field-type+def #'(field-type+def ...)))
+ (((sanitizer* serializer*) ...)
+ (map normalize-extra-args #'((extra-args ...) ...))))
(with-syntax
(((field-getter ...)
(map (lambda (field)
@@ -200,21 +238,18 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
((field-type default-value)
default-value))
#'((field-type def) ...)))
+ ((field-sanitizer ...)
+ (map maybe-value #'(sanitizer* ...)))
((field-serializer ...)
- (map (lambda (type custom-serializer)
+ (map (lambda (type proc)
(and serialize?
- (match custom-serializer
- ((serializer)
- serializer)
- (()
- (if serializer-prefix
- (id #'stem
- serializer-prefix
- #'serialize- type)
- (id #'stem #'serialize- type))))))
+ (or (maybe-value proc)
+ (if serializer-prefix
+ (id #'stem serializer-prefix #'serialize- type)
+ (id #'stem #'serialize- type)))))
#'(field-type ...)
- #'((custom-serializer ...) ...))))
- (define (field-sanitizer name pred)
+ #'(serializer* ...))))
+ (define (default-field-sanitizer name pred)
;; Define a macro for use as a record field sanitizer, where NAME
;; is the name of the field and PRED is the predicate that tells
;; whether a value is valid for this field.
@@ -235,21 +270,29 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
#`(begin
;; Define field validation macros.
- #,@(map field-sanitizer
- #'(field ...)
- #'(field-predicate ...))
+ #,@(filter-map (lambda (name pred sanitizer)
+ (if sanitizer
+ #f
+ (default-field-sanitizer name pred)))
+ #'(field ...)
+ #'(field-predicate ...)
+ #'(field-sanitizer ...))
(define-record-type* #,(id #'stem #'< #'stem #'>)
stem
#,(id #'stem #'make- #'stem)
#,(id #'stem #'stem #'?)
- #,@(map (lambda (name getter def)
- #`(#,name #,getter (default #,def)
+ #,@(map (lambda (name getter def sanitizer)
+ #`(#,name #,getter
+ (default #,def)
(sanitize
- #,(id #'stem #'validate- #'stem #'- name))))
+ #,(or sanitizer
+ (id #'stem
+ #'validate- #'stem #'- name)))))
#'(field ...)
#'(field-getter ...)
- #'(field-default ...))
+ #'(field-default ...)
+ #'(field-sanitizer ...))
(%location #,(id #'stem #'stem #'-source-location)
(default (and=> (current-source-location)
source-properties->location))
@@ -261,6 +304,9 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
(type 'field-type)
(getter field-getter)
(predicate field-predicate)
+ (sanitizer
+ (or field-sanitizer
+ (id #'stem #'validate- #'stem #'- #'field)))
(serializer field-serializer)
(default-value-thunk
(lambda ()
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 4f8a74dc8a..0392cce927 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -2,6 +2,7 @@
;;; Copyright © 2021, 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
;;; Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -22,6 +23,7 @@ (define-module (tests services configuration)
#:use-module (gnu services configuration)
#:use-module (guix diagnostics)
#:use-module (guix gexp)
+ #:autoload (guix i18n) (G_)
#:use-module (srfi srfi-34)
#:use-module (srfi srfi-64))
@@ -46,14 +48,14 @@ (define-configuration port-configuration
(port-configuration-port (port-configuration)))
(test-equal "wrong type for a field"
- '("configuration.scm" 57 11) ;error location
+ '("configuration.scm" 59 11) ;error location
(guard (c ((configuration-error? c)
(let ((loc (error-location c)))
(list (basename (location-file loc))
(location-line loc)
(location-column loc)))))
(port-configuration
- ;; This is line 56; the test relies on line/column numbers!
+ ;; This is line 58; the test relies on line/column numbers!
(port "This is not a number!"))))
(define-configuration port-configuration-cs
@@ -109,6 +111,183 @@ (define-configuration configuration-with-prefix
(let ((config (configuration-with-prefix)))
(serialize-configuration config configuration-with-prefix-fields))))
+\f
+;;;
+;;; define-configuration macro, extra-args literals
+;;;
+
+(define (eval-gexp x)
+ "Get serialized config as string."
+ (eval (gexp->approximate-sexp x)
+ (current-module)))
+
+(define (port? value)
+ (or (string? value) (number? value)))
+
+(define (sanitize-port value)
+ (cond ((number? value) value)
+ ((string? value) (string->number value))
+ (else (raise (formatted-message (G_ "Bad value: ~a") value)))))
+
+(test-group "Basic sanitizer literal tests"
+ (define serialize-port serialize-number)
+
+ (define-configuration config-with-sanitizer
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)))
+
+ (test-equal "default value, sanitizer"
+ 80
+ (config-with-sanitizer-port (config-with-sanitizer)))
+
+ (test-equal "string value, sanitized to number"
+ 56
+ (config-with-sanitizer-port (config-with-sanitizer
+ (port "56"))))
+
+ (define (custom-serialize-port field-name value)
+ (number->string value))
+
+ (define-configuration config-serializer
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (serializer custom-serialize-port)))
+
+ (test-equal "default value, serializer literal"
+ "80"
+ (eval-gexp
+ (serialize-configuration (config-serializer)
+ config-serializer-fields))))
+
+(test-group "empty-serializer as literal/procedure tests"
+ (define-configuration config-with-literal
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ empty-serializer))
+
+ (define-configuration config-with-proc
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (serializer empty-serializer)))
+
+ (test-equal "empty-serializer as literal"
+ ""
+ (eval-gexp
+ (serialize-configuration (config-with-literal)
+ config-with-literal-fields)))
+
+ (test-equal "empty-serializer as procedure"
+ ""
+ (eval-gexp
+ (serialize-configuration (config-with-proc)
+ config-with-proc-fields))))
+
+(test-group "permutation tests"
+ (define-configuration config-san+empty-ser
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ empty-serializer))
+
+ (define-configuration config-san+ser
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ (serializer (lambda _ "foo"))))
+
+ (test-equal "default value, sanitizer, permutation"
+ 80
+ (config-san+empty-ser-port (config-san+empty-ser)))
+
+ (test-equal "default value, serializer, permutation"
+ "foo"
+ (eval-gexp
+ (serialize-configuration (config-san+ser) config-san+ser-fields)))
+
+ (test-equal "string value sanitized to number, permutation"
+ 56
+ (config-san+ser-port (config-san+ser
+ (port "56"))))
+
+ ;; Ordering tests.
+ (define-configuration config-ser+san
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ (sanitizer sanitize-port)
+ (serializer (lambda _ "foo"))))
+
+ (define-configuration config-empty-ser+san
+ (port
+ (port 80)
+ "Lorem Ipsum."
+ empty-serializer
+ (sanitizer sanitize-port)))
+
+ (test-equal "default value, sanitizer, permutation 2"
+ 56
+ (config-empty-ser+san-port (config-empty-ser+san
+ (port "56"))))
+
+ (test-equal "default value, serializer, permutation 2"
+ "foo"
+ (eval-gexp
+ (serialize-configuration (config-ser+san) config-ser+san-fields))))
+
+(test-group "duplicated/conflicting entries"
+ (test-error
+ "duplicate sanitizer" #t
+ (macroexpand '(define-configuration dupe-san
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (sanitizer (lambda () #t))
+ (sanitizer (lambda () #t))))))
+
+ (test-error
+ "duplicate serializer" #t
+ (macroexpand '(define-configuration dupe-ser
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (serializer (lambda _ ""))
+ (serializer (lambda _ ""))))))
+
+ (test-error
+ "conflicting use of serializer + empty-serializer" #t
+ (macroexpand '(define-configuration ser+empty-ser
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (serializer (lambda _ "lorem"))
+ empty-serializer)))))
+
+(test-group "Mix of deprecated and new syntax"
+ (test-error
+ "Mix of bare serializer and new syntax" #t
+ (macroexpand '(define-configuration mixed
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (sanitizer (lambda () #t))
+ (lambda _ "lorem")))))
+
+ (test-error
+ "Mix of bare serializer and new syntax, permutation)" #t
+ (macroexpand '(define-configuration mixed
+ (foo
+ (list '())
+ "Lorem Ipsum."
+ (lambda _ "lorem")
+ (sanitizer (lambda () #t)))))))
+
\f
;;;
;;; define-maybe macro.
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v4 2/5] services: replace bare serializers with (serializer ...)
2023-03-26 18:41 ` [bug#62298] [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Bruno Victal
@ 2023-03-26 18:41 ` Bruno Victal
2023-03-26 18:41 ` [bug#62298] [PATCH v4 3/5] services: mpd: Fix unintentional API breakage for mixer-type field Bruno Victal
` (3 subsequent siblings)
4 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-26 18:41 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/home/services/shells.scm
(home-zsh-configuration)[environment-variables]: Use (serializer ...).
(home-bash-configuration)[aliases, environment-variables]: Ditto.
(home-fish-configuration)[abbreviations, aliases, environment-variables]: Ditto.
* gnu/services/audio.scm (mpd-configuration)[music-dir, playlist-dir, endpoints]
[address, inputs, archive-plugins, input-cache-size, decoders, filters]
[playlist-plugins]: Ditto.
* gnu/services/linux.scm (fstrim-configuration)[extra-arguments]: Ditto.
* gnu/services/security.scm (fail2ban-jail-configuration)[backend, log-encoding]
[extra-content]: Ditto.
* tests/services/configuration.scm: Update tests.
Add test for deprecated bare serializers.
---
gnu/home/services/shells.scm | 12 ++++-----
gnu/services/audio.scm | 45 ++++++++++++++++----------------
gnu/services/linux.scm | 7 ++---
gnu/services/security.scm | 6 ++---
tests/services/configuration.scm | 11 +++++++-
5 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index 3326eb37f4..f05f2221d6 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -133,7 +133,7 @@ (define-configuration home-zsh-configuration
(environment-variables
(alist '())
"Association list of environment variables to set for the Zsh session."
- serialize-posix-env-vars)
+ (serializer serialize-posix-env-vars))
(zshenv
(text-config '())
"List of file-like objects, which will be added to @file{.zshenv}.
@@ -334,7 +334,7 @@ (define-configuration home-bash-configuration
rules for the @code{home-environment-variables-service-type} apply
here (@pxref{Essential Home Services}). The contents of this field will be
added after the contents of the @code{bash-profile} field."
- serialize-posix-env-vars)
+ (serializer serialize-posix-env-vars))
(aliases
(alist '())
"Association list of aliases to set for the Bash session. The aliases will be
@@ -351,7 +351,7 @@ (define-configuration home-bash-configuration
@example
alias ls=\"ls -alF\"
@end example"
- bash-serialize-aliases)
+ (serializer bash-serialize-aliases))
(bash-profile
(text-config '())
"List of file-like objects, which will be added to @file{.bash_profile}.
@@ -536,19 +536,19 @@ (define-configuration home-fish-configuration
(environment-variables
(alist '())
"Association list of environment variables to set in Fish."
- serialize-fish-env-vars)
+ (serializer serialize-fish-env-vars))
(aliases
(alist '())
"Association list of aliases for Fish, both the key and the value
should be a string. An alias is just a simple function that wraps a
command, If you want something more akin to @dfn{aliases} in POSIX
shells, see the @code{abbreviations} field."
- serialize-fish-aliases)
+ (serializer serialize-fish-aliases))
(abbreviations
(alist '())
"Association list of abbreviations for Fish. These are words that,
when typed in the shell, will automatically expand to the full text."
- serialize-fish-abbreviations))
+ (serializer serialize-fish-abbreviations)))
(define (fish-files-service config)
`(("fish/config.fish"
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 4885fb8424..c073b85a32 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -370,7 +370,7 @@ (define-configuration mpd-configuration
(music-dir ; TODO: deprecated, remove later
maybe-string
"The directory to scan for music files."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(playlist-directory
maybe-string
@@ -379,7 +379,7 @@ (define-configuration mpd-configuration
(playlist-dir ; TODO: deprecated, remove later
maybe-string
"The directory to store playlists."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(db-file
maybe-string
@@ -405,16 +405,17 @@ (define-configuration mpd-configuration
port is used.
To use a Unix domain socket, an absolute path or a path starting with @code{~}
can be specified here."
- (lambda (_ endpoints)
- (if (maybe-value-set? endpoints)
- (mpd-serialize-list-of-strings "bind_to_address" endpoints)
- "")))
+ (serializer
+ (lambda (_ endpoints)
+ (if (maybe-value-set? endpoints)
+ (mpd-serialize-list-of-strings "bind_to_address" endpoints)
+ ""))))
(address ; TODO: deprecated, remove later
maybe-string
"The address that mpd will bind to.
To use a Unix domain socket, an absolute path can be specified here."
- mpd-serialize-deprecated-field)
+ (serializer mpd-serialize-deprecated-field))
(database
maybe-mpd-plugin
@@ -431,29 +432,29 @@ (define-configuration mpd-configuration
(inputs
(list-of-mpd-plugin '())
"List of MPD input plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "input" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "input" x))))
(archive-plugins
(list-of-mpd-plugin '())
"List of MPD archive plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "archive_plugin" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "archive_plugin" x))))
(input-cache-size
maybe-string
"MPD input cache size."
- (lambda (_ x)
- (if (maybe-value-set? x)
- #~(string-append "\ninput_cache {\n"
- #$(mpd-serialize-string "size" x)
- "}\n") "")))
+ (serializer (lambda (_ x)
+ (if (maybe-value-set? x)
+ #~(string-append "\ninput_cache {\n"
+ #$(mpd-serialize-string "size" x)
+ "}\n") ""))))
(decoders
(list-of-mpd-plugin '())
"List of MPD decoder plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "decoder" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "decoder" x))))
(resampler
maybe-mpd-plugin
@@ -462,8 +463,8 @@ (define-configuration mpd-configuration
(filters
(list-of-mpd-plugin '())
"List of MPD filter plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "filter" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "filter" x))))
(outputs
(list-of-mpd-plugin-or-output (list (mpd-output)))
@@ -473,8 +474,8 @@ (define-configuration mpd-configuration
(playlist-plugins
(list-of-mpd-plugin '())
"List of MPD playlist plugin configurations."
- (lambda (_ x)
- (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x)))
+ (serializer (lambda (_ x)
+ (mpd-serialize-list-of-mpd-plugin "playlist_plugin" x))))
(extra-options
(alist '())
diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index d085b375a2..229220eeb1 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -213,9 +213,10 @@ (define-configuration fstrim-configuration
maybe-list-of-strings
"Extra options to append to @command{fstrim} (run @samp{man fstrim} for
more information)."
- (lambda (_ value)
- (if (maybe-value-set? value)
- value '())))
+ (serializer
+ (lambda (_ value)
+ (if (maybe-value-set? value)
+ value '()))))
(prefix fstrim-))
(define (serialize-fstrim-configuration config)
diff --git a/gnu/services/security.scm b/gnu/services/security.scm
index 8116072920..e750bb468b 100644
--- a/gnu/services/security.scm
+++ b/gnu/services/security.scm
@@ -200,7 +200,7 @@ (define-configuration fail2ban-jail-configuration
"Backend to use to detect changes in the @code{log-path}. The default is
'auto. To consult the defaults of the jail configuration, refer to the
@file{/etc/fail2ban/jail.conf} file of the @code{fail2ban} package."
- fail2ban-jail-configuration-serialize-backend)
+ (serializer fail2ban-jail-configuration-serialize-backend))
(max-retry
maybe-integer
"The number of failures before a host get banned
@@ -269,7 +269,7 @@ (define-configuration fail2ban-jail-configuration
maybe-symbol
"The encoding of the log files handled by the jail.
Possible values are: @code{'ascii}, @code{'utf-8} and @code{'auto}."
- fail2ban-jail-configuration-serialize-log-encoding)
+ (serializer fail2ban-jail-configuration-serialize-log-encoding))
(log-path
(list-of-strings '())
"The file names of the log files to be monitored.")
@@ -280,7 +280,7 @@ (define-configuration fail2ban-jail-configuration
(text-config '())
"Extra content for the jail configuration, provided as a list of file-like
objects."
- serialize-text-config)
+ (serializer serialize-text-config))
(prefix fail2ban-jail-configuration-))
(define list-of-fail2ban-jail-configurations?
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 0392cce927..8ad5907f37 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -82,6 +82,9 @@ (define (custom-number-serializer name value)
(format #f "~a = ~a;" name value))
(define-configuration serializable-configuration
+ (port (number 80) "The port number." (serializer custom-number-serializer)))
+
+(define-configuration serializable-configuration-deprecated
(port (number 80) "The port number." custom-number-serializer))
(test-assert "serialize-configuration"
@@ -89,8 +92,14 @@ (define-configuration serializable-configuration
(let ((config (serializable-configuration)))
(serialize-configuration config serializable-configuration-fields))))
+(test-assert "serialize-configuration [deprecated]"
+ (gexp?
+ (let ((config (serializable-configuration-deprecated)))
+ (serialize-configuration
+ config serializable-configuration-deprecated-fields))))
+
(define-configuration serializable-configuration
- (port (number 80) "The port number." custom-number-serializer)
+ (port (number 80) "The port number." (serializer custom-number-serializer))
(no-serialization))
(test-assert "serialize-configuration with no-serialization"
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v4 3/5] services: mpd: Fix unintentional API breakage for mixer-type field.
2023-03-26 18:41 ` [bug#62298] [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Bruno Victal
2023-03-26 18:41 ` [bug#62298] [PATCH v4 2/5] services: replace bare serializers with (serializer ...) Bruno Victal
@ 2023-03-26 18:41 ` Bruno Victal
2023-03-26 18:41 ` [bug#62298] [PATCH v4 4/5] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
` (2 subsequent siblings)
4 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-26 18:41 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (mpd-output)[mixer-type]: Use sanitizer to
accept both strings and symbols as values.
---
gnu/services/audio.scm | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index c073b85a32..bc4aed71dc 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,6 +140,11 @@ (define (uglify-field-name field-name)
(define list-of-symbol?
(list-of symbol?))
+\f
+;;;
+;;; MPD
+;;;
+
(define (mpd-serialize-field field-name value)
(let ((field (if (string? field-name) field-name
(uglify-field-name field-name)))
@@ -294,7 +299,17 @@ (define-configuration mpd-output
for this audio output: the @code{hardware} mixer, the @code{software}
mixer, the @code{null} mixer (allows setting the volume, but with no
effect; this can be used as a trick to implement an external mixer
-External Mixer) or no mixer (@code{none}).")
+External Mixer) or no mixer (@code{none})."
+ (sanitizer
+ (lambda (x) ; TODO: deprecated, remove me later.
+ (cond
+ ((symbol? x)
+ (warning (G_ "symbol value for 'mixer-type' is deprecated, \
+use string instead~%"))
+ (symbol->string x))
+ ((string? x) x)
+ (else
+ (configuration-field-error #f 'mixer-type x))))))
(replay-gain-handler
maybe-string
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v4 4/5] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-26 18:41 ` [bug#62298] [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Bruno Victal
2023-03-26 18:41 ` [bug#62298] [PATCH v4 2/5] services: replace bare serializers with (serializer ...) Bruno Victal
2023-03-26 18:41 ` [bug#62298] [PATCH v4 3/5] services: mpd: Fix unintentional API breakage for mixer-type field Bruno Victal
@ 2023-03-26 18:41 ` Bruno Victal
2023-03-26 18:41 ` [bug#62298] [PATCH v4 5/5] services: mympd: " Bruno Victal
2023-04-02 10:46 ` bug#62298: [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Liliana Marie Prikler
4 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-26 18:41 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
Deprecate using strings for these fields and prefer user-account
(resp. user-group) instead to avoid duplication within account-service-type.
Fixes #61570 <https://issues.guix.gnu.org/61570>.
* gnu/services/audio.scm (mpd-serialize-user-account, mpd-serialize-user-group)
(mpd-user-sanitizer, mpd-group-sanitizer): New procedure.
(%mpd-user, %mpd-group): New variable.
(mpd-configuration)[user, group]: Set value type to
user-account (resp. user-group).
(mpd-shepherd-service): Adapt for user-account values in user field.
(mpd-accounts): Adapt for user-account (resp. user-group) in
user (resp. group) field.
* doc/guix.texi (Audio Services): Update documentation.
---
doc/guix.texi | 4 +-
gnu/services/audio.scm | 87 +++++++++++++++++++++++++++++++++---------
2 files changed, 72 insertions(+), 19 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 8604b95f94..ff678ca6ec 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33491,10 +33491,10 @@ Audio Services
@item @code{package} (default: @code{mpd}) (type: file-like)
The MPD package.
-@item @code{user} (default: @code{"mpd"}) (type: string)
+@item @code{user} (type: user-account) (optional)
The user to run mpd as.
-@item @code{group} (default: @code{"mpd"}) (type: string)
+@item @code{group} (type: user-group) (optional)
The group to run mpd as.
@item @code{shepherd-requirement} (default: @code{()}) (type: list-of-symbol)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index bc4aed71dc..3fd309e45d 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -140,6 +140,14 @@ (define (uglify-field-name field-name)
(define list-of-symbol?
(list-of symbol?))
+;; Helpers for deprecated field types, to be removed later.
+(define %lazy-group (make-symbol "%lazy-group"))
+
+(define (inject-group-into-user user group)
+ (user-account
+ (inherit user)
+ (group (user-group-name group))))
+
\f
;;;
;;; MPD
@@ -164,10 +172,31 @@ (define mpd-serialize-boolean mpd-serialize-field)
(define (mpd-serialize-list-of-strings field-name value)
#~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
+(define (mpd-serialize-user-account field-name value)
+ (mpd-serialize-string field-name (user-account-name value)))
+
+(define (mpd-serialize-user-group field-name value)
+ (mpd-serialize-string field-name (user-group-name value)))
+
(define-maybe string (prefix mpd-))
(define-maybe list-of-strings (prefix mpd-))
(define-maybe boolean (prefix mpd-))
+(define %mpd-user
+ (user-account
+ (name "mpd")
+ (group "mpd")
+ (system? #t)
+ (comment "Music Player Daemon (MPD) user")
+ ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
+ (home-directory "/var/lib/mpd")
+ (shell (file-append shadow "/sbin/nologin"))))
+
+(define %mpd-group
+ (user-group
+ (name "mpd")
+ (system? #t)))
+
;;; TODO: Procedures for deprecated fields, to be removed.
(define mpd-deprecated-fields '((music-dir . music-directory)
@@ -197,6 +226,33 @@ (define (mpd-serialize-port field-name value)
(define-maybe port (prefix mpd-))
+;;; Procedures for unsupported value types, to be removed.
+
+(define (mpd-user-sanitizer value)
+ (cond ((user-account? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'user' is deprecated, use \
+user-account instead~%"))
+ (user-account
+ (inherit %mpd-user)
+ (name value)
+ ;; XXX: This is to be lazily substituted in (…-accounts)
+ ;; with the value from 'group'.
+ (group %lazy-group)))
+ (else
+ (configuration-field-error #f 'user value))))
+
+(define (mpd-group-sanitizer value)
+ (cond ((user-group? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'group' is deprecated, use \
+user-group instead~%"))
+ (user-group
+ (inherit %mpd-group)
+ (name value)))
+ (else
+ (configuration-field-error #f 'group value))))
+
;;;
;; Generic MPD plugin record, lists only the most prevalent fields.
@@ -347,12 +403,14 @@ (define-configuration mpd-configuration
empty-serializer)
(user
- (string "mpd")
- "The user to run mpd as.")
+ (user-account %mpd-user)
+ "The user to run mpd as."
+ (sanitizer mpd-user-sanitizer))
(group
- (string "mpd")
- "The group to run mpd as.")
+ (user-group %mpd-group)
+ "The group to run mpd as."
+ (sanitizer mpd-group-sanitizer))
(shepherd-requirement
(list-of-symbol '())
@@ -517,7 +575,8 @@ (define (mpd-shepherd-service config)
log-file playlist-directory
db-file state-file sticker-file
environment-variables)
- (let* ((config-file (mpd-serialize-configuration config)))
+ (let ((config-file (mpd-serialize-configuration config))
+ (username (user-account-name user)))
(shepherd-service
(documentation "Run the MPD (Music Player Daemon)")
(requirement `(user-processes loopback ,@shepherd-requirement))
@@ -526,7 +585,7 @@ (define (mpd-shepherd-service config)
(and=> #$(maybe-value log-file)
(compose mkdir-p dirname))
- (let ((user (getpw #$user)))
+ (let ((user (getpw #$username)))
(for-each
(lambda (x)
(when (and x (not (file-exists? x)))
@@ -560,17 +619,11 @@ (define (mpd-shepherd-service config)
(define (mpd-accounts config)
(match-record config <mpd-configuration> (user group)
- (list (user-group
- (name group)
- (system? #t))
- (user-account
- (name user)
- (group group)
- (system? #t)
- (comment "Music Player Daemon (MPD) user")
- ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data
- (home-directory "/var/lib/mpd")
- (shell (file-append shadow "/sbin/nologin"))))))
+ ;; TODO: Deprecation code, to be removed.
+ (let ((user (if (eq? (user-account-group user) %lazy-group)
+ (inject-group-into-user user group)
+ user)))
+ (list user group))))
(define mpd-service-type
(service-type
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [bug#62298] [PATCH v4 5/5] services: mympd: Use user-account (resp. user-group) for user (resp. group) fields.
2023-03-26 18:41 ` [bug#62298] [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Bruno Victal
` (2 preceding siblings ...)
2023-03-26 18:41 ` [bug#62298] [PATCH v4 4/5] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields Bruno Victal
@ 2023-03-26 18:41 ` Bruno Victal
2023-04-02 10:46 ` bug#62298: [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Liliana Marie Prikler
4 siblings, 0 replies; 47+ messages in thread
From: Bruno Victal @ 2023-03-26 18:41 UTC (permalink / raw)
To: 62298; +Cc: ludo, Bruno Victal, liliana.prikler, maxim.cournoyer
* gnu/services/audio.scm (%mympd-user, %mympd-group): New variable.
(mympd-user-sanitizer, mympd-group-sanitizer): New procedure.
(mympd-configuration)[user, group]: Set value type to
user-account (resp. user-group).
(mympd-serialize-configuration): Adapt for user-account values in user field.
(mympd-accounts): Adapt for user-account (resp. user-group) in
user (resp. group) field.
---
doc/guix.texi | 4 +--
gnu/services/audio.scm | 70 +++++++++++++++++++++++++++++++++---------
2 files changed, 58 insertions(+), 16 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index ff678ca6ec..7695e000a8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33732,10 +33732,10 @@ Audio Services
This is a list of symbols naming Shepherd services that this service
will depend on.
-@item @code{user} (default: @code{"mympd"}) (type: string)
+@item @code{user} (type: user-account) (optional)
Owner of the @command{mympd} process.
-@item @code{group} (default: @code{"nogroup"}) (type: string)
+@item @code{group} (type: user-group) (optional)
Owner group of the @command{mympd} process.
@item @code{work-directory} (default: @code{"/var/lib/mympd"}) (type: string)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 3fd309e45d..76da67944a 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -658,6 +658,48 @@ (define-configuration/no-serialization mympd-ip-acl
(define-maybe/no-serialization integer)
(define-maybe/no-serialization mympd-ip-acl)
+(define %mympd-user
+ (user-account
+ (name "mympd")
+ (group "mympd")
+ (system? #t)
+ (comment "myMPD user")
+ (home-directory "/var/empty")
+ (shell (file-append shadow "/sbin/nologin"))))
+
+(define %mympd-group
+ (user-group
+ (name "mympd")
+ (system? #t)))
+
+;;; TODO: Procedures for unsupported value types, to be removed.
+(define (mympd-user-sanitizer value)
+ (cond ((user-account? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'user' is not supported, use \
+user-account instead~%"))
+ (user-account
+ (inherit %mympd-user)
+ (name value)
+ ;; XXX: this is to be lazily substituted in (…-accounts)
+ ;; with the value from 'group'.
+ (group %lazy-group)))
+ (else
+ (configuration-field-error #f 'user value))))
+
+(define (mympd-group-sanitizer value)
+ (cond ((user-group? value) value)
+ ((string? value)
+ (warning (G_ "string value for 'group' is not supported, use \
+user-group instead~%"))
+ (user-group
+ (inherit %mympd-group)
+ (name value)))
+ (else
+ (configuration-field-error #f 'group 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
@@ -675,13 +717,15 @@ (define-configuration/no-serialization mympd-configuration
empty-serializer)
(user
- (string "mympd")
+ (user-account %mympd-user)
"Owner of the @command{mympd} process."
+ (sanitizer mympd-user-sanitizer)
empty-serializer)
(group
- (string "nogroup")
+ (user-group %mympd-group)
"Owner group of the @command{mympd} process."
+ (sanitizer mympd-group-sanitizer)
empty-serializer)
(work-directory
@@ -816,7 +860,8 @@ (define (mympd-shepherd-service config)
(match-record config <mympd-configuration> (package shepherd-requirement
user work-directory
cache-directory log-level log-to)
- (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level)))
+ (let ((log-level* (format #f "MYMPD_LOGLEVEL=~a" log-level))
+ (username (user-account-name user)))
(shepherd-service
(documentation "Run the myMPD daemon.")
(requirement `(loopback user-processes
@@ -826,7 +871,7 @@ (define (mympd-shepherd-service config)
,@shepherd-requirement))
(provision '(mympd))
(start #~(begin
- (let* ((pw (getpwnam #$user))
+ (let* ((pw (getpwnam #$username))
(uid (passwd:uid pw))
(gid (passwd:gid pw)))
(for-each (lambda (dir)
@@ -836,8 +881,8 @@ (define (mympd-shepherd-service config)
(make-forkexec-constructor
`(#$(file-append package "/bin/mympd")
- "--user" #$user
- #$@(if (eqv? log-to 'syslog) '("--syslog") '())
+ "--user" #$username
+ #$@(if (eq? log-to 'syslog) '("--syslog") '())
"--workdir" #$work-directory
"--cachedir" #$cache-directory)
#:environment-variables (list #$log-level*)
@@ -846,14 +891,11 @@ (define (mympd-shepherd-service config)
(define (mympd-accounts config)
(match-record config <mympd-configuration> (user group)
- (list (user-group (name group)
- (system? #t))
- (user-account (name user)
- (group group)
- (system? #t)
- (comment "myMPD user")
- (home-directory "/var/empty")
- (shell (file-append shadow "/sbin/nologin"))))))
+ ;; TODO: Deprecation code, to be removed.
+ (let ((user (if (eq? (user-account-group user) %lazy-group)
+ (inject-group-into-user user group)
+ user)))
+ (list user group))))
(define (mympd-log-rotation config)
(match-record config <mympd-configuration> (log-to)
--
2.39.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* bug#62298: [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support.
2023-03-26 18:41 ` [bug#62298] [PATCH v4 1/5] services: configuration: Add user-defined sanitizer support Bruno Victal
` (3 preceding siblings ...)
2023-03-26 18:41 ` [bug#62298] [PATCH v4 5/5] services: mympd: " Bruno Victal
@ 2023-04-02 10:46 ` Liliana Marie Prikler
4 siblings, 0 replies; 47+ messages in thread
From: Liliana Marie Prikler @ 2023-04-02 10:46 UTC (permalink / raw)
To: Bruno Victal, 62298-done; +Cc: ludo, maxim.cournoyer
Am Sonntag, dem 26.03.2023 um 19:41 +0100 schrieb Bruno Victal:
> This changes the 'custom-serializer' field into a generic
> 'extra-args' field that can be extended to support new literals.
> With this mechanism, the literals 'sanitizer' allow for user-defined
> sanitizer procedures while the 'serializer' literal is used for
> custom serializer procedures. The 'empty-serializer' was also added
> as a 'literal' and can be used just like it was previously.
> ...
I pushed v4 with changes to the ChangeLog as necessary and the
following:
- mpd-user and mympd-user belong to %lazy-group.
- inject-user-into-group is %set-user-group.
- %mpd-user and %mpd-group are properly documented,
but remain unexported.
Same for %mympd-user and %mympd-group.
Thanks for your hard work and cheers
Liliana
^ permalink raw reply [flat|nested] 47+ messages in thread