unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
@ 2023-06-09 21:18 Bruno Victal
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
                   ` (11 more replies)
  0 siblings, 12 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-09 21:18 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal, ludo, maxim.cournoyer, liliana.prikler

This RFC patch series (more like 3 patch series for the price of 1)
implements the following:

* Serializer Keyword arguments parameter in define-configuration.
  Allows for serializing procedures to be specified as:
    (define* (serialize-string field-name value #:key …)
      …)
  Examples in the unit tests.

* Express serialize-configuration with SRFI-171 transducers.
  The ‘base-transducer’ can be used to craft more specialized
  configuration record serializers.

* Generic INI serializer.
  This procedure can be used for crafting INI files from a
  record-type defined with define-configuration.
  Example for generic-ini in action can be found in the unit test.


These changes are motivated in part by a (in progress) refactoring of
the NetworkManager service-type.

Notes:
* I've left Generic-INI and serializer-kwargs undocumented since this
  is at its RFC stage.
* The (gnu services configuration generic-ini) module contains some trailing
  notes that should be removed before merging in.
* jami-account->alist explicitly checks for the empty-string.
  (which I consider to be a serialization “artifact” arising from define-maybe)
  This is the only (relevant) test that failed and will need to be investigated.

Bruno Victal (5):
  services: configuration: Simplify normalize-extra-args.
  services: configuration: Use transducers within
    serialize-configuration.
  services: fstrim-service-type: Serialize with SRFI-171 transducers.
  services: configuration: Add serializer-kwargs field.
  services: configuration: New generic-ini module.

 Makefile.am                                  |   1 +
 gnu/local.mk                                 |   1 +
 gnu/services/configuration.scm               |  86 +++++++++----
 gnu/services/configuration/generic-ini.scm   | 129 +++++++++++++++++++
 gnu/services/linux.scm                       |  11 +-
 tests/services/configuration.scm             |  38 +++++-
 tests/services/configuration/generic-ini.scm | 106 +++++++++++++++
 7 files changed, 333 insertions(+), 39 deletions(-)
 create mode 100644 gnu/services/configuration/generic-ini.scm
 create mode 100644 tests/services/configuration/generic-ini.scm


base-commit: c348b1be3891e6eb47bbdd9fc1587aba2b6ab0b7
-- 
2.39.2





^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 1/5] services: configuration: Simplify normalize-extra-args.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
@ 2023-06-09 21:20 ` Bruno Victal
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-09 21:20 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/configuration.scm
(define-configuration-helper, normalize-extra-args): Use #f instead of %unset-value.
---
 gnu/services/configuration.scm | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 367b85c1be..dafe72f4fe 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -190,32 +190,32 @@ (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))
+               (sanitizer* #f)
+               (serializer* #f))
       (syntax-case s (sanitizer serializer empty-serializer)
         (((sanitizer proc) tail ...)
-         (if (maybe-value-set? sanitizer*)
-             (syntax-violation 'sanitizer "duplicate entry"
-                               #'proc)
+         (if 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)
+         (if serializer*
+             (syntax-violation 'serializer
+                               "duplicate or conflicting entry" #'proc)
              (loop #'(tail ...) sanitizer* #'proc)))
         ((empty-serializer tail ...)
-         (if (maybe-value-set? serializer*)
+         (if 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*)))
+         (every not (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)))))))
+           (values (list #f #'proc)))))))
 
   (syntax-case syn ()
     ((_ stem (field field-type+def doc extra-args ...) ...)
@@ -239,11 +239,11 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
                      default-value))
                   #'((field-type def) ...)))
             ((field-sanitizer ...)
-             (map maybe-value #'(sanitizer* ...)))
+             #'(sanitizer* ...))
             ((field-serializer ...)
              (map (lambda (type proc)
                     (and serialize?
-                         (or (maybe-value proc)
+                         (or proc
                              (if serializer-prefix
                                  (id #'stem serializer-prefix #'serialize- type)
                                  (id #'stem #'serialize- type)))))

base-commit: c348b1be3891e6eb47bbdd9fc1587aba2b6ab0b7
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 2/5] services: configuration: Use transducers within serialize-configuration.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
@ 2023-06-09 21:20 ` Bruno Victal
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-09 21:20 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Introduces 'base-transducer', a SRFI-171 based transducer that can be used as a
starting point for writing custom configuration record serializing procedures.

This also fixes the symbol maybe-value serialization test case.

* gnu/services/configuration.scm (empty-serializer?): New predicate.
(base-transducer): New procedure.
(serialize-configuration): Adapt to use base-transducer.
* tests/services/configuration.scm: Remove test-expect-fail.
---
 gnu/services/configuration.scm   | 24 +++++++++++++++++++-----
 tests/services/configuration.scm |  4 ----
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index dafe72f4fe..0818b48bb5 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -42,6 +42,7 @@ (define-module (gnu services configuration)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-171)
   #:export (configuration-field
             configuration-field-name
             configuration-field-type
@@ -59,6 +60,7 @@ (define-module (gnu services configuration)
             define-configuration/no-serialization
             no-serialization
 
+            base-transducer
             serialize-configuration
             define-maybe
             define-maybe/no-serialization
@@ -125,13 +127,25 @@ (define-record-type* <configuration-field>
   (default-value-thunk configuration-field-default-value-thunk)
   (documentation configuration-field-documentation))
 
+(define (empty-serializer? field)
+  (eq? empty-serializer
+       (configuration-field-serializer field)))
+
+(define (base-transducer config)
+  (compose (tremove empty-serializer?)
+           ;; Only serialize fields whose value isn't '%unset-marker%.
+           (tfilter (lambda (field)
+                      (let ((field-value ((configuration-field-getter field)
+                                          config)))
+                        (maybe-value-set? field-value))))
+           (tmap (lambda (field)
+                   ((configuration-field-serializer field)
+                    (configuration-field-name field)
+                    ((configuration-field-getter field) config))))))
+
 (define (serialize-configuration config fields)
   #~(string-append
-     #$@(map (lambda (field)
-               ((configuration-field-serializer field)
-                (configuration-field-name field)
-                ((configuration-field-getter field) config)))
-             fields)))
+     #$@(list-transduce (base-transducer config) rcons fields)))
 
 (define-syntax-rule (id ctx parts ...)
   "Assemble PARTS into a raw (unhygienic) identifier."
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 8ad5907f37..1db83bb273 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -337,10 +337,6 @@ (define-maybe symbol)
 (define-configuration config-with-maybe-symbol
   (protocol maybe-symbol ""))
 
-;;; Maybe symbol values are currently seen as serializable, because the
-;;; unspecified value is '%unset-marker%, which is a symbol itself.
-;;; TODO: Remove expected fail marker after resolution.
-(test-expect-fail 1)
 (test-equal "symbol maybe value serialization, unspecified"
   ""
   (gexp->approximate-sexp
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
@ 2023-06-09 21:20 ` Bruno Victal
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 4/5] services: configuration: Add serializer-kwargs field Bruno Victal
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-09 21:20 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/linux.scm (serialize-fstrim-configuration): Refactor to use
base-transducer.
---
 gnu/services/linux.scm | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index d105c42850..3cfa6d6855 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -40,6 +40,7 @@ (define-module (gnu services linux)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-171)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (earlyoom-configuration
@@ -227,13 +228,9 @@ (define-configuration fstrim-configuration
   (prefix fstrim-))
 
 (define (serialize-fstrim-configuration config)
-  (concatenate
-   (filter list?
-           (map (lambda (field)
-                  ((configuration-field-serializer field)
-                   (configuration-field-name field)
-                   ((configuration-field-getter field) config)))
-                fstrim-configuration-fields))))
+  (list-transduce (compose (base-transducer config) tconcatenate)
+                  rcons
+                  fstrim-configuration-fields))
 
 (define (fstrim-mcron-job config)
   (match-record config <fstrim-configuration> (package schedule)
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 4/5] services: configuration: Add serializer-kwargs field.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (2 preceding siblings ...)
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
@ 2023-06-09 21:20 ` Bruno Victal
  2023-06-09 21:21 ` [bug#63985] [PATCH RFC 5/5] services: configuration: New generic-ini module Bruno Victal
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-09 21:20 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Allow relaying additional arguments to a serialize-<type> procedure.

* gnu/services/configuration.scm (configuration-field)
<serializer-kwargs>: New field.
(base-transducer, define-maybe-helper): Adjust to relay additional arguments.
(normalize-extra-args): Implement serializer-kwargs literal.

* tests/services/configuration.scm: Add tests for serializer-kwargs.
---
 gnu/services/configuration.scm   | 48 +++++++++++++++++++++-----------
 tests/services/configuration.scm | 34 ++++++++++++++++++++++
 2 files changed, 66 insertions(+), 16 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 0818b48bb5..b32a5a85ba 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -124,6 +124,7 @@ (define-record-type* <configuration-field>
   (predicate configuration-field-predicate)
   (sanitizer configuration-field-sanitizer)
   (serializer configuration-field-serializer)
+  (serializer-kwargs configuration-field-serializer-kwargs)
   (default-value-thunk configuration-field-default-value-thunk)
   (documentation configuration-field-documentation))
 
@@ -139,9 +140,13 @@ (define (base-transducer config)
                                           config)))
                         (maybe-value-set? field-value))))
            (tmap (lambda (field)
-                   ((configuration-field-serializer field)
-                    (configuration-field-name field)
-                    ((configuration-field-getter field) config))))))
+                   (let ((serializer (configuration-field-serializer field))
+                         (field-name (configuration-field-name field))
+                         (value
+                          ((configuration-field-getter field) config))
+                         (serializer-kwargs
+                          (configuration-field-serializer-kwargs field)))
+                     (apply serializer field-name value serializer-kwargs))))))
 
 (define (serialize-configuration config fields)
   #~(string-append
@@ -168,10 +173,9 @@ (define (define-maybe-helper serialize? prefix syn)
              (or (not (maybe-value-set? val))
                  (stem? val)))
            #,@(if serialize?
-                  (list #'(define (serialize-maybe-stem field-name val)
-                            (if (stem? val)
-                                (serialize-stem field-name val)
-                                "")))
+                  (list #'(define (serialize-maybe-stem field-name val . rest)
+                            (when (maybe-value-set? val)
+                              (apply serialize-stem field-name val rest))))
                   '()))))))
 
 (define-syntax define-maybe
@@ -205,38 +209,49 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
     "Extract and normalize arguments following @var{doc}."
     (let loop ((s s)
                (sanitizer* #f)
-               (serializer* #f))
-      (syntax-case s (sanitizer serializer empty-serializer)
+               (serializer* #f)
+               (serializer-kwargs* #f))
+      (syntax-case s (sanitizer serializer empty-serializer serializer-kwargs)
         (((sanitizer proc) tail ...)
          (if sanitizer*
              (syntax-violation 'sanitizer
                                "duplicate entry" #'proc)
-             (loop #'(tail ...) #'proc serializer*)))
+             (loop #'(tail ...)
+                   #'proc serializer* serializer-kwargs*)))
         (((serializer proc) tail ...)
          (if serializer*
              (syntax-violation 'serializer
                                "duplicate or conflicting entry" #'proc)
-             (loop #'(tail ...) sanitizer* #'proc)))
+             (loop #'(tail ...)
+                   sanitizer* #'proc serializer-kwargs*)))
         ((empty-serializer tail ...)
          (if serializer*
              (syntax-violation 'empty-serializer
                                "duplicate or conflicting entry" #f)
-             (loop #'(tail ...) sanitizer* #'empty-serializer)))
+             (loop #'(tail ...)
+                   sanitizer* #'empty-serializer #f)))
+        (((serializer-kwargs args) tail ...)
+         (if serializer-kwargs*
+             (syntax-violation 'serializer-kwargs
+                               "duplicate or conflicting entry" #f)
+             (loop #'(tail ...)
+                   sanitizer* serializer* #'args)))
         (()  ; stop condition
-         (values (list sanitizer* serializer*)))
+         (values (list sanitizer* serializer*
+                       (or serializer-kwargs* #'(quote ())))))
         ((proc)  ; TODO: deprecated, to be removed.
-         (every not (list sanitizer* serializer*))
+         (every not (list sanitizer* serializer* serializer-kwargs*))
          (begin
            (warning #f (G_ "specifying serializers after documentation is \
 deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
-           (values (list #f #'proc)))))))
+           (values (list #f #'proc #'(quote ()))))))))
 
   (syntax-case syn ()
     ((_ stem (field field-type+def doc extra-args ...) ...)
      (with-syntax
          ((((field-type def) ...)
            (map normalize-field-type+def #'(field-type+def ...)))
-          (((sanitizer* serializer*) ...)
+          (((sanitizer* serializer* serializer-kwargs*) ...)
            (map normalize-extra-args #'((extra-args ...) ...))))
        (with-syntax
            (((field-getter ...)
@@ -322,6 +337,7 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
                        (or field-sanitizer
                            (id #'stem #'validate- #'stem #'- #'field)))
                       (serializer field-serializer)
+                      (serializer-kwargs serializer-kwargs*)
                       (default-value-thunk
                         (lambda ()
                           (if (maybe-value-set? (syntax->datum field-default))
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 1db83bb273..ae1b194e81 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -297,6 +297,40 @@ (define (sanitize-port value)
                     (lambda _ "lorem")
                     (sanitizer (lambda () #t)))))))
 
+(test-group "Serializer keyword arguments"
+  (define* (serialize-port field value #:key host)
+    (format #f "host=~a,port=~d" host value))
+
+  (define-configuration kwarg-config
+    (port
+     (port 80)
+     "Lorem Ipsum."
+     (serializer-kwargs '(#:host "[2001:db8::1]"))))
+
+  (test-equal "keyword argument provided"
+    "host=[2001:db8::1],port=80"
+    (eval-gexp
+     (serialize-configuration (kwarg-config)
+                              kwarg-config-fields))))
+
+(test-group "Serializer keyword arguments, define-maybe"
+  (define* (serialize-port field value #:key host)
+    (format #f "host=~a,port=~d" host value))
+
+  (define-maybe port)
+
+  (define-configuration kwarg-config
+    (port
+     (maybe-port 80)
+     "Lorem Ipsum."
+     (serializer-kwargs '(#:host "[2001:db8::1]"))))
+
+  (test-equal "keyword argument provided, maybe type"
+    "host=[2001:db8::1],port=80"
+    (eval-gexp
+     (serialize-configuration (kwarg-config)
+                              kwarg-config-fields))))
+
 \f
 ;;;
 ;;; define-maybe macro.
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 5/5] services: configuration: New generic-ini module.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (3 preceding siblings ...)
  2023-06-09 21:20 ` [bug#63985] [PATCH RFC 4/5] services: configuration: Add serializer-kwargs field Bruno Victal
@ 2023-06-09 21:21 ` Bruno Victal
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-09 21:21 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Implements a ‘serialize-ini-configuration’ procedure for serializing
record-types defined with define-configuration into generic INI files.

* gnu/services/configuration/generic-ini.scm: New module.
* tests/services/configuration/generic-ini.scm: Add tests for new module.
* Makefile.am: Register tests.
* gnu/local.mk: Register module.
---
 Makefile.am                                  |   1 +
 gnu/local.mk                                 |   1 +
 gnu/services/configuration/generic-ini.scm   | 129 +++++++++++++++++++
 tests/services/configuration/generic-ini.scm | 106 +++++++++++++++
 4 files changed, 237 insertions(+)
 create mode 100644 gnu/services/configuration/generic-ini.scm
 create mode 100644 tests/services/configuration/generic-ini.scm

diff --git a/Makefile.am b/Makefile.am
index ab901df757..8dc9a3438b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -552,6 +552,7 @@ SCM_TESTS =					\
   tests/services.scm				\
   tests/services/file-sharing.scm		\
   tests/services/configuration.scm		\
+  tests/services/configuration/generic-ini.scm	\
   tests/services/lightdm.scm			\
   tests/services/linux.scm			\
   tests/services/telephony.scm			\
diff --git a/gnu/local.mk b/gnu/local.mk
index ce16d37e2b..a05d70aff3 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -670,6 +670,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/cgit.scm			\
   %D%/services/ci.scm				\
   %D%/services/configuration.scm		\
+  %D%/services/configuration/generic-ini.scm	\
   %D%/services/cuirass.scm			\
   %D%/services/cups.scm				\
   %D%/services/databases.scm			\
diff --git a/gnu/services/configuration/generic-ini.scm b/gnu/services/configuration/generic-ini.scm
new file mode 100644
index 0000000000..338a35f90d
--- /dev/null
+++ b/gnu/services/configuration/generic-ini.scm
@@ -0,0 +1,129 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services configuration generic-ini)
+  #:use-module (gnu services configuration)
+  #:use-module (guix gexp)
+  #:use-module (srfi srfi-171)
+  #:use-module (ice-9 match)
+  #:export (ini-entry?
+            list-of-ini-entries?
+
+            serialize-ini-configuration))
+
+;;;
+;;; Generic INI serializer
+;;;
+
+\f
+;;;
+;;; Predicates
+;;;
+
+;; This is the same format used in SRFI-233 but without comment support.
+(define ini-entry?
+  (match-lambda
+    (((? symbol) (? symbol) (? string)) #t)
+    (_ #f)))
+
+(define list-of-ini-entries
+  (list-of ini-entry?))
+
+;;
+;; Overall design document
+;;
+;; This module implements a generic INI serializer for a record-type defined
+;; using define-configuration.
+;; It expects that the serialize-<type> procedures return a list with
+;; three elements of the form:
+;;    (list section key value)
+;; Where ‘section’ and ‘key’ are symbols and ‘value’ is a string.
+;; The fields within define-configuration do not have to be ordered in,
+;; any way whatsoever as the ‘serialize-ini’ will group them up automatically.
+;; This implies that no assumptions should be made regarding the order of the
+;; values in the serializied INI output.
+;;
+;; Additional notes:
+;; Q: Why not replace rcons with string-append and forego the ungexp-splice?
+;; A: The transduction happens outside of the G-Exp while the final string-append
+;;    takes place in the G-Exp.
+;;
+;; Debugging tips: Open a REPL and try one transducer at a time from
+;; ‘ini-transducer’.
+;;
+
+(define (add-section-header partition)
+  (let ((header (caar partition)))
+    (cons (list header)
+          partition)))
+
+(define serializer
+  (match-lambda
+    ((section)
+     #~(format #f "[~a]~%" '#$section))
+    ((section key value)
+     #~(format #f "~a=~a~%" '#$key #$value))
+    ((? string? s)  ; used for the newline between sections
+     s)))
+
+(define ini-transducer
+  (compose (tpartition car)
+           (tmap add-section-header)
+           (tadd-between '("\n"))
+           tconcatenate
+           (tmap serializer)))
+
+;; A “first-pass” serialization is performed and sorted in order
+;; to group up the fields by “section” before passing through the
+;; transducer.
+(define (serialize-ini-configuration config fields)
+  (let* ((srfi-233-IR
+          ;; First pass: “serialize” into a (disordered) list of
+          ;; SRFI-233 entries.
+          (list-transduce (base-transducer config) rcons fields))
+         (comparator (lambda (x y)
+                       ;; Sort the SRFI-233 entries by section.
+                       (string<=? (symbol->string (car x))
+                                  (symbol->string (car y)))))
+         (sorted-entries (sort srfi-233-IR comparator)))
+    #~(string-append
+       #$@(list-transduce ini-transducer rcons sorted-entries))))
+
+;; FIXME:RFC:
+;;       generic-ini- prefixed serializing procs?
+;;       (perhaps prefixed as generic-ini: ?)
+;; Example procedures:
+;;
+(define* (generic-ini-serialize-string field-name value #:key section)
+  (list section field-name value))
+
+;; field-name-transform can be used to “uglify” a field-name,
+;; e.g. want-ipv6?  ->  want_ipv6
+(define* (generic-ini-serialize-boolean field-name value #:key section
+                            (field-name-transform identity))
+  (list section (field-name-transform field-name)
+        (if value "true" "false")))
+
+
+;;; FIXME: delete this before inclusion, these are notes for the first RFC.
+;;;
+;;; Left out for now (but readily extendable):
+;;;  * Custom leading (presumed to be whitespace) characters for entries
+;;;    à la gitconfig, mostly pretty-printing purposes
+;;;  * Configurable delimiter (\n, \r\n, \0, ...)
+;;;  * Configurable Key-value separator (this is usually =)
diff --git a/tests/services/configuration/generic-ini.scm b/tests/services/configuration/generic-ini.scm
new file mode 100644
index 0000000000..c04acdcd46
--- /dev/null
+++ b/tests/services/configuration/generic-ini.scm
@@ -0,0 +1,106 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (tests services configuration generic-ini)
+  #:use-module (gnu services configuration)
+  #:use-module (gnu services configuration generic-ini)
+  #:use-module (guix diagnostics)
+  #:use-module (guix gexp)
+  #:use-module (guix store)
+  #:autoload (guix i18n) (G_)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-64)
+  #:use-module (srfi srfi-71))
+
+;;; Tests for the (gnu services configuration generic-ini) module.
+
+(test-begin "generic-ini serializer")
+
+
+(define expected-output "\
+[featured]
+favourite=true
+track=Deep affection
+artist=Magistina Saga
+album=Creaith Anthem
+
+[main]
+enabled=true
+")
+
+\f
+;;;
+;;; Serializers
+;;;
+(define (strip-trailing-?-character field-name)
+  "Drop rightmost '?' character"
+  (let ((str (symbol->string field-name)))
+    (if (string-suffix? "?" str)
+        (string->symbol (string-drop-right str 1))
+        field-name)))
+
+(define* (serialize-string field-name value #:key section)
+  (list section field-name value))
+
+(define* (serialize-boolean field-name value #:key section)
+  (list section (strip-trailing-?-character field-name)
+        (if value "true" "false")))
+
+\f
+;;;
+;;; Record-type definition
+;;;
+
+(define-configuration foo-configuration
+  (album
+   (string "Creaith Anthem")
+   "Lorem Ipsum …"
+   (serializer-kwargs '(#:section featured)))
+
+  (artist
+   (string "Magistina Saga")
+   "Lorem Ipsum …"
+   (serializer-kwargs '(#:section featured)))
+
+  (track
+   (string "Deep affection")
+   "Lorem Ipsum …"
+   (serializer-kwargs '(#:section featured)))
+
+  (favourite?
+   (boolean #t)
+   "Lorem Ipsum …"
+   (serializer-kwargs '(#:section featured)))
+
+  (enabled?
+   (boolean #t)
+   "Lorem Ipsum …"
+   (serializer-kwargs '(#:section main))))
+
+(test-equal "Well-formed INI output from serialize-ini"
+  expected-output
+  ;; Serialize the above into a string, properly resolving any potential
+  ;; nested G-Exps as well.
+  (let* ((serialized-ini
+          (serialize-ini-configuration (foo-configuration)
+                                       foo-configuration-fields))
+         (lowered conn (with-store store
+                         ((lower-gexp serialized-ini) store))))
+    (eval (lowered-gexp-sexp lowered) (current-module))))
+
+(test-end)
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC v2 1/5] services: configuration: Simplify normalize-extra-args.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (4 preceding siblings ...)
  2023-06-09 21:21 ` [bug#63985] [PATCH RFC 5/5] services: configuration: New generic-ini module Bruno Victal
@ 2023-06-10 20:10 ` Bruno Victal
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-10 20:10 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/configuration.scm
(define-configuration-helper, normalize-extra-args): Use #f instead of %unset-value.
---

Notable changes since RFC v1:

* Jami service now passes tests.
* Typo fixes.
* serializer-kwargs renamed to serializer-options.
* Changed INI example to something more innocuous.

 gnu/services/configuration.scm | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 367b85c1be..dafe72f4fe 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -190,32 +190,32 @@ (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))
+               (sanitizer* #f)
+               (serializer* #f))
       (syntax-case s (sanitizer serializer empty-serializer)
         (((sanitizer proc) tail ...)
-         (if (maybe-value-set? sanitizer*)
-             (syntax-violation 'sanitizer "duplicate entry"
-                               #'proc)
+         (if 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)
+         (if serializer*
+             (syntax-violation 'serializer
+                               "duplicate or conflicting entry" #'proc)
              (loop #'(tail ...) sanitizer* #'proc)))
         ((empty-serializer tail ...)
-         (if (maybe-value-set? serializer*)
+         (if 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*)))
+         (every not (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)))))))
+           (values (list #f #'proc)))))))
 
   (syntax-case syn ()
     ((_ stem (field field-type+def doc extra-args ...) ...)
@@ -239,11 +239,11 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
                      default-value))
                   #'((field-type def) ...)))
             ((field-sanitizer ...)
-             (map maybe-value #'(sanitizer* ...)))
+             #'(sanitizer* ...))
             ((field-serializer ...)
              (map (lambda (type proc)
                     (and serialize?
-                         (or (maybe-value proc)
+                         (or proc
                              (if serializer-prefix
                                  (id #'stem serializer-prefix #'serialize- type)
                                  (id #'stem #'serialize- type)))))

base-commit: 767edbb6fe781d19c19971f2ccd3b0da8fd053fc
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC v2 2/5] services: configuration: Use transducers within serialize-configuration.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (5 preceding siblings ...)
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
@ 2023-06-10 20:10 ` Bruno Victal
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-10 20:10 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Introduces 'base-transducer', a SRFI-171 based transducer that can be used as a
starting point for writing custom configuration record serializing procedures.

This also fixes the symbol maybe-value serialization test case.

* gnu/services/configuration.scm (empty-serializer?): New predicate.
(base-transducer, tfilter-maybe-value): New procedure.
(serialize-configuration): Adapt to use base-transducer.

* gnu/services/telephony.scm (jami-account->alist): Use transducers to skip
fields that are unserializable or whose field maybe-value is unset.

* tests/services/configuration.scm: Remove test-expect-fail.
---
 gnu/services/configuration.scm   | 29 ++++++++++++++++++++++++-----
 gnu/services/telephony.scm       | 27 +++++++++++++--------------
 tests/services/configuration.scm |  4 ----
 3 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index dafe72f4fe..cd2cb8318b 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -42,6 +42,7 @@ (define-module (gnu services configuration)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-171)
   #:export (configuration-field
             configuration-field-name
             configuration-field-type
@@ -59,6 +60,10 @@ (define-module (gnu services configuration)
             define-configuration/no-serialization
             no-serialization
 
+            empty-serializer?
+            tfilter-maybe-value
+            base-transducer
+
             serialize-configuration
             define-maybe
             define-maybe/no-serialization
@@ -125,13 +130,27 @@ (define-record-type* <configuration-field>
   (default-value-thunk configuration-field-default-value-thunk)
   (documentation configuration-field-documentation))
 
+(define (empty-serializer? field)
+  (eq? empty-serializer
+       (configuration-field-serializer field)))
+
+(define (tfilter-maybe-value config)
+  (tfilter (lambda (field)
+             (let ((field-value ((configuration-field-getter field) config)))
+               (maybe-value-set? field-value)))))
+
+(define (base-transducer config)
+  (compose (tremove empty-serializer?)
+           ;; Only serialize fields whose value isn't '%unset-marker%.
+           (tfilter-maybe-value config)
+           (tmap (lambda (field)
+                   ((configuration-field-serializer field)
+                    (configuration-field-name field)
+                    ((configuration-field-getter field) config))))))
+
 (define (serialize-configuration config fields)
   #~(string-append
-     #$@(map (lambda (field)
-               ((configuration-field-serializer field)
-                (configuration-field-name field)
-                ((configuration-field-getter field) config)))
-             fields)))
+     #$@(list-transduce (base-transducer config) rcons fields)))
 
 (define-syntax-rule (id ctx parts ...)
   "Assemble PARTS into a raw (unhygienic) identifier."
diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
index 23ccb8d403..56b7772f58 100644
--- a/gnu/services/telephony.scm
+++ b/gnu/services/telephony.scm
@@ -37,6 +37,7 @@ (define-module (gnu services telephony)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-2)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-171)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (jami-account
@@ -204,22 +205,20 @@ (define (jami-account->alist jami-account-object)
       ('rendezvous-point? "Account.rendezVous")
       ('peer-discovery? "Account.peerDiscovery")
       ('bootstrap-hostnames "Account.hostname")
-      ('name-server-uri "RingNS.uri")
-      (_ #f)))
+      ('name-server-uri "RingNS.uri")))
 
-  (filter-map (lambda (field)
-                (and-let* ((name (field-name->account-detail
+  (define jami-account-transducer
+    (compose (tremove empty-serializer?)
+             (tfilter-maybe-value jami-account-object)
+             (tmap (lambda (field)
+                     (let* ((name (field-name->account-detail
                                   (configuration-field-name field)))
-                           (value ((configuration-field-serializer field)
-                                   name ((configuration-field-getter field)
-                                         jami-account-object)))
-                           ;; The define-maybe default serializer produces an
-                           ;; empty string for unspecified values.
-                           (value* (if (string-null? value)
-                                       #f
-                                       value)))
-                  (cons name value*)))
-              jami-account-fields))
+                            (value ((configuration-field-serializer field)
+                                    name ((configuration-field-getter field)
+                                          jami-account-object))))
+                       (cons name value))))))
+
+  (list-transduce jami-account-transducer rcons jami-account-fields))
 
 (define (jami-account-list? val)
   (and (list? val)
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 8ad5907f37..1db83bb273 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -337,10 +337,6 @@ (define-maybe symbol)
 (define-configuration config-with-maybe-symbol
   (protocol maybe-symbol ""))
 
-;;; Maybe symbol values are currently seen as serializable, because the
-;;; unspecified value is '%unset-marker%, which is a symbol itself.
-;;; TODO: Remove expected fail marker after resolution.
-(test-expect-fail 1)
 (test-equal "symbol maybe value serialization, unspecified"
   ""
   (gexp->approximate-sexp
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC v2 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (6 preceding siblings ...)
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
@ 2023-06-10 20:10 ` Bruno Victal
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 4/5] services: configuration: Add serializer-options field Bruno Victal
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-10 20:10 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/linux.scm (serialize-fstrim-configuration): Refactor to use
base-transducer.
---
 gnu/services/linux.scm | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index d105c42850..3cfa6d6855 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -40,6 +40,7 @@ (define-module (gnu services linux)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-171)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (earlyoom-configuration
@@ -227,13 +228,9 @@ (define-configuration fstrim-configuration
   (prefix fstrim-))
 
 (define (serialize-fstrim-configuration config)
-  (concatenate
-   (filter list?
-           (map (lambda (field)
-                  ((configuration-field-serializer field)
-                   (configuration-field-name field)
-                   ((configuration-field-getter field) config)))
-                fstrim-configuration-fields))))
+  (list-transduce (compose (base-transducer config) tconcatenate)
+                  rcons
+                  fstrim-configuration-fields))
 
 (define (fstrim-mcron-job config)
   (match-record config <fstrim-configuration> (package schedule)
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC v2 4/5] services: configuration: Add serializer-options field.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (7 preceding siblings ...)
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
@ 2023-06-10 20:10 ` Bruno Victal
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 5/5] services: configuration: New generic-ini module Bruno Victal
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-10 20:10 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Allow relaying additional arguments to a serialize-<type> procedure.

* gnu/services/configuration.scm (configuration-field)
<serializer-options>: New field.
(base-transducer, define-maybe-helper): Adjust to relay additional arguments.
(normalize-extra-args): Implement serializer-options literal.

* tests/services/configuration.scm: Add tests for serializer-options.
---
 gnu/services/configuration.scm   | 49 +++++++++++++++++++++-----------
 tests/services/configuration.scm | 34 ++++++++++++++++++++++
 2 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index cd2cb8318b..4eee5a26c2 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -50,6 +50,7 @@ (define-module (gnu services configuration)
             configuration-field-error
             configuration-field-sanitizer
             configuration-field-serializer
+            configuration-field-serializer-options
             configuration-field-getter
             configuration-field-default-value-thunk
             configuration-field-documentation
@@ -127,6 +128,7 @@ (define-record-type* <configuration-field>
   (predicate configuration-field-predicate)
   (sanitizer configuration-field-sanitizer)
   (serializer configuration-field-serializer)
+  (serializer-options configuration-field-serializer-options)
   (default-value-thunk configuration-field-default-value-thunk)
   (documentation configuration-field-documentation))
 
@@ -144,9 +146,13 @@ (define (base-transducer config)
            ;; Only serialize fields whose value isn't '%unset-marker%.
            (tfilter-maybe-value config)
            (tmap (lambda (field)
-                   ((configuration-field-serializer field)
-                    (configuration-field-name field)
-                    ((configuration-field-getter field) config))))))
+                   (let ((serializer (configuration-field-serializer field))
+                         (field-name (configuration-field-name field))
+                         (value
+                          ((configuration-field-getter field) config))
+                         (serializer-options
+                          (configuration-field-serializer-options field)))
+                     (apply serializer field-name value serializer-options))))))
 
 (define (serialize-configuration config fields)
   #~(string-append
@@ -173,10 +179,9 @@ (define (define-maybe-helper serialize? prefix syn)
              (or (not (maybe-value-set? val))
                  (stem? val)))
            #,@(if serialize?
-                  (list #'(define (serialize-maybe-stem field-name val)
-                            (if (stem? val)
-                                (serialize-stem field-name val)
-                                "")))
+                  (list #'(define (serialize-maybe-stem field-name val . rest)
+                            (when (maybe-value-set? val)
+                              (apply serialize-stem field-name val rest))))
                   '()))))))
 
 (define-syntax define-maybe
@@ -210,38 +215,49 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
     "Extract and normalize arguments following @var{doc}."
     (let loop ((s s)
                (sanitizer* #f)
-               (serializer* #f))
-      (syntax-case s (sanitizer serializer empty-serializer)
+               (serializer* #f)
+               (serializer-options* #f))
+      (syntax-case s (sanitizer serializer empty-serializer serializer-options)
         (((sanitizer proc) tail ...)
          (if sanitizer*
              (syntax-violation 'sanitizer
                                "duplicate entry" #'proc)
-             (loop #'(tail ...) #'proc serializer*)))
+             (loop #'(tail ...)
+                   #'proc serializer* serializer-options*)))
         (((serializer proc) tail ...)
          (if serializer*
              (syntax-violation 'serializer
                                "duplicate or conflicting entry" #'proc)
-             (loop #'(tail ...) sanitizer* #'proc)))
+             (loop #'(tail ...)
+                   sanitizer* #'proc serializer-options*)))
         ((empty-serializer tail ...)
          (if serializer*
              (syntax-violation 'empty-serializer
                                "duplicate or conflicting entry" #f)
-             (loop #'(tail ...) sanitizer* #'empty-serializer)))
+             (loop #'(tail ...)
+                   sanitizer* #'empty-serializer #f)))
+        (((serializer-options args) tail ...)
+         (if serializer-options*
+             (syntax-violation 'serializer-options
+                               "duplicate or conflicting entry" #f)
+             (loop #'(tail ...)
+                   sanitizer* serializer* #'args)))
         (()  ; stop condition
-         (values (list sanitizer* serializer*)))
+         (values (list sanitizer* serializer*
+                       (or serializer-options* #'(quote ())))))
         ((proc)  ; TODO: deprecated, to be removed.
-         (every not (list sanitizer* serializer*))
+         (every not (list sanitizer* serializer* serializer-options*))
          (begin
            (warning #f (G_ "specifying serializers after documentation is \
 deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
-           (values (list #f #'proc)))))))
+           (values (list #f #'proc #'(quote ()))))))))
 
   (syntax-case syn ()
     ((_ stem (field field-type+def doc extra-args ...) ...)
      (with-syntax
          ((((field-type def) ...)
            (map normalize-field-type+def #'(field-type+def ...)))
-          (((sanitizer* serializer*) ...)
+          (((sanitizer* serializer* serializer-options*) ...)
            (map normalize-extra-args #'((extra-args ...) ...))))
        (with-syntax
            (((field-getter ...)
@@ -327,6 +343,7 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
                        (or field-sanitizer
                            (id #'stem #'validate- #'stem #'- #'field)))
                       (serializer field-serializer)
+                      (serializer-options serializer-options*)
                       (default-value-thunk
                         (lambda ()
                           (if (maybe-value-set? (syntax->datum field-default))
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 1db83bb273..d23a0cde89 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -297,6 +297,40 @@ (define (sanitize-port value)
                     (lambda _ "lorem")
                     (sanitizer (lambda () #t)))))))
 
+(test-group "Serializer keyword arguments"
+  (define* (serialize-port field value #:key host)
+    (format #f "host=~a,port=~d" host value))
+
+  (define-configuration kwarg-config
+    (port
+     (port 80)
+     "Lorem Ipsum."
+     (serializer-options '(#:host "[2001:db8::1]"))))
+
+  (test-equal "keyword argument provided"
+    "host=[2001:db8::1],port=80"
+    (eval-gexp
+     (serialize-configuration (kwarg-config)
+                              kwarg-config-fields))))
+
+(test-group "Serializer keyword arguments, define-maybe"
+  (define* (serialize-port field value #:key host)
+    (format #f "host=~a,port=~d" host value))
+
+  (define-maybe port)
+
+  (define-configuration kwarg-config
+    (port
+     (maybe-port 80)
+     "Lorem Ipsum."
+     (serializer-options '(#:host "[2001:db8::1]"))))
+
+  (test-equal "keyword argument provided, maybe type"
+    "host=[2001:db8::1],port=80"
+    (eval-gexp
+     (serialize-configuration (kwarg-config)
+                              kwarg-config-fields))))
+
 \f
 ;;;
 ;;; define-maybe macro.
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC v2 5/5] services: configuration: New generic-ini module.
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (8 preceding siblings ...)
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 4/5] services: configuration: Add serializer-options field Bruno Victal
@ 2023-06-10 20:10 ` Bruno Victal
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
  2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
  11 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-10 20:10 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Implements a ‘serialize-ini-configuration’ procedure for serializing
record-types defined with define-configuration into generic INI files.

* gnu/services/configuration/generic-ini.scm: New module.
* tests/services/configuration/generic-ini.scm: Add tests for new module.
* Makefile.am: Register tests.
* gnu/local.mk: Register module.
---
 Makefile.am                                  |   1 +
 gnu/local.mk                                 |   1 +
 gnu/services/configuration/generic-ini.scm   | 129 +++++++++++++++++++
 tests/services/configuration/generic-ini.scm | 103 +++++++++++++++
 4 files changed, 234 insertions(+)
 create mode 100644 gnu/services/configuration/generic-ini.scm
 create mode 100644 tests/services/configuration/generic-ini.scm

diff --git a/Makefile.am b/Makefile.am
index ab901df757..8dc9a3438b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -552,6 +552,7 @@ SCM_TESTS =					\
   tests/services.scm				\
   tests/services/file-sharing.scm		\
   tests/services/configuration.scm		\
+  tests/services/configuration/generic-ini.scm	\
   tests/services/lightdm.scm			\
   tests/services/linux.scm			\
   tests/services/telephony.scm			\
diff --git a/gnu/local.mk b/gnu/local.mk
index ce16d37e2b..a05d70aff3 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -670,6 +670,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/cgit.scm			\
   %D%/services/ci.scm				\
   %D%/services/configuration.scm		\
+  %D%/services/configuration/generic-ini.scm	\
   %D%/services/cuirass.scm			\
   %D%/services/cups.scm				\
   %D%/services/databases.scm			\
diff --git a/gnu/services/configuration/generic-ini.scm b/gnu/services/configuration/generic-ini.scm
new file mode 100644
index 0000000000..88d145bc50
--- /dev/null
+++ b/gnu/services/configuration/generic-ini.scm
@@ -0,0 +1,129 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services configuration generic-ini)
+  #:use-module (gnu services configuration)
+  #:use-module (guix gexp)
+  #:use-module (srfi srfi-171)
+  #:use-module (ice-9 match)
+  #:export (ini-entry?
+            list-of-ini-entries?
+
+            serialize-ini-configuration))
+
+;;;
+;;; Generic INI serializer
+;;;
+
+\f
+;;;
+;;; Predicates
+;;;
+
+;; This is the same format used in SRFI-233 but without comment support.
+(define ini-entry?
+  (match-lambda
+    (((? symbol?) (? symbol?) (? string?)) #t)
+    (_ #f)))
+
+(define list-of-ini-entries?
+  (list-of ini-entry?))
+
+;;
+;; Overall design document
+;;
+;; This module implements a generic INI serializer for a record-type defined
+;; using define-configuration.
+;; It expects that the serialize-<type> procedures return a list with
+;; three elements of the form:
+;;    (list section key value)
+;; Where ‘section’ and ‘key’ are symbols and ‘value’ is a string.
+;; The fields within define-configuration do not have to be ordered in,
+;; any way whatsoever as the ‘serialize-ini’ will group them up automatically.
+;; This implies that no assumptions should be made regarding the order of the
+;; values in the serializied INI output.
+;;
+;; Additional notes:
+;; Q: Why not replace rcons with string-append and forego the ungexp-splice?
+;; A: The transduction happens outside of the G-Exp while the final string-append
+;;    takes place in the G-Exp.
+;;
+;; Debugging tips: Open a REPL and try one transducer at a time from
+;; ‘ini-transducer’.
+;;
+
+(define (add-section-header partition)
+  (let ((header (caar partition)))
+    (cons (list header)
+          partition)))
+
+(define serializer
+  (match-lambda
+    ((section)
+     #~(format #f "[~a]~%" '#$section))
+    ((section key value)
+     #~(format #f "~a=~a~%" '#$key #$value))
+    ;; Used for the newline between sections.
+    ('*section-separator* "\n")))
+
+(define ini-transducer
+  (compose (tpartition car)
+           (tmap add-section-header)
+           (tadd-between '(*section-separator*))
+           tconcatenate
+           (tmap serializer)))
+
+;; A “first-pass” serialization is performed and sorted in order
+;; to group up the fields by “section” before passing through the
+;; transducer.
+(define (serialize-ini-configuration config fields)
+  (let* ((srfi-233-IR
+          ;; First pass: “serialize” into a (disordered) list of
+          ;; SRFI-233 entries.
+          (list-transduce (base-transducer config) rcons fields))
+         (comparator (lambda (x y)
+                       ;; Sort the SRFI-233 entries by section.
+                       (string<=? (symbol->string (car x))
+                                  (symbol->string (car y)))))
+         (sorted-entries (sort srfi-233-IR comparator)))
+    #~(string-append
+       #$@(list-transduce ini-transducer rcons sorted-entries))))
+
+;; FIXME:RFC:
+;;       generic-ini- prefixed serializing procs?
+;;       (perhaps prefixed as generic-ini: ?)
+;; Example procedures:
+;;
+(define* (generic-ini-serialize-string field-name value #:key section)
+  (list section field-name value))
+
+;; field-name-transform can be used to “uglify” a field-name,
+;; e.g. want-ipv6?  ->  want_ipv6
+(define* (generic-ini-serialize-boolean field-name value #:key section
+                            (field-name-transform identity))
+  (list section (field-name-transform field-name)
+        (if value "true" "false")))
+
+
+;;; FIXME: delete this before inclusion, these are notes for the first RFC.
+;;;
+;;; Left out for now (but readily extendable):
+;;;  * Custom leading (presumed to be whitespace) characters for entries
+;;;    à la gitconfig, mostly pretty-printing purposes
+;;;  * Configurable delimiter (\n, \r\n, \0, ...)
+;;;  * Configurable Key-value separator (this is usually =)
diff --git a/tests/services/configuration/generic-ini.scm b/tests/services/configuration/generic-ini.scm
new file mode 100644
index 0000000000..3bdf0e12c2
--- /dev/null
+++ b/tests/services/configuration/generic-ini.scm
@@ -0,0 +1,103 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (tests services configuration generic-ini)
+  #:use-module (gnu services configuration)
+  #:use-module (gnu services configuration generic-ini)
+  #:use-module (guix diagnostics)
+  #:use-module (guix gexp)
+  #:use-module (guix store)
+  #:autoload (guix i18n) (G_)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-64)
+  #:use-module (srfi srfi-71))
+
+;;; Tests for the (gnu services configuration generic-ini) module.
+
+(test-begin "generic-ini serializer")
+
+
+(define expected-output "\
+[ranch]
+shepherd=Emma
+
+[shed]
+enabled=true
+capacity=50
+production=wool
+")
+
+\f
+;;;
+;;; Serializers
+;;;
+(define (strip-trailing-?-character field-name)
+  "Drop rightmost '?' character"
+  (let ((str (symbol->string field-name)))
+    (if (string-suffix? "?" str)
+        (string->symbol (string-drop-right str 1))
+        field-name)))
+
+(define* (serialize-string field-name value #:key section)
+  (list section field-name value))
+
+(define* (serialize-number field-name value #:key section)
+  (list section field-name (number->string value)))
+
+(define* (serialize-boolean field-name value #:key section)
+  (list section (strip-trailing-?-character field-name)
+        (if value "true" "false")))
+
+\f
+;;;
+;;; Record-type definition
+;;;
+
+(define-configuration foo-configuration
+  (production
+   (string "wool")
+   "Lorem Ipsum …"
+   (serializer-options '(#:section shed)))
+
+  (capacity
+   (number 50)
+   "Lorem Ipsum …"
+   (serializer-options '(#:section shed)))
+
+  (enabled?
+   (boolean #t)
+   "Lorem Ipsum …"
+   (serializer-options '(#:section shed)))
+
+  (shepherd
+   (string "Emma")
+   "Lorem Ipsum …"
+   (serializer-options '(#:section ranch))))
+
+(test-equal "Well-formed INI output from serialize-ini"
+  expected-output
+  ;; Serialize the above into a string, properly resolving any potential
+  ;; nested G-Exps as well.
+  (let* ((serialized-ini
+          (serialize-ini-configuration (foo-configuration)
+                                       foo-configuration-fields))
+         (lowered conn (with-store store
+                         ((lower-gexp serialized-ini) store))))
+    (eval (lowered-gexp-sexp lowered) (current-module))))
+
+(test-end)
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 00/11] Service subsystem improvements
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (9 preceding siblings ...)
  2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 5/5] services: configuration: New generic-ini module Bruno Victal
@ 2023-06-26 21:57 ` Bruno Victal
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args Bruno Victal
                     ` (12 more replies)
  2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
  11 siblings, 13 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:57 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal, liliana.prikler, maxim.cournoyer, ludo

This patch-series is an agglomeration of smaller but weakly-related
patch-series, done so in order to build up the case for the changes.
I structured the series in order to make it suitable for cherry-picking.


Summary of changes:

* Plumbing changes to serialize-configuration

By orienting it around SRFI-171 transducers, it's now easier
to build custom configuration serializing procedures.

* New define-configuration syntax: serializer-options

Serializers may now accept more than two arguments.

* New module (gnu services configuration generic-ini)

* Deduplicate often used predicates.

* network-manager-service-type cleanup & new fields.



Notable changes since v2:

* Documentation changes

The documentation for define-configuration was reworded to make later
syntax extensions easier to document.
In addition, the new 'serializer-options' is now documented.


* (gnu services configuration) changes

** New predicates.

Reduce code duplication by migrating some commonly used predicates into
this module.


* generic-ini changes

** Initial field testing

Some deficiencies were found & corrected while doing a first field
testing when network-manager-service-type was refactored to make use
of this module.

** generic-ini- serializers

** Handling of multiple entries

Implemented as a transducer, this is useful to deal with escape-hatch
fields.


* network-manager-service-type changes

** Use define-configuration for <network-manager-configuration>

** Refactored serialization process to use the new generic-ini module

** New configuration fields: log-configuration and extra-options

Provides an escape hatch for options not yet implemented in the
record type.

** Renamed fields: 'network-manager' to 'package'

Naming the field 'package' is more informative & less confusing than a
reduplication of the package-name with the field-name itself.



Omissions in generic-ini:

For now, I've omitted:
* Custom leading (presumed to be whitespace) characters for entries
  à la gitconfig, mostly beautifying purposes
* Configurable delimiter (\n, \r\n, \0, ...)
* Configurable Key-value separator (this is usually =)

These can be implemented later if required.



Notes:

The interface in (gnu services configuration generic-ini) is still in its
infancy and might require further adjustments/additions and I'm still
thinking about its potential generalizations to TOML & co.
For the time being I'd prefer not to promise any interface stability.



Nice to haves:

I didn't have much luck in replacing the memq approach in the predicate
with define-enumeration:

--8<---------------cut here---------------start------------->8---
;; This works.
(define (network-manager-log-level? x)
  (memq x '(off err warn info debug trace)))

;; This does not.
(define-enumeration network-manager-log-level?
  (off err warn info debug trace)
  network-manager-log-level-set)

;; While executing meta-command:
;; ERROR:
;;   1. &origin: "network-manager-log-level?"
;;   2. &message: "not a member of the set"
;;   3. &syntax:
;;      form: #f
;;      subform: #f
--8<---------------cut here---------------end--------------->8---

Would be nice to know what went wrong and whether an enumeration could
be used here instead.


Bruno Victal (11):
  services: configuration: Simplify normalize-extra-args.
  services: configuration: Use transducers within
    serialize-configuration.
  services: fstrim-service-type: Serialize with SRFI-171 transducers.
  doc: Rewrite define-configuration.
  services: configuration: Add serializer-options field.
  services: configuration: New generic-ini module.
  services: configuration: Add some commonly used predicates.
  services: NetworkManager: Use define-configuration and generic-ini.
  services: NetworkManager: Prefer package over network-manager.
  services: NetworkManager: add log-configuration field.
  services: NetworkManager: Add extra-options field.

 Makefile.am                                  |   1 +
 doc/guix.texi                                | 161 +++++----
 gnu/local.mk                                 |   1 +
 gnu/services/audio.scm                       |   7 +-
 gnu/services/configuration.scm               | 108 ++++--
 gnu/services/configuration/generic-ini.scm   | 165 +++++++++
 gnu/services/linux.scm                       |  11 +-
 gnu/services/networking.scm                  | 352 ++++++++++++++-----
 gnu/services/telephony.scm                   |  49 ++-
 tests/services/configuration.scm             |  88 ++++-
 tests/services/configuration/generic-ini.scm | 129 +++++++
 11 files changed, 861 insertions(+), 211 deletions(-)
 create mode 100644 gnu/services/configuration/generic-ini.scm
 create mode 100644 tests/services/configuration/generic-ini.scm


base-commit: ac86174e22fcd762893bd4515786b1376af9397b
-- 
2.39.2





^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-10-02 17:00     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration Bruno Victal
                     ` (11 subsequent siblings)
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/configuration.scm
(define-configuration-helper, normalize-extra-args): Use #f instead of %unset-value.
---
 gnu/services/configuration.scm | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 367b85c1be..dafe72f4fe 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -190,32 +190,32 @@ (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))
+               (sanitizer* #f)
+               (serializer* #f))
       (syntax-case s (sanitizer serializer empty-serializer)
         (((sanitizer proc) tail ...)
-         (if (maybe-value-set? sanitizer*)
-             (syntax-violation 'sanitizer "duplicate entry"
-                               #'proc)
+         (if 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)
+         (if serializer*
+             (syntax-violation 'serializer
+                               "duplicate or conflicting entry" #'proc)
              (loop #'(tail ...) sanitizer* #'proc)))
         ((empty-serializer tail ...)
-         (if (maybe-value-set? serializer*)
+         (if 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*)))
+         (every not (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)))))))
+           (values (list #f #'proc)))))))
 
   (syntax-case syn ()
     ((_ stem (field field-type+def doc extra-args ...) ...)
@@ -239,11 +239,11 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
                      default-value))
                   #'((field-type def) ...)))
             ((field-sanitizer ...)
-             (map maybe-value #'(sanitizer* ...)))
+             #'(sanitizer* ...))
             ((field-serializer ...)
              (map (lambda (type proc)
                     (and serialize?
-                         (or (maybe-value proc)
+                         (or proc
                              (if serializer-prefix
                                  (id #'stem serializer-prefix #'serialize- type)
                                  (id #'stem #'serialize- type)))))

base-commit: ac86174e22fcd762893bd4515786b1376af9397b
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-10-02 17:25     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 03/11] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
                     ` (10 subsequent siblings)
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Introduces 'base-transducer', a SRFI-171 based transducer that can be used as a
starting point for writing custom configuration record serializing procedures.

This also fixes the symbol maybe-value serialization test case.

* gnu/services/configuration.scm (empty-serializer?): New predicate.
(base-transducer, tfilter-maybe-value): New procedure.
(serialize-configuration): Adapt to use base-transducer.

* gnu/services/telephony.scm (jami-account->alist): Use transducers to skip
fields that are unserializable or whose field maybe-value is unset.

* tests/services/configuration.scm: Remove test-expect-fail.
---
 gnu/services/configuration.scm   | 29 ++++++++++++++++++++++++-----
 gnu/services/telephony.scm       | 27 +++++++++++++--------------
 tests/services/configuration.scm |  6 +-----
 3 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index dafe72f4fe..cd2cb8318b 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -42,6 +42,7 @@ (define-module (gnu services configuration)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-171)
   #:export (configuration-field
             configuration-field-name
             configuration-field-type
@@ -59,6 +60,10 @@ (define-module (gnu services configuration)
             define-configuration/no-serialization
             no-serialization
 
+            empty-serializer?
+            tfilter-maybe-value
+            base-transducer
+
             serialize-configuration
             define-maybe
             define-maybe/no-serialization
@@ -125,13 +130,27 @@ (define-record-type* <configuration-field>
   (default-value-thunk configuration-field-default-value-thunk)
   (documentation configuration-field-documentation))
 
+(define (empty-serializer? field)
+  (eq? empty-serializer
+       (configuration-field-serializer field)))
+
+(define (tfilter-maybe-value config)
+  (tfilter (lambda (field)
+             (let ((field-value ((configuration-field-getter field) config)))
+               (maybe-value-set? field-value)))))
+
+(define (base-transducer config)
+  (compose (tremove empty-serializer?)
+           ;; Only serialize fields whose value isn't '%unset-marker%.
+           (tfilter-maybe-value config)
+           (tmap (lambda (field)
+                   ((configuration-field-serializer field)
+                    (configuration-field-name field)
+                    ((configuration-field-getter field) config))))))
+
 (define (serialize-configuration config fields)
   #~(string-append
-     #$@(map (lambda (field)
-               ((configuration-field-serializer field)
-                (configuration-field-name field)
-                ((configuration-field-getter field) config)))
-             fields)))
+     #$@(list-transduce (base-transducer config) rcons fields)))
 
 (define-syntax-rule (id ctx parts ...)
   "Assemble PARTS into a raw (unhygienic) identifier."
diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
index 23ccb8d403..56b7772f58 100644
--- a/gnu/services/telephony.scm
+++ b/gnu/services/telephony.scm
@@ -37,6 +37,7 @@ (define-module (gnu services telephony)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-2)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-171)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (jami-account
@@ -204,22 +205,20 @@ (define (jami-account->alist jami-account-object)
       ('rendezvous-point? "Account.rendezVous")
       ('peer-discovery? "Account.peerDiscovery")
       ('bootstrap-hostnames "Account.hostname")
-      ('name-server-uri "RingNS.uri")
-      (_ #f)))
+      ('name-server-uri "RingNS.uri")))
 
-  (filter-map (lambda (field)
-                (and-let* ((name (field-name->account-detail
+  (define jami-account-transducer
+    (compose (tremove empty-serializer?)
+             (tfilter-maybe-value jami-account-object)
+             (tmap (lambda (field)
+                     (let* ((name (field-name->account-detail
                                   (configuration-field-name field)))
-                           (value ((configuration-field-serializer field)
-                                   name ((configuration-field-getter field)
-                                         jami-account-object)))
-                           ;; The define-maybe default serializer produces an
-                           ;; empty string for unspecified values.
-                           (value* (if (string-null? value)
-                                       #f
-                                       value)))
-                  (cons name value*)))
-              jami-account-fields))
+                            (value ((configuration-field-serializer field)
+                                    name ((configuration-field-getter field)
+                                          jami-account-object))))
+                       (cons name value))))))
+
+  (list-transduce jami-account-transducer rcons jami-account-fields))
 
 (define (jami-account-list? val)
   (and (list? val)
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 8ad5907f37..40a4e74b4d 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -337,13 +337,9 @@ (define-maybe symbol)
 (define-configuration config-with-maybe-symbol
   (protocol maybe-symbol ""))
 
-;;; Maybe symbol values are currently seen as serializable, because the
-;;; unspecified value is '%unset-marker%, which is a symbol itself.
-;;; TODO: Remove expected fail marker after resolution.
-(test-expect-fail 1)
 (test-equal "symbol maybe value serialization, unspecified"
   ""
-  (gexp->approximate-sexp
+  (eval-gexp
    (serialize-configuration (config-with-maybe-symbol)
                             config-with-maybe-symbol-fields)))
 
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 03/11] services: fstrim-service-type: Serialize with SRFI-171 transducers.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args Bruno Victal
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 04/11] doc: Rewrite define-configuration Bruno Victal
                     ` (9 subsequent siblings)
  12 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/linux.scm (serialize-fstrim-configuration): Refactor to use
base-transducer.
---
 gnu/services/linux.scm | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index d105c42850..3cfa6d6855 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -40,6 +40,7 @@ (define-module (gnu services linux)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-171)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (earlyoom-configuration
@@ -227,13 +228,9 @@ (define-configuration fstrim-configuration
   (prefix fstrim-))
 
 (define (serialize-fstrim-configuration config)
-  (concatenate
-   (filter list?
-           (map (lambda (field)
-                  ((configuration-field-serializer field)
-                   (configuration-field-name field)
-                   ((configuration-field-getter field) config)))
-                fstrim-configuration-fields))))
+  (list-transduce (compose (base-transducer config) tconcatenate)
+                  rcons
+                  fstrim-configuration-fields))
 
 (define (fstrim-mcron-job config)
   (match-record config <fstrim-configuration> (package schedule)
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 04/11] doc: Rewrite define-configuration.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (2 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 03/11] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-10-02 18:28     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 05/11] services: configuration: Add serializer-options field Bruno Victal
                     ` (8 subsequent siblings)
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Rewrite this section to make it easier to document later syntactical
changes.

* doc/guix.texi (Complex Configurations): Rewrite define-configuration
documentation. Fix simple serializer example.
---
 doc/guix.texi | 102 +++++++++++++++++++++-----------------------------
 1 file changed, 42 insertions(+), 60 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 853396f776..8355260378 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -41953,54 +41953,33 @@ Complex Configurations
 files, you can use the utilities defined in the @code{(gnu services
 configuration)} module.
 
-The main utility is the @code{define-configuration} macro, which you
-will use to define a Scheme record type (@pxref{Record Overview,,,
-guile, GNU Guile Reference Manual}).  The Scheme record will be
-serialized to a configuration file by using @dfn{serializers}, which are
-procedures that take some kind of Scheme value and returns a
-G-expression (@pxref{G-Expressions}), which should, once serialized to
-the disk, return a string.  More details are listed below.
+The main utility is the @code{define-configuration} macro, a helper
+used to define a Scheme record type (@pxref{Record Overview,,,
+guile, GNU Guile Reference Manual}).  The fields from this Scheme record
+can be serialized using @dfn{serializers}, which are procedures that take
+some kind of Scheme value and translates them into another Scheme value or
+@ref{G-Expressions}.
 
 @defmac define-configuration name clause1 clause2 @dots{}
 Create a record type named @code{@var{name}} that contains the
 fields found in the clauses.
 
-A clause can have one of the following forms:
+A clause has the following form:
 
 @example
 (@var{field-name}
- (@var{type} @var{default-value})
- @var{documentation})
-
-(@var{field-name}
- (@var{type} @var{default-value})
- @var{documentation}
- (serializer @var{serializer}))
-
-(@var{field-name}
- (@var{type})
- @var{documentation})
-
-(@var{field-name}
- (@var{type})
- @var{documentation}
- (serializer @var{serializer}))
-
-(@var{field-name}
- (@var{type})
+ @var{type-decl}
  @var{documentation}
- (sanitizer @var{sanitizer})
-
-(@var{field-name}
- (@var{type})
- @var{documentation}
- (sanitizer @var{sanitizer})
- (serializer @var{serializer}))
+ @var{option*}
+ @dots{})
 @end example
 
 @var{field-name} is an identifier that denotes the name of the field in
 the generated record.
 
+@var{type-decl} is either @code{@var{type}} for fields that require a
+value to be set or @code{(@var{type} @var{default})} otherwise.
+
 @var{type} is the type of the value corresponding to @var{field-name};
 since Guile is untyped, a predicate
 procedure---@code{@var{type}?}---will be called on the value
@@ -42018,6 +41997,28 @@ Complex Configurations
 @var{documentation} is a string formatted with Texinfo syntax which
 should provide a description of what setting this field does.
 
+@var{option*} is one of the following subclauses:
+
+@table @asis
+@item @code{empty-serializer}
+Exclude this field from serialization.
+
+@item @code{(serializer @var{serializer})}
+@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
+@ref{G-Expressions} that represents the content that will be serialized
+to the configuration file.  If none is specified, a procedure of the
+name @code{serialize-@var{type}} will be used.
+
+An example of a simple serializer procedure:
+@lisp
+(define (serialize-boolean field-name value)
+  (let ((value (if value "true" "false")))
+    #~(string-append '#$field-name #$value)))
+@end lisp
+
+@item @code{(sanitizer @var{sanitizer})}
 @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
@@ -42031,21 +42032,7 @@ Complex Configurations
         ((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
-G-expression (@pxref{G-Expressions}) that represents the content that
-will be serialized to the configuration file.  If none is specified, a
-procedure of the name @code{serialize-@var{type}} will be used.
-
-A simple serializer procedure could look like this:
-
-@lisp
-(define (serialize-boolean field-name value)
-  (let ((value (if value "true" "false")))
-    #~(string-append #$field-name #$value)))
-@end lisp
+@end table
 
 In some cases multiple different configuration records might be defined
 in the same file, but their serializers for the same type might have to
@@ -42066,13 +42053,13 @@ Complex Configurations
 
 (define-configuration foo-configuration
   (label
-   (string)
+   string
    "The name of label.")
   (prefix foo-))
 
 (define-configuration bar-configuration
   (ip-address
-   (string)
+   string
    "The IPv4 address for this device.")
   (prefix bar-))
 @end lisp
@@ -42164,11 +42151,6 @@ Complex Configurations
 disk by using something like @code{mixed-text-file}.
 @end deffn
 
-@deffn {Procedure} empty-serializer field-name value
-A serializer that just returns an empty string.  The
-@code{serialize-package} procedure is an alias for this.
-@end deffn
-
 Once you have defined a configuration record, you will most likely also
 want to document it so that other people know to use it.  To help with
 that, there are two procedures, both of which are documented below.
@@ -42271,7 +42253,7 @@ Complex Configurations
 
 (define-configuration contact-configuration
   (name
-   (string)
+   string
    "The name of the contact."
    serialize-contact-name)
   (phone-number
@@ -42281,15 +42263,15 @@ Complex Configurations
    maybe-string
    "The person's email address.")
   (married?
-   (boolean)
+   boolean
    "Whether the person is married."))
 
 (define-configuration contacts-list-configuration
   (name
-   (string)
+   string
    "The name of the owner of this contact list.")
   (email
-   (string)
+   string
    "The owner's email address.")
   (contacts
    (list-of-contact-configurations '())
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 05/11] services: configuration: Add serializer-options field.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (3 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 04/11] doc: Rewrite define-configuration Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-10-02 19:12     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 06/11] services: configuration: New generic-ini module Bruno Victal
                     ` (7 subsequent siblings)
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Allow relaying additional arguments to a serialize-<type> procedure.

* gnu/services/configuration.scm (configuration-field)
<serializer-options>: New field.
(base-transducer, define-maybe-helper): Adjust to relay additional arguments.
(normalize-extra-args): Implement serializer-options literal.
* tests/services/configuration.scm: Add tests for serializer-options.
* doc/guix.texi (Complex Configurations): Document serializer-options.
---
 doc/guix.texi                    | 11 +++++
 gnu/services/configuration.scm   | 49 ++++++++++++-------
 tests/services/configuration.scm | 82 ++++++++++++++++++++++++++++++++
 3 files changed, 126 insertions(+), 16 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 8355260378..14802e9366 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -42032,6 +42032,17 @@ Complex Configurations
         ((symbol? value) (symbol->string value))
         (else (error "bad value"))))
 @end lisp
+
+@item @code{(serializer-options @var{arglst})}
+@var{arglst} is a list of extra arguments that are relayed to the
+serializing procedure.  This allows for writing serialization
+procedures that take more than two arguments.
+
+An example of a serializer procedure that requires additional data:
+@lisp
+(define* (serialize-port field value #:key context)
+  #~(format #f "section=~a,port=~d" #$context #$value))
+@end lisp
 @end table
 
 In some cases multiple different configuration records might be defined
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index cd2cb8318b..4eee5a26c2 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -50,6 +50,7 @@ (define-module (gnu services configuration)
             configuration-field-error
             configuration-field-sanitizer
             configuration-field-serializer
+            configuration-field-serializer-options
             configuration-field-getter
             configuration-field-default-value-thunk
             configuration-field-documentation
@@ -127,6 +128,7 @@ (define-record-type* <configuration-field>
   (predicate configuration-field-predicate)
   (sanitizer configuration-field-sanitizer)
   (serializer configuration-field-serializer)
+  (serializer-options configuration-field-serializer-options)
   (default-value-thunk configuration-field-default-value-thunk)
   (documentation configuration-field-documentation))
 
@@ -144,9 +146,13 @@ (define (base-transducer config)
            ;; Only serialize fields whose value isn't '%unset-marker%.
            (tfilter-maybe-value config)
            (tmap (lambda (field)
-                   ((configuration-field-serializer field)
-                    (configuration-field-name field)
-                    ((configuration-field-getter field) config))))))
+                   (let ((serializer (configuration-field-serializer field))
+                         (field-name (configuration-field-name field))
+                         (value
+                          ((configuration-field-getter field) config))
+                         (serializer-options
+                          (configuration-field-serializer-options field)))
+                     (apply serializer field-name value serializer-options))))))
 
 (define (serialize-configuration config fields)
   #~(string-append
@@ -173,10 +179,9 @@ (define (define-maybe-helper serialize? prefix syn)
              (or (not (maybe-value-set? val))
                  (stem? val)))
            #,@(if serialize?
-                  (list #'(define (serialize-maybe-stem field-name val)
-                            (if (stem? val)
-                                (serialize-stem field-name val)
-                                "")))
+                  (list #'(define (serialize-maybe-stem field-name val . rest)
+                            (when (maybe-value-set? val)
+                              (apply serialize-stem field-name val rest))))
                   '()))))))
 
 (define-syntax define-maybe
@@ -210,38 +215,49 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
     "Extract and normalize arguments following @var{doc}."
     (let loop ((s s)
                (sanitizer* #f)
-               (serializer* #f))
-      (syntax-case s (sanitizer serializer empty-serializer)
+               (serializer* #f)
+               (serializer-options* #f))
+      (syntax-case s (sanitizer serializer empty-serializer serializer-options)
         (((sanitizer proc) tail ...)
          (if sanitizer*
              (syntax-violation 'sanitizer
                                "duplicate entry" #'proc)
-             (loop #'(tail ...) #'proc serializer*)))
+             (loop #'(tail ...)
+                   #'proc serializer* serializer-options*)))
         (((serializer proc) tail ...)
          (if serializer*
              (syntax-violation 'serializer
                                "duplicate or conflicting entry" #'proc)
-             (loop #'(tail ...) sanitizer* #'proc)))
+             (loop #'(tail ...)
+                   sanitizer* #'proc serializer-options*)))
         ((empty-serializer tail ...)
          (if serializer*
              (syntax-violation 'empty-serializer
                                "duplicate or conflicting entry" #f)
-             (loop #'(tail ...) sanitizer* #'empty-serializer)))
+             (loop #'(tail ...)
+                   sanitizer* #'empty-serializer #f)))
+        (((serializer-options args) tail ...)
+         (if serializer-options*
+             (syntax-violation 'serializer-options
+                               "duplicate or conflicting entry" #f)
+             (loop #'(tail ...)
+                   sanitizer* serializer* #'args)))
         (()  ; stop condition
-         (values (list sanitizer* serializer*)))
+         (values (list sanitizer* serializer*
+                       (or serializer-options* #'(quote ())))))
         ((proc)  ; TODO: deprecated, to be removed.
-         (every not (list sanitizer* serializer*))
+         (every not (list sanitizer* serializer* serializer-options*))
          (begin
            (warning #f (G_ "specifying serializers after documentation is \
 deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
-           (values (list #f #'proc)))))))
+           (values (list #f #'proc #'(quote ()))))))))
 
   (syntax-case syn ()
     ((_ stem (field field-type+def doc extra-args ...) ...)
      (with-syntax
          ((((field-type def) ...)
            (map normalize-field-type+def #'(field-type+def ...)))
-          (((sanitizer* serializer*) ...)
+          (((sanitizer* serializer* serializer-options*) ...)
            (map normalize-extra-args #'((extra-args ...) ...))))
        (with-syntax
            (((field-getter ...)
@@ -327,6 +343,7 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
                        (or field-sanitizer
                            (id #'stem #'validate- #'stem #'- #'field)))
                       (serializer field-serializer)
+                      (serializer-options serializer-options*)
                       (default-value-thunk
                         (lambda ()
                           (if (maybe-value-set? (syntax->datum field-default))
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 40a4e74b4d..8b1d1e4749 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -297,6 +297,88 @@ (define (sanitize-port value)
                     (lambda _ "lorem")
                     (sanitizer (lambda () #t)))))))
 
+(test-group "Serializer options"
+  (test-group "Serialize keyword arguments"
+    (define* (serialize-port field value #:key host)
+      (format #f "host=~a,port=~d" host value))
+
+    (define-configuration kwarg-config
+      (port
+       (port 80)
+       "Lorem Ipsum."
+       (serializer-options '(#:host "[2001:db8::1]"))))
+
+    (define-maybe port)
+    (define-configuration kwarg-maybe-config
+      (port
+       (maybe-port 80)
+       "Lorem Ipsum."
+       (serializer-options '(#:host "[2001:db8::1]"))))
+
+    (test-equal "keyword argument provided"
+      "host=[2001:db8::1],port=80"
+      (eval-gexp
+       (serialize-configuration (kwarg-config)
+                                kwarg-config-fields)))
+
+    (test-equal "keyword argument provided, maybe type"
+      "host=[2001:db8::1],port=80"
+      (eval-gexp
+       (serialize-configuration (kwarg-maybe-config)
+                                kwarg-maybe-config-fields))))
+
+  (test-group "Serialize optional arguments"
+    (define* (serialize-port field-name value #:optional override-name)
+      (format #f "~a=~d" (or override-name field-name) value))
+
+    (define-configuration with-optarg
+      (port
+       (port 80)
+       "Lorem Ipsum."
+       (serializer-options '(service-port))))
+
+    (define-configuration without-optarg
+      (port
+       (port 80)
+       "Lorem Ipsum."))
+
+    (test-equal "optional argument, provided"
+      "service-port=80"
+      (eval-gexp (serialize-configuration (with-optarg)
+                                          with-optarg-fields)))
+
+    (test-equal "optional argument, absent"
+      "port=80"
+      (eval-gexp (serialize-configuration (without-optarg)
+                                          without-optarg-fields))))
+
+  (test-group "Serialize optional & keyword arguments"
+    (define* (serialize-port field-name value #:optional override-name
+                             #:key host)
+      (format #f "host=~a,~a=~d" host (or override-name field-name) value))
+
+    (define-configuration mixed-args
+      (port
+       (port 80)
+       "Lorem Ipsum."
+       (serializer-options '(service-port #:host "example.com"))))
+
+    (define-configuration mixed-no-optarg
+      (port
+       (port 80)
+       "Lorem Ipsum."
+       (serializer-options '(#:host "example.com"))))
+
+    (test-equal "mixed arguments, optional provided"
+      "host=example.com,service-port=80"
+      (eval-gexp (serialize-configuration (mixed-args)
+                                          mixed-args-fields)))
+
+    (test-equal "mixed arguments, optional absent"
+      "host=example.com,port=80"
+      (eval-gexp (serialize-configuration (mixed-no-optarg)
+                                          mixed-no-optarg-fields)))))
+
 \f
 ;;;
 ;;; define-maybe macro.
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 06/11] services: configuration: New generic-ini module.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (4 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 05/11] services: configuration: Add serializer-options field Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-10-02 19:15     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 07/11] services: configuration: Add some commonly used predicates Bruno Victal
                     ` (6 subsequent siblings)
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Implements a ‘serialize-ini-configuration’ procedure for serializing
record-types defined with define-configuration into generic INI files.

* gnu/services/configuration/generic-ini.scm: New module.
* tests/services/configuration/generic-ini.scm: Add tests for new module.
* Makefile.am: Register tests.
* gnu/local.mk: Register module.
---
 Makefile.am                                  |   1 +
 gnu/local.mk                                 |   1 +
 gnu/services/configuration/generic-ini.scm   | 165 +++++++++++++++++++
 tests/services/configuration/generic-ini.scm | 129 +++++++++++++++
 4 files changed, 296 insertions(+)
 create mode 100644 gnu/services/configuration/generic-ini.scm
 create mode 100644 tests/services/configuration/generic-ini.scm

diff --git a/Makefile.am b/Makefile.am
index a386e6033c..b6d048f140 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -553,6 +553,7 @@ SCM_TESTS =					\
   tests/services.scm				\
   tests/services/file-sharing.scm		\
   tests/services/configuration.scm		\
+  tests/services/configuration/generic-ini.scm	\
   tests/services/lightdm.scm			\
   tests/services/linux.scm			\
   tests/services/telephony.scm			\
diff --git a/gnu/local.mk b/gnu/local.mk
index e65888a044..796ac33107 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -670,6 +670,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/cgit.scm			\
   %D%/services/ci.scm				\
   %D%/services/configuration.scm		\
+  %D%/services/configuration/generic-ini.scm	\
   %D%/services/cuirass.scm			\
   %D%/services/cups.scm				\
   %D%/services/databases.scm			\
diff --git a/gnu/services/configuration/generic-ini.scm b/gnu/services/configuration/generic-ini.scm
new file mode 100644
index 0000000000..4f83cce13a
--- /dev/null
+++ b/gnu/services/configuration/generic-ini.scm
@@ -0,0 +1,165 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services configuration generic-ini)
+  #:use-module (gnu services configuration)
+  #:use-module (guix gexp)
+  #:use-module (srfi srfi-9)
+  #:use-module (srfi srfi-171)
+  #:use-module (srfi srfi-171 meta)
+  #:use-module (ice-9 match)
+  #:export (ini-entry?
+            list-of-ini-entries?
+
+            ini-entries
+            ini-entries?
+            entries
+
+            serialize-ini-configuration
+            generic-ini-serialize-string
+            generic-ini-serialize-boolean
+            generic-ini-serialize-ini-entry
+            generic-ini-serialize-list-of-ini-entries))
+
+;;;
+;;; Generic INI serializer
+;;;
+
+\f
+;;;
+;;; Predicates
+;;;
+
+;; This is the same format used in SRFI-233 but without comment support.
+(define ini-entry?
+  (match-lambda
+    (((? symbol?) (? symbol?) (? string?)) #t)
+    (_ #f)))
+
+(define list-of-ini-entries?
+  (list-of ini-entry?))
+
+;;
+;; Overall design document
+;;
+;; This module implements a generic INI serializer for a record-type defined
+;; using define-configuration.
+;; It expects that the serialize-<type> procedures return a list with
+;; three elements of the form:
+;;    (list section key value)
+;; Where ‘section’ and ‘key’ are symbols and ‘value’ is a string.
+;; For serializing procedures that have to return multiple entries at once,
+;; such as encountered when synthesizing configuration from a record object
+;; or “escape hatch fields”, it must wrap the result by calling ‘ini-entries’
+;; with a list of INI-entries as described above.
+;; This is implemented as a constructor for a SRFI-9 record type named
+;; “<ini-entries>”.
+;;
+;; The fields within define-configuration do not have to be ordered in,
+;; any way whatsoever as the ‘serialize-ini’ will group them up automatically.
+;; This implies that no assumptions should be made regarding the order of the
+;; values in the serializied INI output.
+;;
+;; Additional notes:
+;; Q: Why not replace rcons with string-append and forego the ungexp-splice?
+;; A: The transduction happens outside of the G-Exp while the final string-append
+;;    takes place in the G-Exp.
+;;
+;; Debugging tips: Open a REPL and try one transducer at a time from
+;; ‘ini-transducer’.
+;;
+
+;; A “bag” holding multiple ini-entries.
+(define-record-type <ini-entries>
+  (ini-entries val)
+  ini-entries?
+  (val entries))
+
+(define (add-section-header partition)
+  (let ((header (caar partition)))
+    (cons (list header)
+          partition)))
+
+(define serializer
+  (match-lambda
+    ((section)
+     #~(format #f "[~a]~%" '#$section))
+    ((section key value)
+     #~(format #f "~a=~a~%" '#$key #$value))
+    ;; Used for the newline between sections.
+    ('*section-separator* "\n")))
+
+(define ini-transducer
+  (compose (tpartition car)
+           (tmap add-section-header)
+           (tadd-between '(*section-separator*))
+           tconcatenate
+           (tmap serializer)))
+
+;; A selective version of ‘tconcatenate’ but for ‘<ini-entries>’ objects only.
+(define (tconcatenate-ini-entries reducer)
+  (case-lambda
+    (() '())
+    ((result) (reducer result))
+    ((result input)
+     (if (ini-entries? input)
+         (list-reduce (preserving-reduced reducer) result (entries input))
+         (reducer result input)))))
+
+;; A “first-pass” serialization is performed and sorted in order
+;; to group up the fields by “section” before passing through the
+;; transducer.
+(define (serialize-ini-configuration config fields)
+  (let* ((srfi-233-IR
+          ;; First pass: “serialize” into a (disordered) list of
+          ;; SRFI-233 entries.
+          (list-transduce (compose (base-transducer config)
+                                   tconcatenate-ini-entries)
+                          rcons fields))
+         (comparator (lambda (x y)
+                       ;; Sort the SRFI-233 entries by section.
+                       (string<=? (symbol->string (car x))
+                                  (symbol->string (car y)))))
+         (sorted-entries (sort srfi-233-IR comparator)))
+    #~(string-append
+       #$@(list-transduce ini-transducer rcons sorted-entries))))
+
+\f
+;;;
+;;; Serializers
+;;;
+
+;; These are “gratuitous” serializers that can be readily used by
+;; using the literal (prefix generic-ini-) within define-configuration.
+
+;; Notes: field-name-transform can be used to “uglify” a field-name,
+;; e.g. want-ipv6?  ->  want_ipv6
+(define* (generic-ini-serialize-string field-name value #:key section
+                                       (field-name-transform identity))
+  (list section (field-name-transform field-name) value))
+
+(define* (generic-ini-serialize-boolean field-name value #:key section
+                            (field-name-transform identity))
+  (list section (field-name-transform field-name)
+        (if value "true" "false")))
+
+(define (generic-ini-serialize-ini-entry field-name value)
+  value)
+
+(define (generic-ini-serialize-list-of-ini-entries field-name value)
+  (ini-entries value))
diff --git a/tests/services/configuration/generic-ini.scm b/tests/services/configuration/generic-ini.scm
new file mode 100644
index 0000000000..797a01af31
--- /dev/null
+++ b/tests/services/configuration/generic-ini.scm
@@ -0,0 +1,129 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (tests services configuration generic-ini)
+  #:use-module (gnu services configuration)
+  #:use-module (gnu services configuration generic-ini)
+  #:use-module (guix diagnostics)
+  #:use-module (guix gexp)
+  #:use-module (guix store)
+  #:autoload (guix i18n) (G_)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-64)
+  #:use-module (srfi srfi-71))
+
+;;; Tests for the (gnu services configuration generic-ini) module.
+
+(test-begin "generic-ini serializer")
+
+
+(define expected-output "\
+[guardians]
+llamas=Tommy,Isabella
+donkeys=Franz,Olly
+
+[ranch]
+shepherd=Emma
+
+[shed]
+colours=Alizarin
+enabled=true
+capacity=50
+production=wool
+
+[vehicles]
+cars=313
+bikes=Amaryllis
+")
+
+\f
+;;;
+;;; Serializers
+;;;
+(define (strip-trailing-?-character field-name)
+  "Drop rightmost '?' character"
+  (let ((str (symbol->string field-name)))
+    (if (string-suffix? "?" str)
+        (string->symbol (string-drop-right str 1))
+        field-name)))
+
+(define* (serialize-string field-name value #:key section)
+  (list section field-name value))
+
+(define* (serialize-number field-name value #:key section)
+  (list section field-name (number->string value)))
+
+(define* (serialize-boolean field-name value #:key section)
+  (list section (strip-trailing-?-character field-name)
+        (if value "true" "false")))
+
+(define serialize-ini-entry
+  generic-ini-serialize-ini-entry)
+
+(define serialize-list-of-ini-entries
+  generic-ini-serialize-list-of-ini-entries)
+
+\f
+;;;
+;;; Record-type definition
+;;;
+
+(define-configuration foo-configuration
+  (production
+   (string "wool")
+   "Lorem Ipsum …"
+   (serializer-options '(#:section shed)))
+
+  (capacity
+   (number 50)
+   "Lorem Ipsum …"
+   (serializer-options '(#:section shed)))
+
+  (enabled?
+   (boolean #t)
+   "Lorem Ipsum …"
+   (serializer-options '(#:section shed)))
+
+  (shepherd
+   (string "Emma")
+   "Lorem Ipsum …"
+   (serializer-options '(#:section ranch)))
+
+  (raw-entry
+   (ini-entry '(shed colours "Alizarin"))
+   "Lorem Ipsum …")
+
+  (escape-hatch
+   (list-of-ini-entries '((vehicles bikes "Amaryllis")
+                          (vehicles cars "313")
+                          (guardians donkeys "Franz,Olly")
+                          (guardians llamas "Tommy,Isabella")))
+   "Lorem Ipsum …"))
+
+(test-equal "Well-formed INI output from serialize-ini"
+  expected-output
+  ;; Serialize the above into a string, properly resolving any potential
+  ;; nested G-Exps as well.
+  (let* ((serialized-ini
+          (serialize-ini-configuration (foo-configuration)
+                                       foo-configuration-fields))
+         (lowered conn (with-store store
+                         ((lower-gexp serialized-ini) store))))
+    (eval (lowered-gexp-sexp lowered) (current-module))))
+
+(test-end)
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 07/11] services: configuration: Add some commonly used predicates.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (5 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 06/11] services: configuration: New generic-ini module Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 08/11] services: NetworkManager: Use define-configuration and generic-ini Bruno Victal
                     ` (5 subsequent siblings)
  12 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/configuration.scm (list-of-packages?, list-of-symbols?): New
predicate.
* gnu/services/audio.scm (list-of-symbol?): Remove.
* gnu/services/telephony.scm (string-list?): Remove.
(serialize-string-list): Rename to …
(serialize-list-of-strings): … this.
(account-fingerprint-list?, jami-account-list?): Use list-of.
* doc/guix.texi: Update it.
---
 doc/guix.texi                  |  4 ++--
 gnu/services/audio.scm         |  7 ++-----
 gnu/services/configuration.scm | 16 ++++++++++++++++
 gnu/services/telephony.scm     | 20 +++++++-------------
 4 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 14802e9366..2f7e734874 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -27850,7 +27850,7 @@ Telephony Services
 connection to the Internet has been lost.  When left unspecified,
 the value from the account archive prevails.
 
-@item @code{bootstrap-hostnames} (type: maybe-string-list)
+@item @code{bootstrap-hostnames} (type: maybe-list-of-strings)
 A list of hostnames or IPs pointing to OpenDHT nodes, that should be
 used to initially join the OpenDHT network.  When left unspecified, the
 value from the account archive prevails.
@@ -34244,7 +34244,7 @@ Audio Services
 The group to run mpd as.
 
 The default @code{%mpd-group} is a system group with name ``mpd''.
-@item @code{shepherd-requirement} (default: @code{'()}) (type: list-of-symbol)
+@item @code{shepherd-requirement} (default: @code{'()}) (type: list-of-symbols)
 A list of symbols naming Shepherd services that this service
 will depend on.
 
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 690409b7a1..89ab9c51c6 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -137,9 +137,6 @@ (define (uglify-field-name field-name)
                                    str)
                                #\-) "_")))
 
-(define list-of-symbol?
-  (list-of symbol?))
-
 ;; Helpers for deprecated field types, to be removed later.
 (define %lazy-group (make-symbol "%lazy-group"))
 
@@ -413,7 +410,7 @@ (define-configuration mpd-configuration
    (sanitizer mpd-group-sanitizer))
 
   (shepherd-requirement
-   (list-of-symbol '())
+   (list-of-symbols '())
    "This is a list of symbols naming Shepherd services that this service
 will depend on."
    empty-serializer)
@@ -711,7 +708,7 @@ (define-configuration/no-serialization mympd-configuration
     empty-serializer)
 
   (shepherd-requirement
-   (list-of-symbol '())
+   (list-of-symbols '())
    "This is a list of symbols naming Shepherd services that this service
 will depend on."
    empty-serializer)
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 4eee5a26c2..fd9130b1ea 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -81,7 +81,9 @@ (define-module (gnu services configuration)
             interpose
             list-of
 
+            list-of-packages?
             list-of-strings?
+            list-of-symbols?
             alist?
             serialize-file-like
             text-config?
@@ -508,6 +510,10 @@ (define* (interpose ls  #:optional (delimiter "\n") (grammar 'infix))
                           (cons delimiter acc))))
               '() ls))
 
+\f
+;;;
+;;; Commonly used predicates
+
 (define (list-of pred?)
   "Return a procedure that takes a list and check if all the elements of
 the list result in @code{#t} when applying PRED? on them."
@@ -516,10 +522,20 @@ (define (list-of pred?)
           (every pred? x)
           #f)))
 
+(define list-of-packages?
+  (list-of package?))
 
 (define list-of-strings?
   (list-of string?))
 
+(define list-of-symbols?
+  (list-of symbol?))
+
+\f
+;;;
+;;; Special serializers
+;;;
+
 (define alist?
   (list-of pair?))
 
diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
index 56b7772f58..c9b5d6cd99 100644
--- a/gnu/services/telephony.scm
+++ b/gnu/services/telephony.scm
@@ -117,15 +117,10 @@ (define (string-or-computed-file? val)
   (or (string? val)
       (computed-file? val)))
 
-(define (string-list? val)
-  (and (list? val)
-       (and-map string? val)))
+(define account-fingerprint-list?
+  (list-of account-fingerprint?))
 
-(define (account-fingerprint-list? val)
-  (and (list? val)
-       (and-map account-fingerprint? val)))
-
-(define-maybe string-list)
+(define-maybe list-of-strings)
 
 (define-maybe/no-serialization account-fingerprint-list)
 
@@ -135,7 +130,7 @@ (define-maybe string)
 
 ;;; The following serializers are used to derive an account details alist from
 ;;; a <jami-account> record.
-(define (serialize-string-list _ val)
+(define (serialize-list-of-strings _ val)
   (string-join val ";"))
 
 (define (serialize-boolean _ val)
@@ -188,7 +183,7 @@ (define-configuration jami-account
 connection to the the Internet has been lost.  When left unspecified, the
 value from the account archive prevails.")
   (bootstrap-hostnames
-   maybe-string-list
+   maybe-list-of-strings
    "A list of hostnames or IPs pointing to OpenDHT nodes, that should be used
 to initially join the OpenDHT network.  When left unspecified, the value from
 the account archive prevails.")
@@ -220,9 +215,8 @@ (define (jami-account->alist jami-account-object)
 
   (list-transduce jami-account-transducer rcons jami-account-fields))
 
-(define (jami-account-list? val)
-  (and (list? val)
-       (and-map jami-account? val)))
+(define jami-account-list?
+  (list-of jami-account?))
 
 (define-maybe/no-serialization jami-account-list)
 
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 08/11] services: NetworkManager: Use define-configuration and generic-ini.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (6 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 07/11] services: configuration: Add some commonly used predicates Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 09/11] services: NetworkManager: Prefer package over network-manager Bruno Victal
                     ` (4 subsequent siblings)
  12 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/networking.scm (<network-manager-configuration>): Define with
define-configuration.
(warn-iwd?-field-deprecation): Use regular define.
(network-manager-accounts): Use match-record.
(network-manager-environment): Subsume logic from vpn-plugin-directory.
(network-manager-shepherd-service): Subsume logic from
network-manager-activation.
(vpn-plugin-directory, network-manager-activation): Remove.
(network-manager-service-type): Adjust to changes listed above.
---
 gnu/services/networking.scm | 199 +++++++++++++++++++++++-------------
 1 file changed, 127 insertions(+), 72 deletions(-)

diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 5657b141d9..a4d3affa6c 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -41,6 +41,7 @@ (define-module (gnu services networking)
   #:use-module (gnu services)
   #:use-module (gnu services base)
   #:use-module (gnu services configuration)
+  #:use-module (gnu services configuration generic-ini)
   #:use-module (gnu services linux)
   #:use-module (gnu services shepherd)
   #:use-module (gnu services dbus)
@@ -1157,92 +1158,145 @@ (define-record-type* <modem-manager-configuration>
 ;;;
 
 ;; TODO: deprecated field, remove later.
-(define-with-syntax-properties (warn-iwd?-field-deprecation
-                                (value properties))
+(define (warn-iwd?-field-deprecation value)
   (when value
-    (warning (source-properties->location properties)
-             (G_ "the 'iwd?' field is deprecated, please use \
+    (warning (G_ "the 'iwd?' field is deprecated, please use \
 'shepherd-requirement' field instead~%")))
   value)
 
-(define-record-type* <network-manager-configuration>
-  network-manager-configuration make-network-manager-configuration
-  network-manager-configuration?
-  (network-manager network-manager-configuration-network-manager
-                   (default network-manager))
-  (shepherd-requirement network-manager-configuration-shepherd-requirement
-                        (default '(wpa-supplicant)))
-  (dns network-manager-configuration-dns
-       (default "default"))
-  (vpn-plugins network-manager-configuration-vpn-plugins ;list of file-like
-               (default '()))
-  (iwd? network-manager-configuration-iwd?  ; TODO: deprecated field, remove.
-        (default #f)
-        (sanitize warn-iwd?-field-deprecation)))
+(define-configuration network-manager-configuration
+  (network-manager
+   (package network-manager)
+   "The NetworkManager package to use."
+   empty-serializer)
+
+  (shepherd-requirement
+   (list-of-symbols '(wpa-supplicant))
+   "This option can be used to provide a list of symbols naming Shepherd
+services that this service will depend on, such as @code{'wpa-supplicant} or
+@code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
+networks."
+   empty-serializer)
+
+  (dns
+   (string "default")
+   "Processing mode for DNS, which affects how NetworkManager uses the
+@code{resolv.conf} configuration file.
+
+@table @samp
+@item default
+NetworkManager will update @code{resolv.conf} to reflect the nameservers
+provided by currently active connections.
+
+@item dnsmasq
+NetworkManager will run @code{dnsmasq} as a local caching nameserver, using a
+@dfn{conditional forwarding} configuration if you are connected to a VPN, and
+then update @code{resolv.conf} to point to the local nameserver.
+
+With this setting, you can share your network connection.  For example when
+you want to share your network connection to another laptop @i{via} an
+Ethernet cable, you can open @command{nm-connection-editor} and configure the
+Wired connection's method for IPv4 and IPv6 to be ``Shared to other computers''
+and reestablish the connection (or reboot).
+
+You can also set up a @dfn{host-to-guest connection} to QEMU VMs
+(@pxref{Installing Guix in a VM}).  With a host-to-guest connection, you can
+e.g.@: access a Web server running on the VM (@pxref{Web Services}) from a Web
+browser on your host system, or connect to the VM @i{via} SSH
+(@pxref{Networking Services, @code{openssh-service-type}}).  To set up a
+host-to-guest connection, run this command once:
 
-(define (network-manager-activation config)
-  ;; Activation gexp for NetworkManager
-  (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins)
-    #~(begin
-        (use-modules (guix build utils))
-        (mkdir-p "/etc/NetworkManager/system-connections")
-        #$@(if (equal? dns "dnsmasq")
-               ;; create directory to store dnsmasq lease file
-               '((mkdir-p "/var/lib/misc"))
-               '()))))
+@example
+nmcli connection add type tun \
+ connection.interface-name tap0 \
+ tun.mode tap tun.owner $(id -u) \
+ ipv4.method shared \
+ ipv4.addresses 172.28.112.1/24
+@end example
 
-(define (vpn-plugin-directory plugins)
-  "Return a directory containing PLUGINS, the NM VPN plugins."
-  (directory-union "network-manager-vpn-plugins" plugins))
+Then each time you launch your QEMU VM (@pxref{Running Guix in a VM}), pass
+@option{-nic tap,ifname=tap0,script=no,downscript=no} to
+@command{qemu-system-...}.
+
+@item none
+NetworkManager will not modify @code{resolv.conf}.
+@end table"
+   (serializer-options '(#:section main)))
+
+  (vpn-plugins
+   (list-of-packages '())
+   "This is the list of available plugins for virtual private networks
+(VPNs).  An example of this is the @code{network-manager-openvpn}
+package, which allows NetworkManager to manage VPNs @i{via} OpenVPN."
+   empty-serializer)
+
+  ;; Deprecated options
+  (iwd?
+   (boolean #f)
+   "Deprecated."
+   (sanitizer warn-iwd?-field-deprecation)
+   (serializer-options '(#:section device))
+   (serializer
+    (lambda (_ value . rest)
+      (let ((value (if value "iwd" "wpa_supplicant")))
+        (apply generic-ini-serialize-string
+               'wifi.backend value rest)))))
+
+  (prefix generic-ini-))
+
+(define (network-manager-serialize-configuration config)
+  (mixed-text-file
+   "NetworkManager.conf"
+   (serialize-ini-configuration config
+                                network-manager-configuration-fields)))
 
 (define (network-manager-accounts config)
   "Return the list of <user-account> and <user-group> for CONFIG."
-  (define nologin
-    (file-append shadow "/sbin/nologin"))
-
-  (define accounts
-    (append-map (lambda (package)
-                  (map (lambda (name)
-                         (user-account (system? #t)
-                                       (name name)
-                                       (group "network-manager")
-                                       (comment "NetworkManager helper")
-                                       (home-directory "/var/empty")
-                                       (create-home-directory? #f)
-                                       (shell nologin)))
-                       (or (assoc-ref (package-properties package)
-                                      'user-accounts)
-                           '())))
-                (network-manager-configuration-vpn-plugins config)))
-
-  (match accounts
-    (()
-     '())
-    (_
-     (cons (user-group (name "network-manager") (system? #t))
-           accounts))))
+  (match-record config <network-manager-configuration>
+    (vpn-plugins)
+    (let* ((nologin (file-append shadow "/sbin/nologin"))
+           (accounts
+            (append-map (lambda (package)
+                          (map (lambda (name)
+                                 (user-account
+                                  (system? #t)
+                                  (name name)
+                                  (group "network-manager")
+                                  (comment "NetworkManager helper")
+                                  (home-directory "/var/empty")
+                                  (create-home-directory? #f)
+                                  (shell nologin)))
+                               (or (assoc-ref (package-properties package)
+                                              'user-accounts)
+                                   '())))
+                        vpn-plugins)))
+      (cond
+       ((null? accounts) '())
+       (else (cons (user-group (name "network-manager")
+                               (system? #t))
+                   accounts))))))
 
 (define (network-manager-environment config)
+  "Define NM_VPN_PLUGIN_DIR variable in the global environment such that
+\"nmcli connection import type openvpn file foo.ovpn\" works."
   (match-record config <network-manager-configuration>
-    (network-manager dns vpn-plugins)
-    ;; Define this variable in the global environment such that
-    ;; "nmcli connection import type openvpn file foo.ovpn" works.
-    `(("NM_VPN_PLUGIN_DIR"
-       . ,(file-append (vpn-plugin-directory vpn-plugins)
-                       "/lib/NetworkManager/VPN")))))
+    (vpn-plugins)
+    (let ((plugin-union (directory-union "network-manager-vpn-plugins"
+                                         vpn-plugins)))
+      `(("NM_VPN_PLUGIN_DIR" . ,(file-append plugin-union
+                                             "/lib/NetworkManager/VPN"))))))
 
 (define (network-manager-shepherd-service config)
   (match-record config <network-manager-configuration>
-    (network-manager shepherd-requirement dns vpn-plugins iwd?)
+    (network-manager shepherd-requirement dns iwd?)
     (let* ((iwd? (or iwd?  ; TODO: deprecated field, remove later.
                      (and shepherd-requirement
                           (memq 'iwd shepherd-requirement))))
-           (conf (plain-file "NetworkManager.conf"
-                             (string-append
-                              "[main]\ndns=" dns "\n"
-                              (if iwd? "[device]\nwifi.backend=iwd\n" ""))))
-           (vpn  (vpn-plugin-directory vpn-plugins)))
+           (conf (network-manager-serialize-configuration config))
+           (vpn-plugin-env (map (match-lambda
+                                  ((key . value)
+                                   #~(string-append #$key "=" #$value)))
+                                (network-manager-environment config))))
       (list (shepherd-service
              (documentation "Run the NetworkManager.")
              (provision '(NetworkManager networking))
@@ -1254,6 +1308,10 @@ (define (network-manager-shepherd-service config)
              (actions (list (shepherd-configuration-action conf)))
              (start
               #~(lambda _
+                  (mkdir-p "/etc/NetworkManager/system-connections")
+                  ;; Create directory to store dnsmasq lease file.
+                  #$@(if (equal? dns "dnsmasq")
+                         '((mkdir-p "/var/lib/misc")) '())
                   (let ((pid
                          (fork+exec-command
                           (list #$(file-append network-manager
@@ -1261,8 +1319,7 @@ (define (network-manager-shepherd-service config)
                                 (string-append "--config=" #$conf)
                                 "--no-daemon")
                           #:environment-variables
-                          (list (string-append "NM_VPN_PLUGIN_DIR=" #$vpn
-                                               "/lib/NetworkManager/VPN")
+                          (list #$@vpn-plugin-env
                                 ;; Override non-existent default users
                                 "NM_OPENVPN_USER="
                                 "NM_OPENVPN_GROUP="
@@ -1301,8 +1358,6 @@ (define network-manager-service-type
                                 network-manager-configuration-network-manager))
             (service-extension account-service-type
                                network-manager-accounts)
-            (service-extension activation-service-type
-                               network-manager-activation)
             (service-extension session-environment-service-type
                                network-manager-environment)
             ;; Add network-manager to the system profile.
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 09/11] services: NetworkManager: Prefer package over network-manager.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (7 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 08/11] services: NetworkManager: Use define-configuration and generic-ini Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-10-02 16:52     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 10/11] services: NetworkManager: add log-configuration field Bruno Victal
                     ` (3 subsequent siblings)
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/networking.scm (<network-manager-configuration>)
[package]: New field.
[network-manager]: Deprecate field.
(network-manager-environment, network-manager-shepherd)
(network-manager-service-type): Adjust to use 'package'.
* doc/guix.texi (Networking Setup): Replace mentions of network-manager with
package.
---
 doc/guix.texi               |  2 +-
 gnu/services/networking.scm | 41 +++++++++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 2f7e734874..974bfa3fb0 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -20352,7 +20352,7 @@ Networking Setup
 Data type representing the configuration of NetworkManager.
 
 @table @asis
-@item @code{network-manager} (default: @code{network-manager})
+@item @code{package} (default: @code{network-manager})
 The NetworkManager package to use.
 
 @item @code{shepherd-requirement} (default: @code{'(wpa-supplicant)})
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index a4d3affa6c..496ff0f0ec 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -166,6 +166,7 @@ (define-module (gnu services networking)
 
             network-manager-configuration
             network-manager-configuration?
+            network-manager-configuration-package
             network-manager-configuration-shepherd-requirement
             network-manager-configuration-dns
             network-manager-configuration-vpn-plugins
@@ -1164,8 +1165,10 @@ (define (warn-iwd?-field-deprecation value)
 'shepherd-requirement' field instead~%")))
   value)
 
+(define-maybe/no-serialization package)
+
 (define-configuration network-manager-configuration
-  (network-manager
+  (package
    (package network-manager)
    "The NetworkManager package to use."
    empty-serializer)
@@ -1242,6 +1245,17 @@ (define-configuration network-manager-configuration
         (apply generic-ini-serialize-string
                'wifi.backend value rest)))))
 
+  (network-manager
+   maybe-package
+   "Deprecated. Use ``package'' field instead."
+   (sanitizer
+    (lambda (value)
+      (when (maybe-value-set? value)
+        (warning (G_ "the 'network-manager' field is deprecated, please use \
+'package' field instead~%")))
+      value))
+   empty-serializer)
+
   (prefix generic-ini-))
 
 (define (network-manager-serialize-configuration config)
@@ -1288,10 +1302,11 @@ (define (network-manager-environment config)
 
 (define (network-manager-shepherd-service config)
   (match-record config <network-manager-configuration>
-    (network-manager shepherd-requirement dns iwd?)
+    (package shepherd-requirement dns iwd?)
     (let* ((iwd? (or iwd?  ; TODO: deprecated field, remove later.
                      (and shepherd-requirement
                           (memq 'iwd shepherd-requirement))))
+           (package (maybe-value network-manager package))
            (conf (network-manager-serialize-configuration config))
            (vpn-plugin-env (map (match-lambda
                                   ((key . value)
@@ -1314,7 +1329,7 @@ (define (network-manager-shepherd-service config)
                          '((mkdir-p "/var/lib/misc")) '())
                   (let ((pid
                          (fork+exec-command
-                          (list #$(file-append network-manager
+                          (list #$(file-append package
                                                "/sbin/NetworkManager")
                                 (string-append "--config=" #$conf)
                                 "--no-daemon")
@@ -1332,7 +1347,7 @@ (define (network-manager-shepherd-service config)
                     ;; to finish starting-up. This is required otherwise
                     ;; services will fail since the network interfaces be
                     ;; absent until NetworkManager finishes setting them up.
-                    (system* #$(file-append network-manager "/bin/nm-online")
+                    (system* #$(file-append package "/bin/nm-online")
                              "--wait-for-startup" "--quiet")
                     ;; XXX: Finally, return the pid from running
                     ;; fork+exec-command to shepherd.
@@ -1342,10 +1357,18 @@ (define (network-manager-shepherd-service config)
 (define network-manager-service-type
   (let ((config->packages
          (lambda (config)
-          (match-record config <network-manager-configuration>
-            (network-manager vpn-plugins)
-            `(,network-manager ,@vpn-plugins)))))
-
+           (match-record config <network-manager-configuration>
+                         (package network-manager vpn-plugins)
+             (let ((package (or (maybe-value network-manager)
+                               package)))
+               `(,package ,@vpn-plugins)))))
+        ;; Handle network-manager field deprecation for
+        ;; polkit-service-type extension.
+        (network-manager-configuration-package*
+         (lambda (config)
+           (match-record config <network-manager-configuration>
+                         (package network-manager)
+             (maybe-value network-manager package)))))
     (service-type
      (name 'network-manager)
      (extensions
@@ -1355,7 +1378,7 @@ (define network-manager-service-type
             (service-extension polkit-service-type
                                (compose
                                 list
-                                network-manager-configuration-network-manager))
+                                network-manager-configuration-package*))
             (service-extension account-service-type
                                network-manager-accounts)
             (service-extension session-environment-service-type
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 10/11] services: NetworkManager: add log-configuration field.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (8 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 09/11] services: NetworkManager: Prefer package over network-manager Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-10-05 16:57     ` Maxim Cournoyer
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 11/11] services: NetworkManager: Add extra-options field Bruno Victal
                     ` (2 subsequent siblings)
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/networking.scm (network-manager-log-level?)
(network-manager-log-domain?, network-manager-log-domains?): New predicate.
(serialize-network-manager-log-level, serialize-network-manager-log-domains):
New procedure.
(network-manager-log-configuration): New record type.
(network-manager-configuration)[log-configuration]: New field.
* doc/guix.texi (Networking Setup): Document it.
---
 doc/guix.texi               |  43 +++++++++++++++
 gnu/services/networking.scm | 107 ++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 974bfa3fb0..76bd1b1413 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -20361,6 +20361,10 @@ Networking Setup
 @code{'iwd} if you require authenticated access for encrypted WiFi or Ethernet
 networks.
 
+@item @code{log-configuration} (default: @code{(network-manager-log-configuration)})
+Logging configuration for NetworkManager.
+This is a @code{<network-manager-log-configuration>} record object.
+
 @item @code{dns} (default: @code{"default"})
 Processing mode for DNS, which affects how NetworkManager uses the
 @code{resolv.conf} configuration file.
@@ -20412,6 +20416,45 @@ Networking Setup
 @end table
 @end deftp
 
+@deftp {Data Type} network-manager-log-configuration
+Available @code{network-manager-log-configuration} fields are:
+
+@table @asis
+@item @code{level} (type: maybe-network-manager-log-level)
+The default logging verbosity level.  Valid values are (in increasing
+order of verbosity): @code{'off}, @code{'err}, @code{'warn},
+@code{'info}, @code{'debug} and @code{'trace}.
+
+@item @code{domains} (type: maybe-network-manager-log-domains)
+Log messages by topic.  The value for this field is a list of
+@var{domains} or pairs of @var{domains} and @var{levels} where the valid
+values for @var{levels} are the same as those described in the ``level''
+field and @var{domains} are any of: @code{'platform}, @code{'rfkill},
+@code{'ether}, @code{'wifi}, @code{'bt}, @code{'mb}, @code{'dhcp4},
+@code{'dhcp6}, @code{'ppp}, @code{'wifi-scan}, @code{'ip4}, @code{'ip6},
+@code{'autoip4}, @code{'dns}, @code{'vpn}, @code{'sharing},
+@code{'supplicant}, @code{'agents}, @code{'settings}, @code{'suspend},
+@code{'core}, @code{'device}, @code{'olpc}, @code{'wimax},
+@code{'infiniband}, @code{'firewall}, @code{'adsl}, @code{'bond},
+@code{'vlan}, @code{'bridge}, @code{'dbus-props}, @code{'team},
+@code{'concheck}, @code{'dcb}, @code{'dispatch}, @code{'audit},
+@code{'systemd}, @code{'vpn-plugin}, @code{'proxy}, @code{'none},
+@code{'all}, @code{'default}, @code{'dhcp} and @code{'ip}.  The log
+level can be overrided per-domain in a pair with a @var{level}.
+For example:
+@lisp
+(network-manager-log-configuration
+  (level 'warn)
+  (domains '(all (wifi . debug) (wifi-scan . off))))
+@end lisp
+
+@item @code{audit?} (type: maybe-boolean)
+Whether to send audit records to @command{auditd}.
+
+@end table
+@end deftp
+
+
 @cindex Connman
 @defvar connman-service-type
 This is the service type to run @url{https://01.org/connman,Connman},
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 496ff0f0ec..33ff5e040f 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -78,7 +78,10 @@ (define-module (gnu services networking)
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-43)
+  #:use-module (srfi srfi-171)
+  #:use-module (ice-9 format)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 string-fun)
   #:use-module (json)
   #:re-export (static-networking-service
                static-networking-service-type)
@@ -164,10 +167,16 @@ (define-module (gnu services networking)
             tor-hidden-service  ; deprecated
             tor-service-type
 
+            network-manager-log-configuration
+            network-manager-log-configuration?
+            network-manager-log-configuration-level
+            network-manager-log-configuration-domains?
+            network-manager-log-configuration-audit?
             network-manager-configuration
             network-manager-configuration?
             network-manager-configuration-package
             network-manager-configuration-shepherd-requirement
+            network-manager-configuration-log-configuration
             network-manager-configuration-dns
             network-manager-configuration-vpn-plugins
             network-manager-service-type
@@ -1158,6 +1167,92 @@ (define-record-type* <modem-manager-configuration>
 ;;; NetworkManager
 ;;;
 
+(define-maybe boolean)
+
+;; See the logging section at
+;; <https://networkmanager.dev/docs/api/latest/NetworkManager.conf.html> for
+;; the list of valid values for the predicates below.
+(define (network-manager-log-level? x)
+  (memq x '(off err warn info debug trace)))
+
+(define (network-manager-log-domain? x)
+  (memq x '(platform rfkill ether wifi bt mb dhcp4 dhcp6 ppp wifi-scan ip4 ip6
+                     autoip4 dns vpn sharing supplicant agents settings
+                     suspend core device olpc wimax infiniband firewall adsl
+                     bond vlan bridge dbus-props team concheck dcb dispatch
+                     audit systemd vpn-plugin proxy
+                     ;; Special NetworkManager domains:
+                     none all default dhcp ip)))
+
+(define (network-manager-log-domains? x)
+  (every
+   (match-lambda
+     (((? network-manager-log-domain?) . (? network-manager-log-level?)) #t)
+     ((? network-manager-log-domain?) #t)
+     (_ #f))
+   x))
+
+(define (serialize-network-manager-log-level field-name value)
+  `(logging level ,(format #f "~:@(~a~)" value)))
+
+(define (serialize-network-manager-log-domains field-name value)
+  (define (uglify-domain-symbol x)
+    (string-replace-substring (symbol->string x) "-" "_"))
+
+  (define serialize-entry
+    (match-lambda
+      (((= uglify-domain-symbol domain) . value)
+       (format #f "~:@(~a:~a~)" domain value))
+      ((= uglify-domain-symbol domain)
+       (format #f "~:@(~a~)" domain))))
+
+  (let ((serialized-value (list-transduce (compose (tmap serialize-entry)
+                                                   (tadd-between ","))
+                                          string-append value)))
+    `(logging domains ,serialized-value)))
+
+(define-maybe network-manager-log-level)
+(define-maybe network-manager-log-domains)
+
+;; This implicitly belongs to the INI "logging" section.
+(define-configuration network-manager-log-configuration
+  (level
+   maybe-network-manager-log-level
+   "The default logging verbosity level.  Valid values are (in increasing
+order of verbosity): @code{'off}, @code{'err}, @code{'warn}, @code{'info},
+@code{'debug} and @code{'trace}.")
+
+  (domains
+   maybe-network-manager-log-domains
+   "Log messages by topic. The value for this field is a list of @var{domains}
+or pairs of @var{domains} and @var{levels} where the valid values for
+@var{levels} are the same as those described in the ``level'' field and
+@var{domains} are any of: @code{'platform}, @code{'rfkill}, @code{'ether},
+@code{'wifi}, @code{'bt}, @code{'mb}, @code{'dhcp4}, @code{'dhcp6},
+@code{'ppp}, @code{'wifi-scan}, @code{'ip4}, @code{'ip6}, @code{'autoip4},
+@code{'dns}, @code{'vpn}, @code{'sharing}, @code{'supplicant}, @code{'agents},
+@code{'settings}, @code{'suspend}, @code{'core}, @code{'device}, @code{'olpc},
+@code{'wimax}, @code{'infiniband}, @code{'firewall}, @code{'adsl}, @code{'bond},
+@code{'vlan}, @code{'bridge}, @code{'dbus-props}, @code{'team}, @code{'concheck},
+@code{'dcb}, @code{'dispatch}, @code{'audit}, @code{'systemd},
+@code{'vpn-plugin}, @code{'proxy}, @code{'none}, @code{'all}, @code{'default},
+@code{'dhcp} and @code{'ip}.
+
+The log level can be overrided per-domain in a pair with a @var{level}.
+For example:
+@lisp
+(network-manager-log-configuration
+  (level 'warn)
+  (domains '(all (wifi . debug) (wifi-scan . off))))
+@end lisp")
+
+  (audit?
+   maybe-boolean
+   "Whether to send audit records to @command{auditd}."
+   (serializer generic-ini-serialize-boolean)
+   (serializer-options `(#:section logging
+                         #:field-name-transform ,(const 'audit)))))
+
 ;; TODO: deprecated field, remove later.
 (define (warn-iwd?-field-deprecation value)
   (when value
@@ -1181,6 +1276,18 @@ (define-configuration network-manager-configuration
 networks."
    empty-serializer)
 
+  (log-configuration
+   (network-manager-log-configuration (network-manager-log-configuration))
+   "Logging configuration for NetworkManager.  This is a
+@code{<network-manager-log-configuration>} record object."
+   (serializer
+    (lambda (_ value)
+      ;; Wrap the serialization of the log-configuration which is a list
+      ;; of INI entries in a ‘ini-entries’ object.
+      (ini-entries (list-transduce
+                    (base-transducer value) rcons
+                    network-manager-log-configuration-fields)))))
+
   (dns
    (string "default")
    "Processing mode for DNS, which affects how NetworkManager uses the
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 11/11] services: NetworkManager: Add extra-options field.
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (9 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 10/11] services: NetworkManager: add log-configuration field Bruno Victal
@ 2023-06-26 21:59   ` Bruno Victal
  2023-10-05 16:59     ` Maxim Cournoyer
  2023-06-27  4:20   ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Liliana Marie Prikler
  2023-09-16 21:22   ` Bruno Victal
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-06-26 21:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/networking.scm (network-manager-configuration)
[extra-options]: New field.
* doc/guix.texi (Networking Setup): Document it.
---
 doc/guix.texi               | 7 +++++++
 gnu/services/networking.scm | 9 +++++++++
 2 files changed, 16 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 76bd1b1413..a74718f216 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -20413,6 +20413,13 @@ Networking Setup
 (VPNs).  An example of this is the @code{network-manager-openvpn}
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN.
 
+@item @code{extra-options} (default: @code{'()}) (type: list-of-ini-entries)
+Additional options to be appended to @file{NetworkManager.conf} (run
+@samp{man networkmanager.conf} for more information).  It expects a list
+whose elements are lists of the form @code{'(@var{section} @var{key}
+@var{value})}, where @var{section} and @var{key} are symbols and
+@var{value} is a string.
+
 @end table
 @end deftp
 
diff --git a/gnu/services/networking.scm b/gnu/services/networking.scm
index 33ff5e040f..4cb1cd60cb 100644
--- a/gnu/services/networking.scm
+++ b/gnu/services/networking.scm
@@ -179,6 +179,7 @@ (define-module (gnu services networking)
             network-manager-configuration-log-configuration
             network-manager-configuration-dns
             network-manager-configuration-vpn-plugins
+            network-manager-configuration-extra-options
             network-manager-service-type
 
             connman-configuration
@@ -1340,6 +1341,14 @@ (define-configuration network-manager-configuration
 package, which allows NetworkManager to manage VPNs @i{via} OpenVPN."
    empty-serializer)
 
+  (extra-options
+   (list-of-ini-entries '())
+   "Additional options to be appended to @file{NetworkManager.conf}  (run
+@samp{man networkmanager.conf} for more information).
+It expects a list whose elements are lists of the form
+@code{'(@var{section} @var{key} @var{value})}, where @var{section} and
+@var{key} are symbols and @var{value} is a string.")
+
   ;; Deprecated options
   (iwd?
    (boolean #f)
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 00/11] Service subsystem improvements
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (10 preceding siblings ...)
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 11/11] services: NetworkManager: Add extra-options field Bruno Victal
@ 2023-06-27  4:20   ` Liliana Marie Prikler
  2023-09-16 21:22   ` Bruno Victal
  12 siblings, 0 replies; 50+ messages in thread
From: Liliana Marie Prikler @ 2023-06-27  4:20 UTC (permalink / raw)
  To: Bruno Victal, 63985; +Cc: ludo, maxim.cournoyer

Am Montag, dem 26.06.2023 um 22:57 +0100 schrieb Bruno Victal:
> ** Renamed fields: 'network-manager' to 'package'
> 
> Naming the field 'package' is more informative & less confusing than
> a reduplication of the package-name with the field-name itself.
This goes against established practice in pretty much every other
service out there.  It'd also be way more painful to read if it was
applied broadly – so many fields simply named "package" in your
configuration might be easier to grep, but certainly not easier to
understand.  As such, I'd suggest to revert this particular change.

As for the other changes I'll have a detailed look later.

Cheers




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 00/11] Service subsystem improvements
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
                     ` (11 preceding siblings ...)
  2023-06-27  4:20   ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Liliana Marie Prikler
@ 2023-09-16 21:22   ` Bruno Victal
  2023-09-16 21:55     ` Liliana Marie Prikler
  12 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-09-16 21:22 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985, ludo, liliana.prikler, maxim.cournoyer

Hi all,

I've been pondering about the changes here and would like to comment
on them:

On 2023-06-26 22:57, Bruno Victal wrote:
> Bruno Victal (11):
>   services: configuration: Simplify normalize-extra-args.
>   services: configuration: Use transducers within
>     serialize-configuration.
>   services: fstrim-service-type: Serialize with SRFI-171 transducers.
>   doc: Rewrite define-configuration.
>   services: configuration: Add serializer-options field.

I think these changes are OK on their own since they add some extra
flexibility to the serialize-configuration procedure and address a
TODO item.

>   services: configuration: New generic-ini module.
>   services: configuration: Add some commonly used predicates.

IMO I'm afraid this might be somewhat short-sighted and would be
better addressed directly in Guile by implementing SRFI-233, perhaps
by doing some adaptations to the approach taken here.

>   services: NetworkManager: Use define-configuration and generic-ini.>   services: NetworkManager: Prefer package over network-manager.
>   services: NetworkManager: add log-configuration field.
>   services: NetworkManager: Add extra-options field.

Naturally these are no longer relevant if this generic-ini module
approach is abandoned.

Comments?


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

Cheers,
Bruno.





^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 00/11] Service subsystem improvements
  2023-09-16 21:22   ` Bruno Victal
@ 2023-09-16 21:55     ` Liliana Marie Prikler
  2023-09-23 13:35       ` Bruno Victal
  2023-09-25 14:06       ` Ludovic Courtès
  0 siblings, 2 replies; 50+ messages in thread
From: Liliana Marie Prikler @ 2023-09-16 21:55 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985, ludo, maxim.cournoyer

Am Samstag, dem 16.09.2023 um 22:22 +0100 schrieb Bruno Victal:
> Hi all,
> 
> I've been pondering about the changes here and would like to comment
> on them:
> 
> On 2023-06-26 22:57, Bruno Victal wrote:
> > Bruno Victal (11):
> >   services: configuration: Simplify normalize-extra-args.
> >   services: configuration: Use transducers within
> >     serialize-configuration.
> >   services: fstrim-service-type: Serialize with SRFI-171
> > transducers.
> >   doc: Rewrite define-configuration.
> >   services: configuration: Add serializer-options field.
> 
> I think these changes are OK on their own since they add some extra
> flexibility to the serialize-configuration procedure and address a
> TODO item.
I'm not sure whether serializer options really add much value.  You can
use functional programming to define serializers for you and pass those
options in a cleaner way IMHO.  The documentation should be updated as
the changes are made.  As for the switch to SRFI 171, I'm not sure
whether backwards compatibility with Guile 2.2 is a requirement
somewhere; if it isn't, that change is probably fine.

> >   services: configuration: New generic-ini module.
> >   services: configuration: Add some commonly used predicates.
> 
> IMO I'm afraid this might be somewhat short-sighted and would be
> better addressed directly in Guile by implementing SRFI-233, perhaps
> by doing some adaptations to the approach taken here.
Even if Guile implemented SRFI 233 now, I'm not sure we could use it
tomorrow.  And even once we can use SRFI 233, we can keep backwards-
compatibility be re-exporting things.  The question is how necessary it
will be for us to maintain our own INI format writer.  NetworkManager
is one use case, but perhaps we have others (perhaps even in the gnome
world – gdm maybe?)

> >   services: NetworkManager: Use define-configuration and generic-
> > ini.>   services: NetworkManager: Prefer package over network-
> > manager.
> >   services: NetworkManager: add log-configuration field.
> >   services: NetworkManager: Add extra-options field.
> 
> Naturally these are no longer relevant if this generic-ini module
> approach is abandoned.
I think we can still upgrade this to define-configuration without a
generic-ini, but see above.  That being said, we can certainly split
this into two series at the point you currently feel comfortable with
and work from there.

WDYT?




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 00/11] Service subsystem improvements
  2023-09-16 21:55     ` Liliana Marie Prikler
@ 2023-09-23 13:35       ` Bruno Victal
  2023-09-23 15:22         ` Liliana Marie Prikler
  2023-09-25 14:06       ` Ludovic Courtès
  1 sibling, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-09-23 13:35 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: 63985, ludo, maxim.cournoyer

On 2023-09-16 22:55, Liliana Marie Prikler wrote:
> I'm not sure whether serializer options really add much value.  You can
> use functional programming to define serializers for you and pass those
> options in a cleaner way IMHO.  The documentation should be updated as
> the changes are made.  As for the switch to SRFI 171, I'm not sure
> whether backwards compatibility with Guile 2.2 is a requirement
> somewhere; if it isn't, that change is probably fine.

Is SRFI-171 not available in Guile 2.2?
Your last remark surprised me though: is Guix not running with Guile 3.0?
I was the impression that this was the case since otherwise wouldn't this
imply that `spawn' & co. can't be used anywhere?

> Even if Guile implemented SRFI 233 now, I'm not sure we could use it
> tomorrow.  And even once we can use SRFI 233, we can keep backwards-
> compatibility be re-exporting things. (…)

In that aspect I'm willing to be patient vs. maintaining a temporary
(read as: permanent) code though I think that it should be possible to
make it backwards-compatible, even if we completely gut out its innards
later and replace them with SRFI-233.

> (…) The question is how necessary it
> will be for us to maintain our own INI format writer.  NetworkManager
> is one use case, but perhaps we have others (perhaps even in the gnome
> world – gdm maybe?)

Certainly there are many applications that make use of INI-like files
for configuration and for INI ones it would be convenient, though I
should caution that there are many things that can look like INI but
aren't:

* NetworkManager accepts some entries that have append behavior via
'KW += val' and have repetition. In some cases I think the ordering
matters too. (Since our define-configuration definition for it
doesn't attempt to fully cover every nook and cranny of it I think
using INI here doesn't hurt.)

* TOML

* Files that can have leading entries but without a section. These
can be thought to belong to some top level but invisible section yet
the generic-ini doesn't handle these. (yet)


There's some assumptions I made while writing generic-ini which make
it not as generic as imparted by its name and as such, it can only be
used in the following conditions:

* The ordering of the entries and sections doesn't matter.
* Every entry belongs to a section.
* (… perhaps more? …)

>> Naturally these are no longer relevant if this generic-ini module
>> approach is abandoned.
> I think we can still upgrade this to define-configuration without a
> generic-ini, but see above.  That being said, we can certainly split> this into two series (…)

Sure.

> (…) at the point you currently feel comfortable with
> and work from there.
> 
> WDYT?

I'm inclined to write-off the generic-ini though as discussed above,
there's some demand for some kind of INI format writer so personally
I'd be OK with temporarily maintaining this writer if we can really
make it an experiment/true to the word “temporary” thing. This would
mean that:

* It should be only used internally by services living in Guix
repository. I'm OK with going around and reworking/replacing usages
of it when the time comes to retire it/when guile gets this INI thing
natively. (i.e. #:export (…) doesn't mean that I'm intending it to
be used outside of the repo with stability promises.)


WDYT?

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

Cheers,
Bruno.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 00/11] Service subsystem improvements
  2023-09-23 13:35       ` Bruno Victal
@ 2023-09-23 15:22         ` Liliana Marie Prikler
  0 siblings, 0 replies; 50+ messages in thread
From: Liliana Marie Prikler @ 2023-09-23 15:22 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985, ludo, maxim.cournoyer

Am Samstag, dem 23.09.2023 um 14:35 +0100 schrieb Bruno Victal:
> On 2023-09-16 22:55, Liliana Marie Prikler wrote:
> > I'm not sure whether serializer options really add much value.  You
> > can use functional programming to define serializers for you and
> > pass those options in a cleaner way IMHO.  The documentation should
> > be updated as the changes are made.  As for the switch to SRFI 171,
> > I'm not sure whether backwards compatibility with Guile 2.2 is a
> > requirement somewhere; if it isn't, that change is probably fine.
> 
> Is SRFI-171 not available in Guile 2.2?
> Your last remark surprised me though: is Guix not running with Guile
> 3.0?  I was the impression that this was the case since otherwise
> wouldn't this imply that `spawn' & co. can't be used anywhere?
Nope, it was introduced with 3.0.  Again, "I'm not sure" meaning "I
don't know".  Would be nice to have another reviewer check this.  
(Looking at CC:) Maxim? Ludo?

> 
> > (…) The question is how necessary it
> > will be for us to maintain our own INI format writer. 
> > NetworkManager is one use case, but perhaps we have others (perhaps
> > even in the gnome world – gdm maybe?)
> 
> Certainly there are many applications that make use of INI-like files
> for configuration and for INI ones it would be convenient, though I
> should caution that there are many things that can look like INI but
> aren't:
> 
> * NetworkManager accepts some entries that have append behavior via
> 'KW += val' and have repetition. In some cases I think the ordering
> matters too. (Since our define-configuration definition for it
> doesn't attempt to fully cover every nook and cranny of it I think
> using INI here doesn't hurt.)
> 
> * TOML
> 
> * Files that can have leading entries but without a section. These
> can be thought to belong to some top level but invisible section yet
> the generic-ini doesn't handle these. (yet)
> 
> 
> There's some assumptions I made while writing generic-ini which make
> it not as generic as imparted by its name and as such, it can only be
> used in the following conditions:
> 
> * The ordering of the entries and sections doesn't matter.
> * Every entry belongs to a section.
> * (… perhaps more? …)
Some of these look like bugs, others are a result of trying to cover
too much.  Let's perhaps just cover the simple case of 

[maybe a section]
<var> <op> <val>

where <op> will almost always be "=" rather than worrying about the
specification of e.g. TOML.  If needed, a TOML-specific module can
hopefully reuse the procedures by which we produce INI files.

> > 
> > (…) at the point you currently feel comfortable with
> > and work from there.
> > 
> > WDYT?
> 
> I'm inclined to write-off the generic-ini though as discussed above,
> there's some demand for some kind of INI format writer so personally
> I'd be OK with temporarily maintaining this writer if we can really
> make it an experiment/true to the word “temporary” thing. This would
> mean that:
> 
> * It should be only used internally by services living in Guix
> repository. I'm OK with going around and reworking/replacing usages
> of it when the time comes to retire it/when guile gets this INI thing
> natively. (i.e. #:export (…) doesn't mean that I'm intending it to
> be used outside of the repo with stability promises.)
> 
> 
> WDYT?
Do you have commit access?  The only real place where you can
experiment in the (guix) namespace is on feature branches and if you
feel like you need to experiment further, I'd recommend doing so.  If
not, you could roll out a channel with an extension like the one that
was uses for (guix home)¹.  You might also want to reach out to guix-
devel to try and explain your approach to everyone in terms of how it
would simplify writing services.

Cheers

¹ I think the Guix Home thing itself shows that you can put
technological previews to Guix itself and have them tested (and
depended on!) by many.  This may or may not be what you want, there
sadly isn't a "clean" option.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 00/11] Service subsystem improvements
  2023-09-16 21:55     ` Liliana Marie Prikler
  2023-09-23 13:35       ` Bruno Victal
@ 2023-09-25 14:06       ` Ludovic Courtès
  1 sibling, 0 replies; 50+ messages in thread
From: Ludovic Courtès @ 2023-09-25 14:06 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: 63985, Bruno Victal, maxim.cournoyer

Hello!

Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:

> I'm not sure whether serializer options really add much value.  You can
> use functional programming to define serializers for you and pass those
> options in a cleaner way IMHO.  The documentation should be updated as
> the changes are made.  As for the switch to SRFI 171, I'm not sure
> whether backwards compatibility with Guile 2.2 is a requirement
> somewhere; if it isn't, that change is probably fine.

Backward compatibility with 2.2 is not required in the service code.

(The places where compatibility with 2.2 or even 2.0 may be required are
some of the (guix build …) modules and core (guix …) modules, the latter
so that a very old Guix can still pull the new one.)

Ludo’.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 09/11] services: NetworkManager: Prefer package over network-manager Bruno Victal
@ 2023-10-02 16:52     ` Maxim Cournoyer
  0 siblings, 0 replies; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-02 16:52 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> * gnu/services/networking.scm (<network-manager-configuration>)
> [package]: New field.
> [network-manager]: Deprecate field.
> (network-manager-environment, network-manager-shepherd)
> (network-manager-service-type): Adjust to use 'package'.
> * doc/guix.texi (Networking Setup): Replace mentions of network-manager with
> package.

Like Liliana, I don't see the added value here; it goes against the
current convention which makes things worst IMO.  I'd drop it.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args Bruno Victal
@ 2023-10-02 17:00     ` Maxim Cournoyer
  2023-10-07 12:36       ` [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args. (was: bug#63985: [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration) Bruno Victal
  0 siblings, 1 reply; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-02 17:00 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> * gnu/services/configuration.scm
> (define-configuration-helper, normalize-extra-args): Use #f instead of %unset-value.
> ---
>  gnu/services/configuration.scm | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
> index 367b85c1be..dafe72f4fe 100644
> --- a/gnu/services/configuration.scm
> +++ b/gnu/services/configuration.scm
> @@ -190,32 +190,32 @@ (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))
> +               (sanitizer* #f)
> +               (serializer* #f))
>        (syntax-case s (sanitizer serializer empty-serializer)
>          (((sanitizer proc) tail ...)
> -         (if (maybe-value-set? sanitizer*)
> -             (syntax-violation 'sanitizer "duplicate entry"
> -                               #'proc)
> +         (if 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)
> +         (if serializer*
> +             (syntax-violation 'serializer
> +                               "duplicate or conflicting entry" #'proc)
>               (loop #'(tail ...) sanitizer* #'proc)))
>          ((empty-serializer tail ...)
> -         (if (maybe-value-set? serializer*)
> +         (if serializer*
>               (syntax-violation 'empty-serializer
>                                 "duplicate or conflicting entry" #f)
>               (loop #'(tail ...) sanitizer* #'empty-serializer)))
>          (()  ; stop condition

The above LGTM.

>           (values (list sanitizer* serializer*)))
>          ((proc)  ; TODO: deprecated, to be removed.
> -         (null? (filter-map maybe-value-set? (list sanitizer* serializer*)))
> +         (every not (list sanitizer* serializer*))

Alternatively, using DeMorgan's law:  (not (or sanitizer* serializer*))

>           (begin
>             (warning #f (G_ "specifying serializers after documentation is \
>  deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
> -           (values (list %unset-value #'proc)))))))
> +           (values (list #f #'proc)))))))
>  
>    (syntax-case syn ()
>      ((_ stem (field field-type+def doc extra-args ...) ...)
> @@ -239,11 +239,11 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
>                       default-value))
>                    #'((field-type def) ...)))
>              ((field-sanitizer ...)
> -             (map maybe-value #'(sanitizer* ...)))
> +             #'(sanitizer* ...))
>              ((field-serializer ...)
>               (map (lambda (type proc)
>                      (and serialize?
> -                         (or (maybe-value proc)
> +                         (or proc

I haven't applied it locally so may be out of context, but how do we
ensure here that sanitizer and proc aren't set to #f before calling
them?

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration Bruno Victal
@ 2023-10-02 17:25     ` Maxim Cournoyer
  2023-10-07 13:39       ` [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration. (was : bug#63985: [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration) Bruno Victal
  0 siblings, 1 reply; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-02 17:25 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hello,

Bruno Victal <mirai@makinata.eu> writes:

> Introduces 'base-transducer', a SRFI-171 based transducer that can be used as a
> starting point for writing custom configuration record serializing procedures.
>
> This also fixes the symbol maybe-value serialization test case.
>
> * gnu/services/configuration.scm (empty-serializer?): New predicate.
> (base-transducer, tfilter-maybe-value): New procedure.
> (serialize-configuration): Adapt to use base-transducer.
>
> * gnu/services/telephony.scm (jami-account->alist): Use transducers to skip
> fields that are unserializable or whose field maybe-value is unset.
>
> * tests/services/configuration.scm: Remove test-expect-fail.

Pretty cool stuff!

> ---
>  gnu/services/configuration.scm   | 29 ++++++++++++++++++++++++-----
>  gnu/services/telephony.scm       | 27 +++++++++++++--------------
>  tests/services/configuration.scm |  6 +-----
>  3 files changed, 38 insertions(+), 24 deletions(-)
>
> diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
> index dafe72f4fe..cd2cb8318b 100644
> --- a/gnu/services/configuration.scm
> +++ b/gnu/services/configuration.scm
> @@ -42,6 +42,7 @@ (define-module (gnu services configuration)
>    #:use-module (srfi srfi-26)
>    #:use-module (srfi srfi-34)
>    #:use-module (srfi srfi-35)
> +  #:use-module (srfi srfi-171)
>    #:export (configuration-field
>              configuration-field-name
>              configuration-field-type
> @@ -59,6 +60,10 @@ (define-module (gnu services configuration)
>              define-configuration/no-serialization
>              no-serialization
>  
> +            empty-serializer?
> +            tfilter-maybe-value
> +            base-transducer
> +
>              serialize-configuration
>              define-maybe
>              define-maybe/no-serialization
> @@ -125,13 +130,27 @@ (define-record-type* <configuration-field>
>    (default-value-thunk configuration-field-default-value-thunk)
>    (documentation configuration-field-documentation))
>  
> +(define (empty-serializer? field)
> +  (eq? empty-serializer
> +       (configuration-field-serializer field)))
> +
> +(define (tfilter-maybe-value config)
> +  (tfilter (lambda (field)
> +             (let ((field-value ((configuration-field-getter field) config)))
> +               (maybe-value-set? field-value)))))
> +
> +(define (base-transducer config)
> +  (compose (tremove empty-serializer?)
> +           ;; Only serialize fields whose value isn't '%unset-marker%.
> +           (tfilter-maybe-value config)
> +           (tmap (lambda (field)
> +                   ((configuration-field-serializer field)
> +                    (configuration-field-name field)
> +                    ((configuration-field-getter field) config))))))
> +

Please add docstrings.

>  (define (serialize-configuration config fields)
>    #~(string-append
> -     #$@(map (lambda (field)
> -               ((configuration-field-serializer field)
> -                (configuration-field-name field)
> -                ((configuration-field-getter field) config)))
> -             fields)))
> +     #$@(list-transduce (base-transducer config) rcons fields)))
>

While at it, please add a docstring for this procedure as well.

>  (define-syntax-rule (id ctx parts ...)
>    "Assemble PARTS into a raw (unhygienic) identifier."
> diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
> index 23ccb8d403..56b7772f58 100644
> --- a/gnu/services/telephony.scm
> +++ b/gnu/services/telephony.scm
> @@ -37,6 +37,7 @@ (define-module (gnu services telephony)
>    #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-2)
>    #:use-module (srfi srfi-26)
> +  #:use-module (srfi srfi-171)
>    #:use-module (ice-9 format)
>    #:use-module (ice-9 match)
>    #:export (jami-account
> @@ -204,22 +205,20 @@ (define (jami-account->alist jami-account-object)
>        ('rendezvous-point? "Account.rendezVous")
>        ('peer-discovery? "Account.peerDiscovery")
>        ('bootstrap-hostnames "Account.hostname")
> -      ('name-server-uri "RingNS.uri")
> -      (_ #f)))
> +      ('name-server-uri "RingNS.uri")))
>  
> -  (filter-map (lambda (field)
> -                (and-let* ((name (field-name->account-detail
> +  (define jami-account-transducer
> +    (compose (tremove empty-serializer?)
> +             (tfilter-maybe-value jami-account-object)
> +             (tmap (lambda (field)
> +                     (let* ((name (field-name->account-detail
>                                    (configuration-field-name field)))
> -                           (value ((configuration-field-serializer field)
> -                                   name ((configuration-field-getter field)
> -                                         jami-account-object)))
> -                           ;; The define-maybe default serializer produces an
> -                           ;; empty string for unspecified values.
> -                           (value* (if (string-null? value)
> -                                       #f
> -                                       value)))
> -                  (cons name value*)))
> -              jami-account-fields))
> +                            (value ((configuration-field-serializer field)
> +                                    name ((configuration-field-getter field)
> +                                          jami-account-object))))
> +                       (cons name value))))))
> +
> +  (list-transduce jami-account-transducer rcons jami-account-fields))

Could you please state in a comment under "(define
jami-account-transducer" why the base transducer doesn't suffice?  It
isn't obvious to me from a casual inspection.  I guess it's because
base-transducer is not recursive?  Should it be?

>  (define (jami-account-list? val)
>    (and (list? val)
> diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
> index 8ad5907f37..40a4e74b4d 100644
> --- a/tests/services/configuration.scm
> +++ b/tests/services/configuration.scm
> @@ -337,13 +337,9 @@ (define-maybe symbol)
>  (define-configuration config-with-maybe-symbol
>    (protocol maybe-symbol ""))
>  
> -;;; Maybe symbol values are currently seen as serializable, because the
> -;;; unspecified value is '%unset-marker%, which is a symbol itself.
> -;;; TODO: Remove expected fail marker after resolution.
> -(test-expect-fail 1)
>  (test-equal "symbol maybe value serialization, unspecified"
>    ""
> -  (gexp->approximate-sexp
> +  (eval-gexp
>     (serialize-configuration (config-with-maybe-symbol)
>                              config-with-maybe-symbol-fields)))

That's nice, though I don't understand why gexp->approximate needs to be
turned into eval-gexp?

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 04/11] doc: Rewrite define-configuration Bruno Victal
@ 2023-10-02 18:28     ` Maxim Cournoyer
  2023-10-07 14:21       ` Bruno Victal
  0 siblings, 1 reply; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-02 18:28 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> Rewrite this section to make it easier to document later syntactical
> changes.
>
> * doc/guix.texi (Complex Configurations): Rewrite define-configuration
> documentation. Fix simple serializer example.

[...]

> +@item @code{(serializer @var{serializer})}
> +@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
> +@ref{G-Expressions} that represents the content that will be serialized
> +to the configuration file.  If none is specified, a procedure of the
> +name @code{serialize-@var{type}} will be used.
> +
> +An example of a simple serializer procedure:
> +@lisp
> +(define (serialize-boolean field-name value)
> +  (let ((value (if value "true" "false")))
> +    #~(string-append '#$field-name #$value)))
> +@end lisp
> +

I know this is adapted from old code, but shouldn't there be a "=" in
that string-append, between the field name and its value?  Also, using
gexps here seems unnecessary and may confuse the reader.

The rest LGTM!

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 05/11] services: configuration: Add serializer-options field Bruno Victal
@ 2023-10-02 19:12     ` Maxim Cournoyer
  2023-10-06 18:29       ` Bruno Victal
  0 siblings, 1 reply; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-02 19:12 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hi Bruno,

Bruno Victal <mirai@makinata.eu> writes:

> Allow relaying additional arguments to a serialize-<type> procedure.
>
> * gnu/services/configuration.scm (configuration-field)
> <serializer-options>: New field.
> (base-transducer, define-maybe-helper): Adjust to relay additional arguments.
> (normalize-extra-args): Implement serializer-options literal.
> * tests/services/configuration.scm: Add tests for serializer-options.
> * doc/guix.texi (Complex Configurations): Document serializer-options.

Interesting!

> ---
>  doc/guix.texi                    | 11 +++++
>  gnu/services/configuration.scm   | 49 ++++++++++++-------
>  tests/services/configuration.scm | 82 ++++++++++++++++++++++++++++++++
>  3 files changed, 126 insertions(+), 16 deletions(-)
>
> diff --git a/doc/guix.texi b/doc/guix.texi
> index 8355260378..14802e9366 100644
> --- a/doc/guix.texi
> +++ b/doc/guix.texi
> @@ -42032,6 +42032,17 @@ Complex Configurations
>          ((symbol? value) (symbol->string value))
>          (else (error "bad value"))))
>  @end lisp
> +
> +@item @code{(serializer-options @var{arglst})}
> +@var{arglst} is a list of extra arguments that are relayed to the
> +serializing procedure.  This allows for writing serialization
> +procedures that take more than two arguments.
> +
> +An example of a serializer procedure that requires additional data:
> +@lisp
> +(define* (serialize-port field value #:key context)
> +  #~(format #f "section=~a,port=~d" #$context #$value))
> +@end lisp
>  @end table
>  
>  In some cases multiple different configuration records might be defined
> diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
> index cd2cb8318b..4eee5a26c2 100644
> --- a/gnu/services/configuration.scm
> +++ b/gnu/services/configuration.scm
> @@ -50,6 +50,7 @@ (define-module (gnu services configuration)
>              configuration-field-error
>              configuration-field-sanitizer
>              configuration-field-serializer
> +            configuration-field-serializer-options
>              configuration-field-getter
>              configuration-field-default-value-thunk
>              configuration-field-documentation
> @@ -127,6 +128,7 @@ (define-record-type* <configuration-field>
>    (predicate configuration-field-predicate)
>    (sanitizer configuration-field-sanitizer)
>    (serializer configuration-field-serializer)
> +  (serializer-options configuration-field-serializer-options)
>    (default-value-thunk configuration-field-default-value-thunk)
>    (documentation configuration-field-documentation))
>  
> @@ -144,9 +146,13 @@ (define (base-transducer config)
>             ;; Only serialize fields whose value isn't '%unset-marker%.
>             (tfilter-maybe-value config)
>             (tmap (lambda (field)
> -                   ((configuration-field-serializer field)
> -                    (configuration-field-name field)
> -                    ((configuration-field-getter field) config))))))
> +                   (let ((serializer (configuration-field-serializer field))
> +                         (field-name (configuration-field-name field))
> +                         (value
> +                          ((configuration-field-getter field) config))
> +                         (serializer-options
> +                          (configuration-field-serializer-options field)))
> +                     (apply serializer field-name value serializer-options))))))
>  
>  (define (serialize-configuration config fields)
>    #~(string-append
> @@ -173,10 +179,9 @@ (define (define-maybe-helper serialize? prefix syn)
>               (or (not (maybe-value-set? val))
>                   (stem? val)))
>             #,@(if serialize?
> -                  (list #'(define (serialize-maybe-stem field-name val)
> -                            (if (stem? val)
> -                                (serialize-stem field-name val)
> -                                "")))
> +                  (list #'(define (serialize-maybe-stem field-name val . rest)
> +                            (when (maybe-value-set? val)
> +                              (apply serialize-stem field-name val rest))))
>                    '()))))))
>  
>  (define-syntax define-maybe
> @@ -210,38 +215,49 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
>      "Extract and normalize arguments following @var{doc}."
>      (let loop ((s s)
>                 (sanitizer* #f)
> -               (serializer* #f))
> -      (syntax-case s (sanitizer serializer empty-serializer)
> +               (serializer* #f)
> +               (serializer-options* #f))
> +      (syntax-case s (sanitizer serializer empty-serializer serializer-options)
>          (((sanitizer proc) tail ...)
>           (if sanitizer*
>               (syntax-violation 'sanitizer
>                                 "duplicate entry" #'proc)
> -             (loop #'(tail ...) #'proc serializer*)))
> +             (loop #'(tail ...)
> +                   #'proc serializer* serializer-options*)))
>          (((serializer proc) tail ...)
>           (if serializer*
>               (syntax-violation 'serializer
>                                 "duplicate or conflicting entry" #'proc)
> -             (loop #'(tail ...) sanitizer* #'proc)))
> +             (loop #'(tail ...)
> +                   sanitizer* #'proc serializer-options*)))
>          ((empty-serializer tail ...)
>           (if serializer*
>               (syntax-violation 'empty-serializer
>                                 "duplicate or conflicting entry" #f)
> -             (loop #'(tail ...) sanitizer* #'empty-serializer)))
> +             (loop #'(tail ...)
> +                   sanitizer* #'empty-serializer #f)))
> +        (((serializer-options args) tail ...)
> +         (if serializer-options*
> +             (syntax-violation 'serializer-options
> +                               "duplicate or conflicting entry" #f)
> +             (loop #'(tail ...)
> +                   sanitizer* serializer* #'args)))
>          (()  ; stop condition
> -         (values (list sanitizer* serializer*)))
> +         (values (list sanitizer* serializer*
> +                       (or serializer-options* #'(quote ())))))
>          ((proc)  ; TODO: deprecated, to be removed.
> -         (every not (list sanitizer* serializer*))
> +         (every not (list sanitizer* serializer* serializer-options*))
>           (begin
>             (warning #f (G_ "specifying serializers after documentation is \
>  deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
> -           (values (list #f #'proc)))))))
> +           (values (list #f #'proc #'(quote ()))))))))
>  
>    (syntax-case syn ()
>      ((_ stem (field field-type+def doc extra-args ...) ...)
>       (with-syntax
>           ((((field-type def) ...)
>             (map normalize-field-type+def #'(field-type+def ...)))
> -          (((sanitizer* serializer*) ...)
> +          (((sanitizer* serializer* serializer-options*) ...)
>             (map normalize-extra-args #'((extra-args ...) ...))))
>         (with-syntax
>             (((field-getter ...)
> @@ -327,6 +343,7 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
>                         (or field-sanitizer
>                             (id #'stem #'validate- #'stem #'- #'field)))
>                        (serializer field-serializer)
> +                      (serializer-options serializer-options*)
>                        (default-value-thunk
>                          (lambda ()
>                            (if (maybe-value-set? (syntax->datum field-default))
> diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
> index 40a4e74b4d..8b1d1e4749 100644
> --- a/tests/services/configuration.scm
> +++ b/tests/services/configuration.scm
> @@ -297,6 +297,88 @@ (define (sanitize-port value)
>                      (lambda _ "lorem")
>                      (sanitizer (lambda () #t)))))))
>  
> +(test-group "Serializer options"
> +  (test-group "Serialize keyword arguments"
> +    (define* (serialize-port field value #:key host)
> +      (format #f "host=~a,port=~d" host value))
> +
> +    (define-configuration kwarg-config
> +      (port
> +       (port 80)
> +       "Lorem Ipsum."
> +       (serializer-options '(#:host "[2001:db8::1]"))))
> +
> +    (define-maybe port)
> +    (define-configuration kwarg-maybe-config
> +      (port
> +       (maybe-port 80)
> +       "Lorem Ipsum."
> +       (serializer-options '(#:host "[2001:db8::1]"))))
> +
> +    (test-equal "keyword argument provided"
> +      "host=[2001:db8::1],port=80"
> +      (eval-gexp
> +       (serialize-configuration (kwarg-config)
> +                                kwarg-config-fields)))
> +
> +    (test-equal "keyword argument provided, maybe type"
> +      "host=[2001:db8::1],port=80"
> +      (eval-gexp
> +       (serialize-configuration (kwarg-maybe-config)
> +                                kwarg-maybe-config-fields))))
> +
> +  (test-group "Serialize optional arguments"
> +    (define* (serialize-port field-name value #:optional override-name)
> +      (format #f "~a=~d" (or override-name field-name) value))
> +
> +    (define-configuration with-optarg
> +      (port
> +       (port 80)
> +       "Lorem Ipsum."
> +       (serializer-options '(service-port))))
> +
> +    (define-configuration without-optarg
> +      (port
> +       (port 80)
> +       "Lorem Ipsum."))
> +
> +    (test-equal "optional argument, provided"
> +      "service-port=80"
> +      (eval-gexp (serialize-configuration (with-optarg)
> +                                          with-optarg-fields)))
> +
> +    (test-equal "optional argument, absent"
> +      "port=80"
> +      (eval-gexp (serialize-configuration (without-optarg)
> +                                          without-optarg-fields))))
> +
> +  (test-group "Serialize optional & keyword arguments"
> +    (define* (serialize-port field-name value #:optional override-name
> +                             #:key host)
> +      (format #f "host=~a,~a=~d" host (or override-name field-name) value))
> +
> +    (define-configuration mixed-args
> +      (port
> +       (port 80)
> +       "Lorem Ipsum."
> +       (serializer-options '(service-port #:host "example.com"))))
> +
> +    (define-configuration mixed-no-optarg
> +      (port
> +       (port 80)
> +       "Lorem Ipsum."
> +       (serializer-options '(#:host "example.com"))))
> +
> +    (test-equal "mixed arguments, optional provided"
> +      "host=example.com,service-port=80"
> +      (eval-gexp (serialize-configuration (mixed-args)
> +                                          mixed-args-fields)))
> +
> +    (test-equal "mixed arguments, optional absent"
> +      "host=example.com,port=80"
> +      (eval-gexp (serialize-configuration (mixed-no-optarg)
> +                                          mixed-no-optarg-fields)))))

Could you offer some of your perspective as to why you preferred that
approach compared to generating multiple, tailored for use, serializers,
possibly created via a procedure?  Was there a problem with doing so, or
what is less readable, etc.?

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 06/11] services: configuration: New generic-ini module Bruno Victal
@ 2023-10-02 19:15     ` Maxim Cournoyer
  0 siblings, 0 replies; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-02 19:15 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> Implements a ‘serialize-ini-configuration’ procedure for serializing
> record-types defined with define-configuration into generic INI files.
>
> * gnu/services/configuration/generic-ini.scm: New module.
> * tests/services/configuration/generic-ini.scm: Add tests for new module.
> * Makefile.am: Register tests.
> * gnu/local.mk: Register module.

Nitpick: I'd perhaps rename the module simply 'ini', to shorten a bit
the module namespace, already quite long.

> ---
>  Makefile.am                                  |   1 +
>  gnu/local.mk                                 |   1 +
>  gnu/services/configuration/generic-ini.scm   | 165 +++++++++++++++++++
>  tests/services/configuration/generic-ini.scm | 129 +++++++++++++++
>  4 files changed, 296 insertions(+)
>  create mode 100644 gnu/services/configuration/generic-ini.scm
>  create mode 100644 tests/services/configuration/generic-ini.scm
>
> diff --git a/Makefile.am b/Makefile.am
> index a386e6033c..b6d048f140 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -553,6 +553,7 @@ SCM_TESTS =					\
>    tests/services.scm				\
>    tests/services/file-sharing.scm		\
>    tests/services/configuration.scm		\
> +  tests/services/configuration/generic-ini.scm	\
>    tests/services/lightdm.scm			\
>    tests/services/linux.scm			\
>    tests/services/telephony.scm			\
> diff --git a/gnu/local.mk b/gnu/local.mk
> index e65888a044..796ac33107 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -670,6 +670,7 @@ GNU_SYSTEM_MODULES =				\
>    %D%/services/cgit.scm			\
>    %D%/services/ci.scm				\
>    %D%/services/configuration.scm		\
> +  %D%/services/configuration/generic-ini.scm	\
>    %D%/services/cuirass.scm			\
>    %D%/services/cups.scm				\
>    %D%/services/databases.scm			\
> diff --git a/gnu/services/configuration/generic-ini.scm b/gnu/services/configuration/generic-ini.scm
> new file mode 100644
> index 0000000000..4f83cce13a
> --- /dev/null
> +++ b/gnu/services/configuration/generic-ini.scm
> @@ -0,0 +1,165 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (gnu services configuration generic-ini)
> +  #:use-module (gnu services configuration)
> +  #:use-module (guix gexp)
> +  #:use-module (srfi srfi-9)
> +  #:use-module (srfi srfi-171)
> +  #:use-module (srfi srfi-171 meta)
> +  #:use-module (ice-9 match)
> +  #:export (ini-entry?
> +            list-of-ini-entries?
> +
> +            ini-entries
> +            ini-entries?
> +            entries
> +
> +            serialize-ini-configuration
> +            generic-ini-serialize-string
> +            generic-ini-serialize-boolean
> +            generic-ini-serialize-ini-entry
> +            generic-ini-serialize-list-of-ini-entries))
> +
> +;;;
> +;;; Generic INI serializer
> +;;;
> +

Nothing here?  I'd turn this into a Commentary comment, and document
that this is intended to match in behavior SRFI-233, so that it can
eventually be replaced without much fuss when it lands to Guile.

> +\f
> +;;;
> +;;; Predicates
> +;;;
> +
> +;; This is the same format used in SRFI-233 but without comment support.
> +(define ini-entry?
> +  (match-lambda
> +    (((? symbol?) (? symbol?) (? string?)) #t)
> +    (_ #f)))
> +
> +(define list-of-ini-entries?
> +  (list-of ini-entry?))
> +
> +;;
> +;; Overall design document
> +;;
> +;; This module implements a generic INI serializer for a record-type defined
> +;; using define-configuration.
> +;; It expects that the serialize-<type> procedures return a list with
> +;; three elements of the form:
> +;;    (list section key value)
> +;; Where ‘section’ and ‘key’ are symbols and ‘value’ is a string.
> +;; For serializing procedures that have to return multiple entries at once,
> +;; such as encountered when synthesizing configuration from a record object
> +;; or “escape hatch fields”, it must wrap the result by calling ‘ini-entries’
> +;; with a list of INI-entries as described above.
> +;; This is implemented as a constructor for a SRFI-9 record type named
> +;; “<ini-entries>”.
> +;;
> +;; The fields within define-configuration do not have to be ordered in,
> +;; any way whatsoever as the ‘serialize-ini’ will group them up automatically.
> +;; This implies that no assumptions should be made regarding the order of the
> +;; values in the serializied INI output.
> +;;
> +;; Additional notes:
> +;; Q: Why not replace rcons with string-append and forego the ungexp-splice?
> +;; A: The transduction happens outside of the G-Exp while the final string-append
> +;;    takes place in the G-Exp.
> +;;
> +;; Debugging tips: Open a REPL and try one transducer at a time from
> +;; ‘ini-transducer’.
> +;;

This should go to the Commentary section.

> +
> +;; A “bag” holding multiple ini-entries.
> +(define-record-type <ini-entries>
> +  (ini-entries val)
> +  ini-entries?
> +  (val entries))
> +
> +(define (add-section-header partition)
> +  (let ((header (caar partition)))
> +    (cons (list header)
> +          partition)))
> +
> +(define serializer
> +  (match-lambda
> +    ((section)
> +     #~(format #f "[~a]~%" '#$section))
> +    ((section key value)
> +     #~(format #f "~a=~a~%" '#$key #$value))
> +    ;; Used for the newline between sections.
> +    ('*section-separator* "\n")))
> +
> +(define ini-transducer
> +  (compose (tpartition car)
> +           (tmap add-section-header)
> +           (tadd-between '(*section-separator*))
> +           tconcatenate
> +           (tmap serializer)))
> +
> +;; A selective version of ‘tconcatenate’ but for ‘<ini-entries>’ objects only.
> +(define (tconcatenate-ini-entries reducer)
> +  (case-lambda
> +    (() '())
> +    ((result) (reducer result))
> +    ((result input)
> +     (if (ini-entries? input)
> +         (list-reduce (preserving-reduced reducer) result (entries input))
> +         (reducer result input)))))
> +
> +;; A “first-pass” serialization is performed and sorted in order
> +;; to group up the fields by “section” before passing through the
> +;; transducer.
> +(define (serialize-ini-configuration config fields)
> +  (let* ((srfi-233-IR
> +          ;; First pass: “serialize” into a (disordered) list of
> +          ;; SRFI-233 entries.
> +          (list-transduce (compose (base-transducer config)
> +                                   tconcatenate-ini-entries)
> +                          rcons fields))
> +         (comparator (lambda (x y)
> +                       ;; Sort the SRFI-233 entries by section.
> +                       (string<=? (symbol->string (car x))
> +                                  (symbol->string (car y)))))
> +         (sorted-entries (sort srfi-233-IR comparator)))
> +    #~(string-append
> +       #$@(list-transduce ini-transducer rcons sorted-entries))))
> +

Please add doc strings to all new procedures.  I think comments in
Scheme are more commonly nested inside the procedure than on top of it,
like is custom in C, though that's mostly based on what I saw in Guix.

> +\f
> +;;;
> +;;; Serializers
> +;;;
> +
> +;; These are “gratuitous” serializers that can be readily used by
> +;; using the literal (prefix generic-ini-) within define-configuration.
> +

Instead of gratuitous, which sounds pejorative to me, I'd reword to
"Convenience serializers that can be ..."

> +;; Notes: field-name-transform can be used to “uglify” a field-name,
> +;; e.g. want-ipv6?  ->  want_ipv6
> +(define* (generic-ini-serialize-string field-name value #:key section
> +                                       (field-name-transform identity))
> +  (list section (field-name-transform field-name) value))
> +
> +(define* (generic-ini-serialize-boolean field-name value #:key section
> +                            (field-name-transform identity))
> +  (list section (field-name-transform field-name)
> +        (if value "true" "false")))
> +
> +(define (generic-ini-serialize-ini-entry field-name value)
> +  value)
> +
> +(define (generic-ini-serialize-list-of-ini-entries field-name value)
> +  (ini-entries value))

Here also, please add docstrings.

> diff --git a/tests/services/configuration/generic-ini.scm b/tests/services/configuration/generic-ini.scm
> new file mode 100644
> index 0000000000..797a01af31
> --- /dev/null
> +++ b/tests/services/configuration/generic-ini.scm
> @@ -0,0 +1,129 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2023 Bruno Victal <mirai@makinata.eu>
> +;;;
> +;;; This file is part of GNU Guix.
> +;;;
> +;;; GNU Guix is free software; you can redistribute it and/or modify it
> +;;; under the terms of the GNU General Public License as published by
> +;;; the Free Software Foundation; either version 3 of the License, or (at
> +;;; your option) any later version.
> +;;;
> +;;; GNU Guix is distributed in the hope that it will be useful, but
> +;;; WITHOUT ANY WARRANTY; without even the implied warranty of
> +;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +;;; GNU General Public License for more details.
> +;;;
> +;;; You should have received a copy of the GNU General Public License
> +;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
> +
> +(define-module (tests services configuration generic-ini)
> +  #:use-module (gnu services configuration)
> +  #:use-module (gnu services configuration generic-ini)
> +  #:use-module (guix diagnostics)
> +  #:use-module (guix gexp)
> +  #:use-module (guix store)
> +  #:autoload (guix i18n) (G_)

Are all these modules truly needed, e.g. the i18n one and (guix
diagnostics)?

> +  #:use-module (srfi srfi-34)
> +  #:use-module (srfi srfi-64)
> +  #:use-module (srfi srfi-71))
> +
> +;;; Tests for the (gnu services configuration generic-ini) module.
> +
> +(test-begin "generic-ini serializer")
> +
> +
> +(define expected-output "\
> +[guardians]
> +llamas=Tommy,Isabella
> +donkeys=Franz,Olly
> +
> +[ranch]
> +shepherd=Emma
> +
> +[shed]
> +colours=Alizarin
> +enabled=true
> +capacity=50
> +production=wool
> +
> +[vehicles]
> +cars=313
> +bikes=Amaryllis
> +")
> +
> +\f
> +;;;
> +;;; Serializers
> +;;;
> +(define (strip-trailing-?-character field-name)

nitpick: 'strip-trailing-?' seems explicit enough to me :-).

> +  "Drop rightmost '?' character"
> +  (let ((str (symbol->string field-name)))
> +    (if (string-suffix? "?" str)
> +        (string->symbol (string-drop-right str 1))
> +        field-name)))
> +
> +(define* (serialize-string field-name value #:key section)
> +  (list section field-name value))
> +
> +(define* (serialize-number field-name value #:key section)
> +  (list section field-name (number->string value)))
> +
> +(define* (serialize-boolean field-name value #:key section)
> +  (list section (strip-trailing-?-character field-name)
> +        (if value "true" "false")))
> +
> +(define serialize-ini-entry
> +  generic-ini-serialize-ini-entry)
> +
> +(define serialize-list-of-ini-entries
> +  generic-ini-serialize-list-of-ini-entries)
> +
> +\f
> +;;;
> +;;; Record-type definition
> +;;;
> +
> +(define-configuration foo-configuration
> +  (production
> +   (string "wool")
> +   "Lorem Ipsum …"
> +   (serializer-options '(#:section shed)))
> +
> +  (capacity
> +   (number 50)
> +   "Lorem Ipsum …"
> +   (serializer-options '(#:section shed)))
> +
> +  (enabled?
> +   (boolean #t)
> +   "Lorem Ipsum …"
> +   (serializer-options '(#:section shed)))
> +
> +  (shepherd
> +   (string "Emma")
> +   "Lorem Ipsum …"
> +   (serializer-options '(#:section ranch)))
> +
> +  (raw-entry
> +   (ini-entry '(shed colours "Alizarin"))
> +   "Lorem Ipsum …")
> +
> +  (escape-hatch
> +   (list-of-ini-entries '((vehicles bikes "Amaryllis")
> +                          (vehicles cars "313")
> +                          (guardians donkeys "Franz,Olly")
> +                          (guardians llamas "Tommy,Isabella")))
> +   "Lorem Ipsum …"))
> +
> +(test-equal "Well-formed INI output from serialize-ini"
> +  expected-output
> +  ;; Serialize the above into a string, properly resolving any potential
> +  ;; nested G-Exps as well.
> +  (let* ((serialized-ini
> +          (serialize-ini-configuration (foo-configuration)
> +                                       foo-configuration-fields))
> +         (lowered conn (with-store store
> +                         ((lower-gexp serialized-ini) store))))
> +    (eval (lowered-gexp-sexp lowered) (current-module))))
> +

Since 'conn' appears unused, I think you don't need to bind it at all,
and can then drop (srfi srfi-71).

--
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 10/11] services: NetworkManager: add log-configuration field.
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 10/11] services: NetworkManager: add log-configuration field Bruno Victal
@ 2023-10-05 16:57     ` Maxim Cournoyer
  0 siblings, 0 replies; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-05 16:57 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> * gnu/services/networking.scm (network-manager-log-level?)
> (network-manager-log-domain?, network-manager-log-domains?): New predicate.
> (serialize-network-manager-log-level, serialize-network-manager-log-domains):
> New procedure.
> (network-manager-log-configuration): New record type.
> (network-manager-configuration)[log-configuration]: New field.
> * doc/guix.texi (Networking Setup): Document it.

My attention level is not too high anymore, but it LGTM.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 11/11] services: NetworkManager: Add extra-options field.
  2023-06-26 21:59   ` [bug#63985] [PATCH v3 11/11] services: NetworkManager: Add extra-options field Bruno Victal
@ 2023-10-05 16:59     ` Maxim Cournoyer
  0 siblings, 0 replies; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-05 16:59 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> * gnu/services/networking.scm (network-manager-configuration)
> [extra-options]: New field.
> * doc/guix.texi (Networking Setup): Document it.

LGTM!

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-10-02 19:12     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
@ 2023-10-06 18:29       ` Bruno Victal
  0 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-10-06 18:29 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 63985

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

Hi Maxim,

On 2023-10-02 20:12, Maxim Cournoyer wrote:
> Could you offer some of your perspective as to why you preferred that
> approach compared to generating multiple, tailored for use, serializers,
> possibly created via a procedure?  Was there a problem with doing so, or
> what is less readable, etc.?

Right, as already pointed out by Liliana in [1]. As for the perspective I'm
afraid the reason is a rather unexciting one: I completely overlooked that
it was another¹ approach that could have been done instead! (and a much simpler
one at that)

I'm dropping this one as well since it's clear now that there's a cleaner
approach for passing these kinds of additional data.


[1]: Message-ID: <6445e508cbc8a9f92d3a54263193936d168cd7cf.camel@gmail.com>
     Link: <https://lists.gnu.org/archive/html/guix-patches/2023-09/msg01205.html>

¹PS: Example attached for any future readers interested in the topic.

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

Cheers,
Bruno.

[-- Attachment #2: options-as-procedure-call-sample.scm --]
[-- Type: text/x-scheme, Size: 811 bytes --]

#!/usr/bin/env -S guile --no-auto-compile
!#

(use-modules (gnu services configuration)
             (guix gexp)
             (srfi srfi-64))


(test-begin "services-configuration")

(define (eval-gexp x)
  "Get serialized config as string."
  (eval (gexp->approximate-sexp x)
        (current-module)))

(test-group "Parametric arguments"
  (define* (make-serialize-port #:key host)
    (lambda (field value)
      (format #f "host=~a,port=~d" host value)))

  (define-configuration kwarg-config
    (port
     (number 80)
     "Lorem Ipsum."
     (serializer (make-serialize-port #:host "[2001:db8::1]"))))

  (test-equal "keyword argument provided"
    "host=[2001:db8::1],port=80"
    (eval-gexp
     (serialize-configuration (kwarg-config)
                              kwarg-config-fields))))

(test-end)

^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args. (was: bug#63985: [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration)
  2023-10-02 17:00     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
@ 2023-10-07 12:36       ` Bruno Victal
  0 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 12:36 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 63985

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> Bruno Victal <mirai@makinata.eu> writes:
>>           (begin
>>             (warning #f (G_ "specifying serializers after documentation is \
>>  deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
>> -           (values (list %unset-value #'proc)))))))
>> +           (values (list #f #'proc)))))))
>>  
>>    (syntax-case syn ()
>>      ((_ stem (field field-type+def doc extra-args ...) ...)
>> @@ -239,11 +239,11 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
>>                       default-value))
>>                    #'((field-type def) ...)))
>>              ((field-sanitizer ...)
>> -             (map maybe-value #'(sanitizer* ...)))
>> +             #'(sanitizer* ...))
>>              ((field-serializer ...)
>>               (map (lambda (type proc)
>>                      (and serialize?
>> -                         (or (maybe-value proc)
>> +                         (or proc
>
> I haven't applied it locally so may be out of context, but how do we
> ensure here that sanitizer and proc aren't set to #f before calling
> them?

In the (or proc …) clause the logic is still equivalent to the previous
(maybe-value proc) wrapped one. There's no problem with proc being #f as
it either means that the field is marked as `no-serialization' or it
will use the “default” serializers that appears latter within the (or …)
clause.

For sanitizers it boils down to this check further down:

--8<---------------cut here---------------start------------->8---
;; Define field validation macros.
#,@(filter-map (lambda (name pred sanitizer)
                 (if sanitizer
                     #f
                     (default-field-sanitizer name pred)))
               #'(field ...)
               #'(field-predicate ...)
               #'(field-sanitizer ...))
--8<---------------cut here---------------end--------------->8---

So if a custom sanitizer wasn't provided (which is marked internally as
#f) then this filter-map will take care of defining a default one for
it.

-- 
Thanks,
Bruno.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration. (was : bug#63985: [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration)
  2023-10-02 17:25     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
@ 2023-10-07 13:39       ` Bruno Victal
  2023-10-07 14:37         ` Maxim Cournoyer
  0 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 13:39 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 63985

On 2023-10-02 18:25, Maxim Cournoyer wrote:
> Bruno Victal <mirai@makinata.eu> writes:
>> diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
>> index 23ccb8d403..56b7772f58 100644
>> --- a/gnu/services/telephony.scm
>> +++ b/gnu/services/telephony.scm
>> @@ -204,22 +205,20 @@ (define (jami-account->alist jami-account-object)
>>        ('rendezvous-point? "Account.rendezVous")
>>        ('peer-discovery? "Account.peerDiscovery")
>>        ('bootstrap-hostnames "Account.hostname")
>> -      ('name-server-uri "RingNS.uri")
>> -      (_ #f)))
>> +      ('name-server-uri "RingNS.uri")))
>>  
>> -  (filter-map (lambda (field)
>> -                (and-let* ((name (field-name->account-detail
>> +  (define jami-account-transducer
>> +    (compose (tremove empty-serializer?)
>> +             (tfilter-maybe-value jami-account-object)
>> +             (tmap (lambda (field)
>> +                     (let* ((name (field-name->account-detail
>>                                    (configuration-field-name field)))
>> -                           (value ((configuration-field-serializer field)
>> -                                   name ((configuration-field-getter field)
>> -                                         jami-account-object)))
>> -                           ;; The define-maybe default serializer produces an
>> -                           ;; empty string for unspecified values.
>> -                           (value* (if (string-null? value)
>> -                                       #f
>> -                                       value)))
>> -                  (cons name value*)))
>> -              jami-account-fields))
>> +                            (value ((configuration-field-serializer field)
>> +                                    name ((configuration-field-getter field)
>> +                                          jami-account-object))))
>> +                       (cons name value))))))
>> +
>> +  (list-transduce jami-account-transducer rcons jami-account-fields))
> 
> Could you please state in a comment under "(define
> jami-account-transducer" why the base transducer doesn't suffice?  It
> isn't obvious to me from a casual inspection.  I guess it's because
> base-transducer is not recursive?  Should it be?

This is because you're changing the field names of the JAMI-ACCOUNT record type
through `field-name->account-detail'.
Conventional serializers are (serializer (field-name value)) procedures and this is
how `base-transducer' calls them. Here you want to do something of the sort
(serializer (translate-field-name name) value) so a custom transducer was written to
account for this detour.

I wonder if we could simply this with some functional programming as discussed in
[1] instead.

>>  (test-equal "symbol maybe value serialization, unspecified"
>>    ""
>> -  (gexp->approximate-sexp
>> +  (eval-gexp
>>     (serialize-configuration (config-with-maybe-symbol)
>>                              config-with-maybe-symbol-fields)))
> 
> That's nice, though I don't understand why gexp->approximate needs to be
> turned into eval-gexp?

Using `gexp->approximate-sexp' alone doesn't result in a evaluation of the
serialization process so eval-gexp has to be used in order to actually perform
this test.


[1]: Message-ID: <673081be-14c1-4864-9bd1-1cbc908823a6@makinata.eu>
     Link: <https://lists.gnu.org/archive/html/guix-patches/2023-10/msg00313.html>

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

Cheers,
Bruno.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-10-02 18:28     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
@ 2023-10-07 14:21       ` Bruno Victal
  2023-10-07 16:35         ` Maxim Cournoyer
  0 siblings, 1 reply; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 14:21 UTC (permalink / raw)
  To: Maxim Cournoyer; +Cc: 63985

On 2023-10-02 19:28, Maxim Cournoyer wrote:
>> +An example of a simple serializer procedure:
>> +@lisp
>> +(define (serialize-boolean field-name value)
>> +  (let ((value (if value "true" "false")))
>> +    #~(string-append '#$field-name #$value)))
>> +@end lisp
>> +
> 
> I know this is adapted from old code, but shouldn't there be a "=" in
> that string-append, between the field name and its value? […]

I think there's no particular meaning for that isolated snippet but I
believe the original author did intend for there to be a "=" symbol as
you have pointed out. I've went ahead and added the “ = ” (with spaces)
variant instead.

> […] Also, using gexps here seems unnecessary and may confuse the reader.

I don't think so because in general you're going to call
`serialize-configuration' which returns a G-Expression so it's more or
less given that using G-Expressions is perfectly valid and using them
from the outset allows your fields to serialize any kind of value such as
package objects (for something like `gcc-path = /gnu/store/…'), etc.

Thus, not using G-Exps is more restrictive in terms of what you can
serialize. It might not make a difference in some cases but I think
this is a potential source of trouble when someone tries to serialize
a gexp object with a non-gexp ready serializer. (e.g. performing
string-append with a string and a package object)

I'm aware of some cases where this is not a problem such as the case
where you write your own version of `serialize-configuration' to
handle the serialization in some other manner. (perhaps using `list'
instead of `string-append' for the final step)
Such a case is clearly in the “advanced guix hacker” arena and thus
I don't think there's a risk for confusion here.

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

Cheers,
Bruno.




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration. (was : bug#63985: [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration)
  2023-10-07 13:39       ` [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration. (was : bug#63985: [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration) Bruno Victal
@ 2023-10-07 14:37         ` Maxim Cournoyer
  0 siblings, 0 replies; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-07 14:37 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hello,

Bruno Victal <mirai@makinata.eu> writes:

> On 2023-10-02 18:25, Maxim Cournoyer wrote:
>> Bruno Victal <mirai@makinata.eu> writes:
>>> diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
>>> index 23ccb8d403..56b7772f58 100644
>>> --- a/gnu/services/telephony.scm
>>> +++ b/gnu/services/telephony.scm
>>> @@ -204,22 +205,20 @@ (define (jami-account->alist jami-account-object)
>>>        ('rendezvous-point? "Account.rendezVous")
>>>        ('peer-discovery? "Account.peerDiscovery")
>>>        ('bootstrap-hostnames "Account.hostname")
>>> -      ('name-server-uri "RingNS.uri")
>>> -      (_ #f)))
>>> +      ('name-server-uri "RingNS.uri")))
>>>  
>>> -  (filter-map (lambda (field)
>>> -                (and-let* ((name (field-name->account-detail
>>> +  (define jami-account-transducer
>>> +    (compose (tremove empty-serializer?)
>>> +             (tfilter-maybe-value jami-account-object)
>>> +             (tmap (lambda (field)
>>> +                     (let* ((name (field-name->account-detail
>>>                                    (configuration-field-name field)))
>>> -                           (value ((configuration-field-serializer field)
>>> -                                   name ((configuration-field-getter field)
>>> -                                         jami-account-object)))
>>> -                           ;; The define-maybe default serializer produces an
>>> -                           ;; empty string for unspecified values.
>>> -                           (value* (if (string-null? value)
>>> -                                       #f
>>> -                                       value)))
>>> -                  (cons name value*)))
>>> -              jami-account-fields))
>>> +                            (value ((configuration-field-serializer field)
>>> +                                    name ((configuration-field-getter field)
>>> +                                          jami-account-object))))
>>> +                       (cons name value))))))
>>> +
>>> +  (list-transduce jami-account-transducer rcons jami-account-fields))
>> 
>> Could you please state in a comment under "(define
>> jami-account-transducer" why the base transducer doesn't suffice?  It
>> isn't obvious to me from a casual inspection.  I guess it's because
>> base-transducer is not recursive?  Should it be?
>
> This is because you're changing the field names of the JAMI-ACCOUNT record type
> through `field-name->account-detail'.
> Conventional serializers are (serializer (field-name value)) procedures and this is
> how `base-transducer' calls them. Here you want to do something of the sort
> (serializer (translate-field-name name) value) so a custom transducer was written to
> account for this detour.
>
> I wonder if we could simply this with some functional programming as discussed in
> [1] instead.
>
>>>  (test-equal "symbol maybe value serialization, unspecified"
>>>    ""
>>> -  (gexp->approximate-sexp
>>> +  (eval-gexp
>>>     (serialize-configuration (config-with-maybe-symbol)
>>>                              config-with-maybe-symbol-fields)))
>> 
>> That's nice, though I don't understand why gexp->approximate needs to be
>> turned into eval-gexp?
>
> Using `gexp->approximate-sexp' alone doesn't result in a evaluation of the
> serialization process so eval-gexp has to be used in order to actually perform
> this test.

Great, thanks for the explanations!

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration
  2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
                   ` (10 preceding siblings ...)
  2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
@ 2023-10-07 15:57 ` Bruno Victal
  2023-10-07 15:57   ` [bug#63985] [PATCH v4 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
                     ` (5 more replies)
  11 siblings, 6 replies; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 15:57 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal, Maxim Cournoyer

Notable changes since v3:
* Reduced series scope for SRFI-171 related changes and documentation
improvements.
* Added docstrings.
  Note: docstring for `serialize-configuration' lifted from guix.texi.
* Addressed feedback from v3.

Bruno Victal (5):
  services: configuration: Simplify normalize-extra-args.
  services: configuration: Use transducers within
    serialize-configuration.
  services: fstrim-service-type: Serialize with SRFI-171 transducers.
  doc: Rewrite define-configuration.
  services: configuration: Add some commonly used predicates.

 doc/guix.texi                    | 108 +++++++++++++------------------
 gnu/services/audio.scm           |   7 +-
 gnu/services/configuration.scm   |  81 +++++++++++++++++------
 gnu/services/linux.scm           |  11 ++--
 gnu/services/telephony.scm       |  49 ++++++--------
 tests/services/configuration.scm |   6 +-
 6 files changed, 136 insertions(+), 126 deletions(-)


base-commit: b566e1a98a74d84d3978cffefd05295602c9445d
-- 
2.41.0





^ permalink raw reply	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v4 2/5] services: configuration: Use transducers within serialize-configuration.
  2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
@ 2023-10-07 15:57   ` Bruno Victal
  2023-10-07 15:59   ` [bug#63985] [PATCH v4 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 15:57 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal, Maxim Cournoyer

Introduces 'base-transducer', a SRFI-171 based transducer that can be used as a
starting point for writing custom configuration record serializing procedures.

This also fixes the symbol maybe-value serialization test case.

* gnu/services/configuration.scm (empty-serializer?): New predicate.
(base-transducer, tfilter-maybe-value): New procedure.
(serialize-configuration): Adapt to use base-transducer.

* gnu/services/telephony.scm (jami-account->alist): Use transducers to skip
fields that are unserializable or whose field maybe-value is unset.

* tests/services/configuration.scm: Remove test-expect-fail.
---
 gnu/services/configuration.scm   | 38 +++++++++++++++++++++++++++-----
 gnu/services/telephony.scm       | 27 +++++++++++------------
 tests/services/configuration.scm |  6 +----
 3 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index ecc5049a79..aa5fb832d5 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -42,6 +42,7 @@ (define-module (gnu services configuration)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-171)
   #:export (configuration-field
             configuration-field-name
             configuration-field-type
@@ -59,6 +60,10 @@ (define-module (gnu services configuration)
             define-configuration/no-serialization
             no-serialization
 
+            empty-serializer?
+            tfilter-maybe-value
+            base-transducer
+
             serialize-configuration
             define-maybe
             define-maybe/no-serialization
@@ -125,13 +130,36 @@ (define-record-type* <configuration-field>
   (default-value-thunk configuration-field-default-value-thunk)
   (documentation configuration-field-documentation))
 
+(define (empty-serializer? field)
+  "Predicate that checks whether FIELD is exempt from serialization."
+  (eq? empty-serializer
+       (configuration-field-serializer field)))
+
+(define (tfilter-maybe-value config)
+  "Return a transducer for CONFIG that removes all maybe-type fields whose
+value is '%unset-marker."
+  (tfilter (lambda (field)
+             (let ((field-value ((configuration-field-getter field) config)))
+               (maybe-value-set? field-value)))))
+
+(define (base-transducer config)
+  "Return a transducer for CONFIG that calls the serializing procedures only
+for fields marked for serialization and whose values are not '%unset-marker."
+  (compose (tremove empty-serializer?)
+           ;; Only serialize fields whose value isn't '%unset-marker%.
+           (tfilter-maybe-value config)
+           (tmap (lambda (field)
+                   ((configuration-field-serializer field)
+                    (configuration-field-name field)
+                    ((configuration-field-getter field) config))))))
+
 (define (serialize-configuration config fields)
+  "Return a G-expression that contains the values corresponding to the
+FIELDS of CONFIG, a record that has been generated by `define-configuration'.
+The G-expression can then be serialized to disk by using something like
+`mixed-text-file'."
   #~(string-append
-     #$@(map (lambda (field)
-               ((configuration-field-serializer field)
-                (configuration-field-name field)
-                ((configuration-field-getter field) config)))
-             fields)))
+     #$@(list-transduce (base-transducer config) rcons fields)))
 
 (define-syntax-rule (id ctx parts ...)
   "Assemble PARTS into a raw (unhygienic) identifier."
diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
index 23ccb8d403..56b7772f58 100644
--- a/gnu/services/telephony.scm
+++ b/gnu/services/telephony.scm
@@ -37,6 +37,7 @@ (define-module (gnu services telephony)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-2)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-171)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (jami-account
@@ -204,22 +205,20 @@ (define (jami-account->alist jami-account-object)
       ('rendezvous-point? "Account.rendezVous")
       ('peer-discovery? "Account.peerDiscovery")
       ('bootstrap-hostnames "Account.hostname")
-      ('name-server-uri "RingNS.uri")
-      (_ #f)))
+      ('name-server-uri "RingNS.uri")))
 
-  (filter-map (lambda (field)
-                (and-let* ((name (field-name->account-detail
+  (define jami-account-transducer
+    (compose (tremove empty-serializer?)
+             (tfilter-maybe-value jami-account-object)
+             (tmap (lambda (field)
+                     (let* ((name (field-name->account-detail
                                   (configuration-field-name field)))
-                           (value ((configuration-field-serializer field)
-                                   name ((configuration-field-getter field)
-                                         jami-account-object)))
-                           ;; The define-maybe default serializer produces an
-                           ;; empty string for unspecified values.
-                           (value* (if (string-null? value)
-                                       #f
-                                       value)))
-                  (cons name value*)))
-              jami-account-fields))
+                            (value ((configuration-field-serializer field)
+                                    name ((configuration-field-getter field)
+                                          jami-account-object))))
+                       (cons name value))))))
+
+  (list-transduce jami-account-transducer rcons jami-account-fields))
 
 (define (jami-account-list? val)
   (and (list? val)
diff --git a/tests/services/configuration.scm b/tests/services/configuration.scm
index 8ad5907f37..40a4e74b4d 100644
--- a/tests/services/configuration.scm
+++ b/tests/services/configuration.scm
@@ -337,13 +337,9 @@ (define-maybe symbol)
 (define-configuration config-with-maybe-symbol
   (protocol maybe-symbol ""))
 
-;;; Maybe symbol values are currently seen as serializable, because the
-;;; unspecified value is '%unset-marker%, which is a symbol itself.
-;;; TODO: Remove expected fail marker after resolution.
-(test-expect-fail 1)
 (test-equal "symbol maybe value serialization, unspecified"
   ""
-  (gexp->approximate-sexp
+  (eval-gexp
    (serialize-configuration (config-with-maybe-symbol)
                             config-with-maybe-symbol-fields)))
 
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v4 1/5] services: configuration: Simplify normalize-extra-args.
  2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
  2023-10-07 15:57   ` [bug#63985] [PATCH v4 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
@ 2023-10-07 15:59   ` Bruno Victal
  2023-10-07 15:59   ` [bug#63985] [PATCH v4 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 15:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/configuration.scm
(define-configuration-helper, normalize-extra-args): Use #f instead of %unset-value.
---
 gnu/services/configuration.scm | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index 367b85c1be..ecc5049a79 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -190,32 +190,32 @@ (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))
+               (sanitizer* #f)
+               (serializer* #f))
       (syntax-case s (sanitizer serializer empty-serializer)
         (((sanitizer proc) tail ...)
-         (if (maybe-value-set? sanitizer*)
-             (syntax-violation 'sanitizer "duplicate entry"
-                               #'proc)
+         (if 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)
+         (if serializer*
+             (syntax-violation 'serializer
+                               "duplicate or conflicting entry" #'proc)
              (loop #'(tail ...) sanitizer* #'proc)))
         ((empty-serializer tail ...)
-         (if (maybe-value-set? serializer*)
+         (if 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*)))
+         (not (or sanitizer* serializer*))
          (begin
            (warning #f (G_ "specifying serializers after documentation is \
 deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc))
-           (values (list %unset-value #'proc)))))))
+           (values (list #f #'proc)))))))
 
   (syntax-case syn ()
     ((_ stem (field field-type+def doc extra-args ...) ...)
@@ -239,11 +239,11 @@ (define (define-configuration-helper serialize? serializer-prefix syn)
                      default-value))
                   #'((field-type def) ...)))
             ((field-sanitizer ...)
-             (map maybe-value #'(sanitizer* ...)))
+             #'(sanitizer* ...))
             ((field-serializer ...)
              (map (lambda (type proc)
                     (and serialize?
-                         (or (maybe-value proc)
+                         (or proc
                              (if serializer-prefix
                                  (id #'stem serializer-prefix #'serialize- type)
                                  (id #'stem #'serialize- type)))))
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v4 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers.
  2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
  2023-10-07 15:57   ` [bug#63985] [PATCH v4 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
  2023-10-07 15:59   ` [bug#63985] [PATCH v4 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
@ 2023-10-07 15:59   ` Bruno Victal
  2023-10-07 15:59   ` [bug#63985] [PATCH v4 4/5] doc: Rewrite define-configuration Bruno Victal
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 15:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

* gnu/services/linux.scm (serialize-fstrim-configuration): Refactor to use
base-transducer.
---
 gnu/services/linux.scm | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/gnu/services/linux.scm b/gnu/services/linux.scm
index 1f01b39a21..9ee0d93030 100644
--- a/gnu/services/linux.scm
+++ b/gnu/services/linux.scm
@@ -41,6 +41,7 @@ (define-module (gnu services linux)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
   #:use-module (srfi srfi-35)
+  #:use-module (srfi srfi-171)
   #:use-module (ice-9 format)
   #:use-module (ice-9 match)
   #:export (earlyoom-configuration
@@ -252,13 +253,9 @@ (define-configuration fstrim-configuration
   (prefix fstrim-))
 
 (define (serialize-fstrim-configuration config)
-  (concatenate
-   (filter list?
-           (map (lambda (field)
-                  ((configuration-field-serializer field)
-                   (configuration-field-name field)
-                   ((configuration-field-getter field) config)))
-                fstrim-configuration-fields))))
+  (list-transduce (compose (base-transducer config) tconcatenate)
+                  rcons
+                  fstrim-configuration-fields))
 
 (define (fstrim-mcron-job config)
   (match-record config <fstrim-configuration> (package schedule)
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v4 4/5] doc: Rewrite define-configuration.
  2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
                     ` (2 preceding siblings ...)
  2023-10-07 15:59   ` [bug#63985] [PATCH v4 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
@ 2023-10-07 15:59   ` Bruno Victal
  2023-10-07 15:59   ` [bug#63985] [PATCH v4 5/5] services: configuration: Add some commonly used predicates Bruno Victal
  2023-10-07 16:57   ` bug#63985: SRFI-171 based improvements for define-configuration Maxim Cournoyer
  5 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 15:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal

Rewrite this section to make it easier to document later syntactical
changes.

* doc/guix.texi (Complex Configurations): Rewrite define-configuration
documentation. Fix simple serializer example.
---
 doc/guix.texi | 102 +++++++++++++++++++++-----------------------------
 1 file changed, 42 insertions(+), 60 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 52a573f0d8..e4a1523dfb 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -42576,54 +42576,33 @@ Complex Configurations
 files, you can use the utilities defined in the @code{(gnu services
 configuration)} module.
 
-The main utility is the @code{define-configuration} macro, which you
-will use to define a Scheme record type (@pxref{Record Overview,,,
-guile, GNU Guile Reference Manual}).  The Scheme record will be
-serialized to a configuration file by using @dfn{serializers}, which are
-procedures that take some kind of Scheme value and returns a
-G-expression (@pxref{G-Expressions}), which should, once serialized to
-the disk, return a string.  More details are listed below.
+The main utility is the @code{define-configuration} macro, a helper
+used to define a Scheme record type (@pxref{Record Overview,,,
+guile, GNU Guile Reference Manual}).  The fields from this Scheme record
+can be serialized using @dfn{serializers}, which are procedures that take
+some kind of Scheme value and translates them into another Scheme value or
+@ref{G-Expressions}.
 
 @defmac define-configuration name clause1 clause2 @dots{}
 Create a record type named @code{@var{name}} that contains the
 fields found in the clauses.
 
-A clause can have one of the following forms:
+A clause has the following form:
 
 @example
 (@var{field-name}
- (@var{type} @var{default-value})
- @var{documentation})
-
-(@var{field-name}
- (@var{type} @var{default-value})
- @var{documentation}
- (serializer @var{serializer}))
-
-(@var{field-name}
- (@var{type})
- @var{documentation})
-
-(@var{field-name}
- (@var{type})
- @var{documentation}
- (serializer @var{serializer}))
-
-(@var{field-name}
- (@var{type})
+ @var{type-decl}
  @var{documentation}
- (sanitizer @var{sanitizer})
-
-(@var{field-name}
- (@var{type})
- @var{documentation}
- (sanitizer @var{sanitizer})
- (serializer @var{serializer}))
+ @var{option*}
+ @dots{})
 @end example
 
 @var{field-name} is an identifier that denotes the name of the field in
 the generated record.
 
+@var{type-decl} is either @code{@var{type}} for fields that require a
+value to be set or @code{(@var{type} @var{default})} otherwise.
+
 @var{type} is the type of the value corresponding to @var{field-name};
 since Guile is untyped, a predicate
 procedure---@code{@var{type}?}---will be called on the value
@@ -42641,6 +42620,28 @@ Complex Configurations
 @var{documentation} is a string formatted with Texinfo syntax which
 should provide a description of what setting this field does.
 
+@var{option*} is one of the following subclauses:
+
+@table @asis
+@item @code{empty-serializer}
+Exclude this field from serialization.
+
+@item @code{(serializer @var{serializer})}
+@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
+@ref{G-Expressions} that represents the content that will be serialized
+to the configuration file.  If none is specified, a procedure of the
+name @code{serialize-@var{type}} will be used.
+
+An example of a simple serializer procedure:
+@lisp
+(define (serialize-boolean field-name value)
+  (let ((value (if value "true" "false")))
+    #~(string-append '#$field-name " = " #$value)))
+@end lisp
+
+@item @code{(sanitizer @var{sanitizer})}
 @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
@@ -42654,21 +42655,7 @@ Complex Configurations
         ((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
-G-expression (@pxref{G-Expressions}) that represents the content that
-will be serialized to the configuration file.  If none is specified, a
-procedure of the name @code{serialize-@var{type}} will be used.
-
-A simple serializer procedure could look like this:
-
-@lisp
-(define (serialize-boolean field-name value)
-  (let ((value (if value "true" "false")))
-    #~(string-append #$field-name #$value)))
-@end lisp
+@end table
 
 In some cases multiple different configuration records might be defined
 in the same file, but their serializers for the same type might have to
@@ -42689,13 +42676,13 @@ Complex Configurations
 
 (define-configuration foo-configuration
   (label
-   (string)
+   string
    "The name of label.")
   (prefix foo-))
 
 (define-configuration bar-configuration
   (ip-address
-   (string)
+   string
    "The IPv4 address for this device.")
   (prefix bar-))
 @end lisp
@@ -42787,11 +42774,6 @@ Complex Configurations
 disk by using something like @code{mixed-text-file}.
 @end deffn
 
-@deffn {Procedure} empty-serializer field-name value
-A serializer that just returns an empty string.  The
-@code{serialize-package} procedure is an alias for this.
-@end deffn
-
 Once you have defined a configuration record, you will most likely also
 want to document it so that other people know to use it.  To help with
 that, there are two procedures, both of which are documented below.
@@ -42894,7 +42876,7 @@ Complex Configurations
 
 (define-configuration contact-configuration
   (name
-   (string)
+   string
    "The name of the contact."
    serialize-contact-name)
   (phone-number
@@ -42904,15 +42886,15 @@ Complex Configurations
    maybe-string
    "The person's email address.")
   (married?
-   (boolean)
+   boolean
    "Whether the person is married."))
 
 (define-configuration contacts-list-configuration
   (name
-   (string)
+   string
    "The name of the owner of this contact list.")
   (email
-   (string)
+   string
    "The owner's email address.")
   (contacts
    (list-of-contact-configurations '())
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH v4 5/5] services: configuration: Add some commonly used predicates.
  2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
                     ` (3 preceding siblings ...)
  2023-10-07 15:59   ` [bug#63985] [PATCH v4 4/5] doc: Rewrite define-configuration Bruno Victal
@ 2023-10-07 15:59   ` Bruno Victal
  2023-10-07 16:57   ` bug#63985: SRFI-171 based improvements for define-configuration Maxim Cournoyer
  5 siblings, 0 replies; 50+ messages in thread
From: Bruno Victal @ 2023-10-07 15:59 UTC (permalink / raw)
  To: 63985; +Cc: Bruno Victal, Maxim Cournoyer

* gnu/services/configuration.scm (list-of-packages?, list-of-symbols?): New
predicate.
* gnu/services/audio.scm (list-of-symbol?): Remove.
* gnu/services/telephony.scm (string-list?): Remove.
(serialize-string-list): Rename to …
(serialize-list-of-strings): … this.
(account-fingerprint-list?, jami-account-list?): Use list-of.
* doc/guix.texi: Update it.
---
 doc/guix.texi                  |  6 +++---
 gnu/services/audio.scm         |  7 ++-----
 gnu/services/configuration.scm | 17 +++++++++++++++++
 gnu/services/telephony.scm     | 20 +++++++-------------
 4 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index e4a1523dfb..1deb784f56 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -28188,7 +28188,7 @@ Telephony Services
 connection to the Internet has been lost.  When left unspecified,
 the value from the account archive prevails.
 
-@item @code{bootstrap-hostnames} (type: maybe-string-list)
+@item @code{bootstrap-hostnames} (type: maybe-list-of-strings)
 A list of hostnames or IPs pointing to OpenDHT nodes, that should be
 used to initially join the OpenDHT network.  When left unspecified, the
 value from the account archive prevails.
@@ -34509,7 +34509,7 @@ Audio Services
 
 The default @code{%mpd-group} is a system group with name ``mpd''.
 
-@item @code{shepherd-requirement} (default: @code{'()}) (type: list-of-symbol)
+@item @code{shepherd-requirement} (default: @code{'()}) (type: list-of-symbols)
 A list of symbols naming Shepherd services that this service
 will depend on.
 
@@ -34759,7 +34759,7 @@ Audio Services
 @item @code{package} (default: @code{mympd}) (type: file-like)
 The package object of the myMPD server.
 
-@item @code{shepherd-requirement} (default: @code{'()}) (type: list-of-symbol)
+@item @code{shepherd-requirement} (default: @code{'()}) (type: list-of-symbols)
 This is a list of symbols naming Shepherd services that this service
 will depend on.
 
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 260abdefed..ae991ced4d 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -138,9 +138,6 @@ (define (uglify-field-name field-name)
                                    str)
                                #\-) "_")))
 
-(define list-of-symbol?
-  (list-of symbol?))
-
 ;; Helpers for deprecated field types, to be removed later.
 (define %lazy-group (make-symbol "%lazy-group"))
 
@@ -428,7 +425,7 @@ (define-configuration mpd-configuration
    (sanitizer mpd-group-sanitizer))
 
   (shepherd-requirement
-   (list-of-symbol '())
+   (list-of-symbols '())
    "This is a list of symbols naming Shepherd services that this service
 will depend on."
    empty-serializer)
@@ -763,7 +760,7 @@ (define-configuration/no-serialization mympd-configuration
     empty-serializer)
 
   (shepherd-requirement
-   (list-of-symbol '())
+   (list-of-symbols '())
    "This is a list of symbols naming Shepherd services that this service
 will depend on."
    empty-serializer)
diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm
index aa5fb832d5..d2b1687496 100644
--- a/gnu/services/configuration.scm
+++ b/gnu/services/configuration.scm
@@ -80,7 +80,9 @@ (define-module (gnu services configuration)
             interpose
             list-of
 
+            list-of-packages?
             list-of-strings?
+            list-of-symbols?
             alist?
             serialize-file-like
             text-config?
@@ -500,6 +502,11 @@ (define* (interpose ls  #:optional (delimiter "\n") (grammar 'infix))
                           (cons delimiter acc))))
               '() ls))
 
+\f
+;;;
+;;; Commonly used predicates
+;;;
+
 (define (list-of pred?)
   "Return a procedure that takes a list and check if all the elements of
 the list result in @code{#t} when applying PRED? on them."
@@ -508,10 +515,20 @@ (define (list-of pred?)
           (every pred? x)
           #f)))
 
+(define list-of-packages?
+  (list-of package?))
 
 (define list-of-strings?
   (list-of string?))
 
+(define list-of-symbols?
+  (list-of symbol?))
+
+\f
+;;;
+;;; Special serializers
+;;;
+
 (define alist?
   (list-of pair?))
 
diff --git a/gnu/services/telephony.scm b/gnu/services/telephony.scm
index 56b7772f58..c9b5d6cd99 100644
--- a/gnu/services/telephony.scm
+++ b/gnu/services/telephony.scm
@@ -117,15 +117,10 @@ (define (string-or-computed-file? val)
   (or (string? val)
       (computed-file? val)))
 
-(define (string-list? val)
-  (and (list? val)
-       (and-map string? val)))
+(define account-fingerprint-list?
+  (list-of account-fingerprint?))
 
-(define (account-fingerprint-list? val)
-  (and (list? val)
-       (and-map account-fingerprint? val)))
-
-(define-maybe string-list)
+(define-maybe list-of-strings)
 
 (define-maybe/no-serialization account-fingerprint-list)
 
@@ -135,7 +130,7 @@ (define-maybe string)
 
 ;;; The following serializers are used to derive an account details alist from
 ;;; a <jami-account> record.
-(define (serialize-string-list _ val)
+(define (serialize-list-of-strings _ val)
   (string-join val ";"))
 
 (define (serialize-boolean _ val)
@@ -188,7 +183,7 @@ (define-configuration jami-account
 connection to the the Internet has been lost.  When left unspecified, the
 value from the account archive prevails.")
   (bootstrap-hostnames
-   maybe-string-list
+   maybe-list-of-strings
    "A list of hostnames or IPs pointing to OpenDHT nodes, that should be used
 to initially join the OpenDHT network.  When left unspecified, the value from
 the account archive prevails.")
@@ -220,9 +215,8 @@ (define (jami-account->alist jami-account-object)
 
   (list-transduce jami-account-transducer rcons jami-account-fields))
 
-(define (jami-account-list? val)
-  (and (list? val)
-       (and-map jami-account? val)))
+(define jami-account-list?
+  (list-of jami-account?))
 
 (define-maybe/no-serialization jami-account-list)
 
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration
  2023-10-07 14:21       ` Bruno Victal
@ 2023-10-07 16:35         ` Maxim Cournoyer
  0 siblings, 0 replies; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-07 16:35 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985

Hi,

Bruno Victal <mirai@makinata.eu> writes:

> On 2023-10-02 19:28, Maxim Cournoyer wrote:
>>> +An example of a simple serializer procedure:
>>> +@lisp
>>> +(define (serialize-boolean field-name value)
>>> +  (let ((value (if value "true" "false")))
>>> +    #~(string-append '#$field-name #$value)))
>>> +@end lisp
>>> +
>> 
>> I know this is adapted from old code, but shouldn't there be a "=" in
>> that string-append, between the field name and its value? […]
>
> I think there's no particular meaning for that isolated snippet but I
> believe the original author did intend for there to be a "=" symbol as
> you have pointed out. I've went ahead and added the “ = ” (with spaces)
> variant instead.
>
>> […] Also, using gexps here seems unnecessary and may confuse the reader.
>
> I don't think so because in general you're going to call
> `serialize-configuration' which returns a G-Expression so it's more or
> less given that using G-Expressions is perfectly valid and using them
> from the outset allows your fields to serialize any kind of value such as
> package objects (for something like `gcc-path = /gnu/store/…'), etc.
>
> Thus, not using G-Exps is more restrictive in terms of what you can
> serialize. It might not make a difference in some cases but I think
> this is a potential source of trouble when someone tries to serialize
> a gexp object with a non-gexp ready serializer. (e.g. performing
> string-append with a string and a package object)

Agreed.

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

* bug#63985: SRFI-171 based improvements for define-configuration
  2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
                     ` (4 preceding siblings ...)
  2023-10-07 15:59   ` [bug#63985] [PATCH v4 5/5] services: configuration: Add some commonly used predicates Bruno Victal
@ 2023-10-07 16:57   ` Maxim Cournoyer
  5 siblings, 0 replies; 50+ messages in thread
From: Maxim Cournoyer @ 2023-10-07 16:57 UTC (permalink / raw)
  To: Bruno Victal; +Cc: 63985-done, Liliana Prikler

Hello,

Bruno Victal <mirai@makinata.eu> writes:

[...]

> Bruno Victal (5):
>   services: configuration: Simplify normalize-extra-args.
>   services: configuration: Use transducers within
>     serialize-configuration.
>   services: fstrim-service-type: Serialize with SRFI-171 transducers.
>   doc: Rewrite define-configuration.
>   services: configuration: Add some commonly used predicates.

Now pushed!

-- 
Thanks,
Maxim




^ permalink raw reply	[flat|nested] 50+ messages in thread

end of thread, other threads:[~2023-10-07 16:59 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-09 21:18 [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Bruno Victal
2023-06-09 21:20 ` [bug#63985] [PATCH RFC 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
2023-06-09 21:20 ` [bug#63985] [PATCH RFC 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
2023-06-09 21:20 ` [bug#63985] [PATCH RFC 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
2023-06-09 21:20 ` [bug#63985] [PATCH RFC 4/5] services: configuration: Add serializer-kwargs field Bruno Victal
2023-06-09 21:21 ` [bug#63985] [PATCH RFC 5/5] services: configuration: New generic-ini module Bruno Victal
2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 4/5] services: configuration: Add serializer-options field Bruno Victal
2023-06-10 20:10 ` [bug#63985] [PATCH RFC v2 5/5] services: configuration: New generic-ini module Bruno Victal
2023-06-26 21:57 ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Bruno Victal
2023-06-26 21:59   ` [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args Bruno Victal
2023-10-02 17:00     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
2023-10-07 12:36       ` [bug#63985] [PATCH v3 01/11] services: configuration: Simplify normalize-extra-args. (was: bug#63985: [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration) Bruno Victal
2023-06-26 21:59   ` [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration Bruno Victal
2023-10-02 17:25     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
2023-10-07 13:39       ` [bug#63985] [PATCH v3 02/11] services: configuration: Use transducers within serialize-configuration. (was : bug#63985: [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration) Bruno Victal
2023-10-07 14:37         ` Maxim Cournoyer
2023-06-26 21:59   ` [bug#63985] [PATCH v3 03/11] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
2023-06-26 21:59   ` [bug#63985] [PATCH v3 04/11] doc: Rewrite define-configuration Bruno Victal
2023-10-02 18:28     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
2023-10-07 14:21       ` Bruno Victal
2023-10-07 16:35         ` Maxim Cournoyer
2023-06-26 21:59   ` [bug#63985] [PATCH v3 05/11] services: configuration: Add serializer-options field Bruno Victal
2023-10-02 19:12     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
2023-10-06 18:29       ` Bruno Victal
2023-06-26 21:59   ` [bug#63985] [PATCH v3 06/11] services: configuration: New generic-ini module Bruno Victal
2023-10-02 19:15     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
2023-06-26 21:59   ` [bug#63985] [PATCH v3 07/11] services: configuration: Add some commonly used predicates Bruno Victal
2023-06-26 21:59   ` [bug#63985] [PATCH v3 08/11] services: NetworkManager: Use define-configuration and generic-ini Bruno Victal
2023-06-26 21:59   ` [bug#63985] [PATCH v3 09/11] services: NetworkManager: Prefer package over network-manager Bruno Victal
2023-10-02 16:52     ` [bug#63985] [PATCH RFC 0/5] Generic INI serializer & SRFI-171 for define-configuration Maxim Cournoyer
2023-06-26 21:59   ` [bug#63985] [PATCH v3 10/11] services: NetworkManager: add log-configuration field Bruno Victal
2023-10-05 16:57     ` Maxim Cournoyer
2023-06-26 21:59   ` [bug#63985] [PATCH v3 11/11] services: NetworkManager: Add extra-options field Bruno Victal
2023-10-05 16:59     ` Maxim Cournoyer
2023-06-27  4:20   ` [bug#63985] [PATCH v3 00/11] Service subsystem improvements Liliana Marie Prikler
2023-09-16 21:22   ` Bruno Victal
2023-09-16 21:55     ` Liliana Marie Prikler
2023-09-23 13:35       ` Bruno Victal
2023-09-23 15:22         ` Liliana Marie Prikler
2023-09-25 14:06       ` Ludovic Courtès
2023-10-07 15:57 ` [bug#63985] [PATCH v4 0/5] SRFI-171 based improvements for define-configuration Bruno Victal
2023-10-07 15:57   ` [bug#63985] [PATCH v4 2/5] services: configuration: Use transducers within serialize-configuration Bruno Victal
2023-10-07 15:59   ` [bug#63985] [PATCH v4 1/5] services: configuration: Simplify normalize-extra-args Bruno Victal
2023-10-07 15:59   ` [bug#63985] [PATCH v4 3/5] services: fstrim-service-type: Serialize with SRFI-171 transducers Bruno Victal
2023-10-07 15:59   ` [bug#63985] [PATCH v4 4/5] doc: Rewrite define-configuration Bruno Victal
2023-10-07 15:59   ` [bug#63985] [PATCH v4 5/5] services: configuration: Add some commonly used predicates Bruno Victal
2023-10-07 16:57   ` bug#63985: SRFI-171 based improvements for define-configuration Maxim Cournoyer

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).