Re: Add a way to disable serialization support to (guix services configuration)
Hi, Xinglu Chen writes: [...] > From 90d63a46a29a8080b7f95eabcec115c5c2c6481e Mon Sep 17 00:00:00 2001 > Message-Id: > <90d63a46a29a8080b7f95eabcec115c5c2c6481e.1619869705.git.pub...@yoctocell.xyz> > In-Reply-To: > References: > From: Xinglu Chen > Date: Sat, 1 May 2021 13:31:27 +0200 > Subject: [PATCH 2/2] services: configuration: Add ability to use custom > serializer. > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > Some configuration values should not be serialized to a configuration file, > e.g. command line options for a daemon. > > * gnu/services/configuration.scm (define-configuration): Add option to use a > custom serializer. > (empty-serializer): New procedure. > (serialize-package): Alias to ‘empty-serializer’. > --- > gnu/services/configuration.scm | 16 ++-- > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm > index 85e1ac78cb..7024b13136 100644 > --- a/gnu/services/configuration.scm > +++ b/gnu/services/configuration.scm > @@ -44,6 +44,7 @@ > define-configuration > validate-configuration > generate-documentation > +empty-serializer > serialize-package)) > > ;;; Commentary: > @@ -116,7 +117,7 @@ > (define-syntax define-configuration >(lambda (stx) > (syntax-case stx () > - ((_ stem (field (field-type properties ...) doc) ...) > + ((_ stem (field (field-type properties ...) doc custom-serializer ...) > ...) > (with-syntax (((field-getter ...) >(map (lambda (field) > (id #'stem #'stem #'- field)) > @@ -133,9 +134,12 @@ > (_ (syntax 'disabled))) > #'((field-type properties ...) ...))) > ((field-serializer ...) > - (map (lambda (type) > - (id #'stem #'serialize- type)) > - #'(field-type ... > + (map (lambda (type serializer) > + (match serializer > + (((serializer)) serializer) > + (_ (id #'stem #'serialize- type > + #'(field-type ...) > + #'((custom-serializer ...) ... > #`(begin > (define-record-type* #,(id #'stem #'< #'stem #'>) > #,(id #'stem #'% #'stem) > @@ -176,8 +180,8 @@ > #,(id #'stem #'stem #'-fields)) > conf > > -(define (serialize-package field-name val) > - "") > +(define (empty-serializer field-name val) "") > +(define serialize-package empty-serializer) > > ;; A little helper to make it easier to document all those fields. > (define (generate-documentation documentation documentation-name) Rebased on a change that allows to globally (at the configuration level) disable serialization and pushed as b3e99d3399. Thank you! :-) Maxim
Re: Add a way to disable serialization support to (guix services configuration)
Hi Maxim, On Fri, May 07 2021, Maxim Cournoyer wrote: > Hello Xinglu! > > Thank you for working on it! You are very welcome! These are things that have annoyed me enough so I decided (try) to fix it myself :) >> +(define (configuration-no-default-value kind field) >> + (configuration-error >> + (format #f "`~a' in `~a' does not have a default value" kind field))) > > The kind and field should be inverted. Good catch >>configuration-field make-configuration-field configuration-field? >> @@ -112,7 +116,7 @@ >> (define-syntax define-configuration >>(lambda (stx) >> (syntax-case stx () >> - ((_ stem (field (field-type def) doc) ...) >> + ((_ stem (field (field-type properties ...) doc) ...) > > I'd rather keep the 'def' binding for the default value; properties is > too vague and implies many of them, which is not a supported syntax. Yeah, not exactly sure why I changed it to ‘properties’, anyway... >> (with-syntax (((field-getter ...) >>(map (lambda (field) >> (id #'stem #'stem #'- field)) >> @@ -121,36 +125,56 @@ >>(map (lambda (type) >> (id #'stem type #'?)) >> #'(field-type ...))) >> + ((field-default ...) >> + (map (match-lambda >> + ((field-type default _ ...) default) >> + ;; We get warnings about `disabled' being an >> + ;; unbound variable unless we quote it. >> + (_ (syntax 'disabled))) > > Here I think it'd be better to have the pattern more strict (e.g, > (field-type default-value) or (field-type); so as to not accept invalid > syntax. Good point! > I also think it'd be clearer to use another symbol than 'disabled, as > this already has a meaning for the validator and could confuse readers. Yeah, it’s not very descriptive. >> + #'((field-type properties ...) ...))) >> ((field-serializer ...) >>(map (lambda (type) >> (id #'stem #'serialize- type)) >> #'(field-type ... >> - #`(begin >> - (define-record-type* #,(id #'stem #'< #'stem #'>) >> - #,(id #'stem #'% #'stem) >> - #,(id #'stem #'make- #'stem) >> - #,(id #'stem #'stem #'?) >> - (%location #,(id #'stem #'-location) >> -(default (and=> (current-source-location) >> -source-properties->location)) >> -(innate)) >> - (field field-getter (default def)) >> - ...) >> - (define #,(id #'stem #'stem #'-fields) >> - (list (configuration-field >> -(name 'field) >> -(type 'field-type) >> -(getter field-getter) >> -(predicate field-predicate) >> -(serializer field-serializer) >> -(default-value-thunk (lambda () def)) >> -(documentation doc)) >> - ...)) >> - (define-syntax-rule (stem arg (... ...)) >> - (let ((conf (#,(id #'stem #'% #'stem) arg (... ... >> - (validate-configuration conf >> - #,(id #'stem #'stem #'-fields)) >> - conf >> + #`(begin >> + (define-record-type* #,(id #'stem #'< #'stem #'>) >> + #,(id #'stem #'% #'stem) >> + #,(id #'stem #'make- #'stem) >> + #,(id #'stem #'stem #'?) >> + (%location #,(id #'stem #'-location) >> + (default (and=> (current-source-location) >> + source-properties->location)) >> + (innate)) >> + #,@(map (lambda (name getter def) >> + (if (equal? (syntax->datum def) (quote 'disabled)) > > nitpick: eq? suffices to check for symbols. > >> + #`(#,name #,getter) >> + #`(#,name #,getter (default #,def >> + #'(field ...) >> + #'(field-getter ...) >> + #'(field-default ...))) >> + (define #,(id #'stem #'stem #'-fields) >> + (list (configuration-field >> + (name 'field) >> + (type 'field-type) >> + (getter field-getter) >> + (predicate field-predicate) >> + (serializer field-serializer) >> + ;; TODO: What if there is no default value? > > Seems
Re: Add a way to disable serialization support to (guix services configuration)
Hello Xinglu! Thank you for working on it! I spent the evening trying things but none worked, so your kudos for finding how to make it work! :-). Some comments follow (and a patch implementing them): Xinglu Chen writes: [...] > @@ -63,6 +64,9 @@ > (define (configuration-missing-field kind field) >(configuration-error > (format #f "~a configuration missing required field ~a" kind field))) > +(define (configuration-no-default-value kind field) > + (configuration-error > + (format #f "`~a' in `~a' does not have a default value" kind field))) The kind and field should be inverted. > (define-record-type* >configuration-field make-configuration-field configuration-field? > @@ -112,7 +116,7 @@ > (define-syntax define-configuration >(lambda (stx) > (syntax-case stx () > - ((_ stem (field (field-type def) doc) ...) > + ((_ stem (field (field-type properties ...) doc) ...) I'd rather keep the 'def' binding for the default value; properties is too vague and implies many of them, which is not a supported syntax. > (with-syntax (((field-getter ...) >(map (lambda (field) > (id #'stem #'stem #'- field)) > @@ -121,36 +125,56 @@ >(map (lambda (type) > (id #'stem type #'?)) > #'(field-type ...))) > + ((field-default ...) > + (map (match-lambda > + ((field-type default _ ...) default) > + ;; We get warnings about `disabled' being an > + ;; unbound variable unless we quote it. > + (_ (syntax 'disabled))) Here I think it'd be better to have the pattern more strict (e.g, (field-type default-value) or (field-type); so as to not accept invalid syntax. I also think it'd be clearer to use another symbol than 'disabled, as this already has a meaning for the validator and could confuse readers. > + #'((field-type properties ...) ...))) > ((field-serializer ...) >(map (lambda (type) > (id #'stem #'serialize- type)) > #'(field-type ... > - #`(begin > - (define-record-type* #,(id #'stem #'< #'stem #'>) > - #,(id #'stem #'% #'stem) > - #,(id #'stem #'make- #'stem) > - #,(id #'stem #'stem #'?) > - (%location #,(id #'stem #'-location) > -(default (and=> (current-source-location) > -source-properties->location)) > -(innate)) > - (field field-getter (default def)) > - ...) > - (define #,(id #'stem #'stem #'-fields) > - (list (configuration-field > -(name 'field) > -(type 'field-type) > -(getter field-getter) > -(predicate field-predicate) > -(serializer field-serializer) > -(default-value-thunk (lambda () def)) > -(documentation doc)) > - ...)) > - (define-syntax-rule (stem arg (... ...)) > - (let ((conf (#,(id #'stem #'% #'stem) arg (... ... > - (validate-configuration conf > - #,(id #'stem #'stem #'-fields)) > - conf > + #`(begin > + (define-record-type* #,(id #'stem #'< #'stem #'>) > + #,(id #'stem #'% #'stem) > + #,(id #'stem #'make- #'stem) > + #,(id #'stem #'stem #'?) > + (%location #,(id #'stem #'-location) > + (default (and=> (current-source-location) > + source-properties->location)) > + (innate)) > + #,@(map (lambda (name getter def) > + (if (equal? (syntax->datum def) (quote 'disabled)) nitpick: eq? suffices to check for symbols. > + #`(#,name #,getter) > + #`(#,name #,getter (default #,def > + #'(field ...) > + #'(field-getter ...) > + #'(field-default ...))) > + (define #,(id #'stem #'stem #'-fields) > + (list (configuration-field > + (name 'field) > + (type 'field-type) > + (getter field-getter) > + (predicate field-predicate) > + (serializer field-serializer) > + ;; TODO: What if there is no default value? Seems this TODO was
Re: Add a way to disable serialization support to (guix services configuration)
Hi, On Fri, Apr 23 2021, Xinglu Chen wrote: >> Wouldn’t it be nicer to write: >> >> (define-configuration foo >> (bar (integer 123) "doc" no-serializer) >> (baz (string "") "doc")) >> >> where ‘bar’ wouldn’t have a serializer and ‘baz’ would? >> >> It’s also probably easier to implement correctly. > > I think that would be a good idea, maybe it could also make having a > default value be optional, like this: > > #+begin_src scheme > (define-configuration foo > (bar (integer) "doc" no-serializer) ;no default > (baz (string "default") "doc")) > #+end_src Since nobody seemed to show any objections, I went ahead and did something like this myself (see the attached patches). My macros skills aren’t that good so I hope I didn’t make any obvious mistakes. :) I haven’t tested it thoroughly, but at least it didn’t break anything when I ran ‘./pre-inst-env guix home build ...’. :) >From 231281ebf555295e83513873293a1ad3eab884a8 Mon Sep 17 00:00:00 2001 Message-Id: <231281ebf555295e83513873293a1ad3eab884a8.1619869705.git.pub...@yoctocell.xyz> In-Reply-To: References: From: Xinglu Chen Date: Sat, 1 May 2021 13:24:43 +0200 Subject: [PATCH 1/2] services: configuration: Support fields without default values. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not all fields in a configuration have a sensible default value, e.g. ‘user.name’ in gitconfig, the user should have to set that themselves * gnu/services/configuration.scm (configuration-missing-field): New procedure. (define-configuration): Make default value optional. --- gnu/services/configuration.scm | 78 ++ 1 file changed, 51 insertions(+), 27 deletions(-) diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm index 90f12a8d39..85e1ac78cb 100644 --- a/gnu/services/configuration.scm +++ b/gnu/services/configuration.scm @@ -2,6 +2,7 @@ ;;; Copyright © 2015 Andy Wingo ;;; Copyright © 2017 Mathieu Othacehe ;;; Copyright © 2017, 2018 Clément Lassieur +;;; Copyright © 2021 Xinglu Chen ;;; ;;; This file is part of GNU Guix. ;;; @@ -63,6 +64,9 @@ (define (configuration-missing-field kind field) (configuration-error (format #f "~a configuration missing required field ~a" kind field))) +(define (configuration-no-default-value kind field) + (configuration-error + (format #f "`~a' in `~a' does not have a default value" kind field))) (define-record-type* configuration-field make-configuration-field configuration-field? @@ -112,7 +116,7 @@ (define-syntax define-configuration (lambda (stx) (syntax-case stx () - ((_ stem (field (field-type def) doc) ...) + ((_ stem (field (field-type properties ...) doc) ...) (with-syntax (((field-getter ...) (map (lambda (field) (id #'stem #'stem #'- field)) @@ -121,36 +125,56 @@ (map (lambda (type) (id #'stem type #'?)) #'(field-type ...))) + ((field-default ...) + (map (match-lambda + ((field-type default _ ...) default) + ;; We get warnings about `disabled' being an + ;; unbound variable unless we quote it. + (_ (syntax 'disabled))) + #'((field-type properties ...) ...))) ((field-serializer ...) (map (lambda (type) (id #'stem #'serialize- type)) #'(field-type ... - #`(begin - (define-record-type* #,(id #'stem #'< #'stem #'>) - #,(id #'stem #'% #'stem) - #,(id #'stem #'make- #'stem) - #,(id #'stem #'stem #'?) - (%location #,(id #'stem #'-location) -(default (and=> (current-source-location) -source-properties->location)) -(innate)) - (field field-getter (default def)) - ...) - (define #,(id #'stem #'stem #'-fields) - (list (configuration-field -(name 'field) -(type 'field-type) -(getter field-getter) -(predicate field-predicate) -(serializer field-serializer) -(default-value-thunk (lambda () def)) -(documentation doc)) - ...)) - (define-syntax-rule (stem arg (... ...)) - (let ((conf (#,(id #'stem #'% #'stem) arg (... ... - (validate-configuration conf - #,(id #'stem #'stem #'-fields)) - conf +
Re: Add a way to disable serialization support to (guix services configuration)
Hi, On Fri, Apr 23 2021, Ludovic Courtès wrote: > Hi, > > Maxim Cournoyer skribis: > >>> +(define-syntax-rule (without-field-serialization definition) >>> + (syntax-parameterize ((configuration-field-serialization? >>> + (identifier-syntax #f))) >>> +definition >>> +#t)) >>> + >>> +(without-field-serialization >>> + (define-configuration foo >>> +(bar (integer 123) "doc"))) > > In hindsight, I find this syntax quite inelegant and suboptimal. > > Wouldn’t it be nicer to write: > > (define-configuration foo > (bar (integer 123) "doc" no-serializer) > (baz (string "") "doc")) > > where ‘bar’ wouldn’t have a serializer and ‘baz’ would? > > It’s also probably easier to implement correctly. I think that would be a good idea, maybe it could also make having a default value be optional, like this: #+begin_src scheme (define-configuration foo (bar (integer) "doc" no-serializer) ;no default (baz (string "default") "doc")) #+end_src Maxim and I had a discussion about this[1]. [1]: https://yhetil.org/guix-devel/87k0ov7w72@yoctocell.xyz
Re: Add a way to disable serialization support to (guix services configuration)
Hi, Maxim Cournoyer skribis: >> +(define-syntax-rule (without-field-serialization definition) >> + (syntax-parameterize ((configuration-field-serialization? >> + (identifier-syntax #f))) >> +definition >> +#t)) >> + >> +(without-field-serialization >> + (define-configuration foo >> +(bar (integer 123) "doc"))) In hindsight, I find this syntax quite inelegant and suboptimal. Wouldn’t it be nicer to write: (define-configuration foo (bar (integer 123) "doc" no-serializer) (baz (string "") "doc")) where ‘bar’ wouldn’t have a serializer and ‘baz’ would? It’s also probably easier to implement correctly. Ludo’.
Re: Add a way to disable serialization support to (guix services configuration)
Hello again, Ludovic Courtès writes: [...] > diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm > index 90f12a8d39..20e1647335 100644 > --- a/gnu/services/configuration.scm > +++ b/gnu/services/configuration.scm > @@ -109,6 +109,9 @@ > (define (serialize-maybe-stem field-name val) > (if (stem? val) (serialize-stem field-name val) "" > > +(define-syntax-parameter configuration-field-serialization? > + (identifier-syntax #t)) > + > (define-syntax define-configuration >(lambda (stx) > (syntax-case stx () > @@ -123,7 +126,8 @@ > #'(field-type ...))) > ((field-serializer ...) >(map (lambda (type) > - (id #'stem #'serialize- type)) > + #`(and configuration-field-serialization? > +#,(id #'stem #'serialize- type))) > #'(field-type ... > #`(begin > (define-record-type* #,(id #'stem #'< #'stem #'>) > @@ -152,6 +156,16 @@ > #,(id #'stem #'stem #'-fields)) > conf > > +(define-syntax-rule (without-field-serialization definition) > + (syntax-parameterize ((configuration-field-serialization? > + (identifier-syntax #f))) > +definition > +#t)) > + > +(without-field-serialization > + (define-configuration foo > +(bar (integer 123) "doc"))) > + > (define (serialize-package field-name val) >"") Actually, this doesn't seem to work; when exporting the 'without-field-serialization' from this module and using it in another module (wrapping a (define-configuration ...) form), the compiler complains that the 'foo' variable is not defined; perhaps because foo is exported as part of the module definition? If you have an idea why, hints welcome. Maxim
Re: Add a way to disable serialization support to (guix services configuration)
Hi Ludovic, Ludovic Courtès writes: > Hi Maxim, > > Maxim Cournoyer skribis: > >> I've rediscovered the little gem that is (guix services configurations), >> and attempted to make it more generally useful by adding an option to >> opt out of serialization (which is not well adapted for producing a list >> of command line arguments from the configuration for example): > > In the meantime I saw you discuss this on #guile so maybe you’ve solved > the issue now? No, thank you for your reply! :-) > If not, attached is a possible solution and example. It’s not perfect: > in the example, you’ll get a warning about ‘serialize-integer’ being > unbound, which is completely harmless but suboptimal. Not sure how to > address that. > > diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm > index 90f12a8d39..20e1647335 100644 > --- a/gnu/services/configuration.scm > +++ b/gnu/services/configuration.scm > @@ -109,6 +109,9 @@ > (define (serialize-maybe-stem field-name val) > (if (stem? val) (serialize-stem field-name val) "" > > +(define-syntax-parameter configuration-field-serialization? > + (identifier-syntax #t)) > + > (define-syntax define-configuration >(lambda (stx) > (syntax-case stx () > @@ -123,7 +126,8 @@ > #'(field-type ...))) > ((field-serializer ...) >(map (lambda (type) > - (id #'stem #'serialize- type)) > + #`(and configuration-field-serialization? > +#,(id #'stem #'serialize- type))) I tried: (if (syntax->datum configuration-field-serialization?) #,(id #'stem #'serialize- type) #false) hoping to get rid of the warning, but that appears to be strictly equivalent, resulting in the same warning. I also don't know how to fix that. > #'(field-type ... > #`(begin > (define-record-type* #,(id #'stem #'< #'stem #'>) > @@ -152,6 +156,16 @@ > #,(id #'stem #'stem #'-fields)) > conf > > +(define-syntax-rule (without-field-serialization definition) > + (syntax-parameterize ((configuration-field-serialization? > + (identifier-syntax #f))) > +definition > +#t)) > + > +(without-field-serialization > + (define-configuration foo > +(bar (integer 123) "doc"))) > + > (define (serialize-package field-name val) >"") That does work, thanks! I'm a bit uncomfortable adding more warnings to the Guix build output; OTOH it provides value (another user recently pointed to the same annoyance here [0]). Maxim [0] https://lists.gnu.org/archive/html/guix-devel/2021-04/msg00319.html
Re: Add a way to disable serialization support to (guix services configuration)
Hi Maxim, Maxim Cournoyer skribis: > I've rediscovered the little gem that is (guix services configurations), > and attempted to make it more generally useful by adding an option to > opt out of serialization (which is not well adapted for producing a list > of command line arguments from the configuration for example): In the meantime I saw you discuss this on #guile so maybe you’ve solved the issue now? If not, attached is a possible solution and example. It’s not perfect: in the example, you’ll get a warning about ‘serialize-integer’ being unbound, which is completely harmless but suboptimal. Not sure how to address that. HTH! Ludo’. diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.scm index 90f12a8d39..20e1647335 100644 --- a/gnu/services/configuration.scm +++ b/gnu/services/configuration.scm @@ -109,6 +109,9 @@ (define (serialize-maybe-stem field-name val) (if (stem? val) (serialize-stem field-name val) "" +(define-syntax-parameter configuration-field-serialization? + (identifier-syntax #t)) + (define-syntax define-configuration (lambda (stx) (syntax-case stx () @@ -123,7 +126,8 @@ #'(field-type ...))) ((field-serializer ...) (map (lambda (type) - (id #'stem #'serialize- type)) + #`(and configuration-field-serialization? +#,(id #'stem #'serialize- type))) #'(field-type ... #`(begin (define-record-type* #,(id #'stem #'< #'stem #'>) @@ -152,6 +156,16 @@ #,(id #'stem #'stem #'-fields)) conf +(define-syntax-rule (without-field-serialization definition) + (syntax-parameterize ((configuration-field-serialization? + (identifier-syntax #f))) +definition +#t)) + +(without-field-serialization + (define-configuration foo +(bar (integer 123) "doc"))) + (define (serialize-package field-name val) "")
Add a way to disable serialization support to (guix services configuration)
Hello Guix! I've rediscovered the little gem that is (guix services configurations), and attempted to make it more generally useful by adding an option to opt out of serialization (which is not well adapted for producing a list of command line arguments from the configuration for example): --8<---cut here---start->8--- 1 file changed, 10 insertions(+), 1 deletion(-) gnu/services/configuration.scm | 11 ++- modified gnu/services/configuration.scm @@ -38,6 +38,9 @@ configuration-field-getter configuration-field-default-value-thunk configuration-field-documentation + +%with-serialization? + serialize-configuration define-maybe define-configuration @@ -51,6 +54,11 @@ ;;; ;;; Code: +;;; XXX: This doesn't actually work as a parameter with macros such as +;;; define-configuration; it is to be used as a plain global variable. +;;; Experiments with define-syntax-parameter did not work either. +(define %with-serialization? (make-parameter #true)) + (define-condition-type configuration-error?) @@ -123,7 +131,8 @@ #'(field-type ...))) ((field-serializer ...) (map (lambda (type) - (id #'stem #'serialize- type)) + (and (%with-serialization?) + (id #'stem #'serialize- type))) #'(field-type ... #`(begin (define-record-type* #,(id #'stem #'< #'stem #'>) --8<---cut here---end--->8--- Unfortunately, it doesn't work, at least when using it from 'guix system'. I've also tried a version relying on syntax-parameter instead of a parameter, with the same result. Would someone know how it could be made to work? Thanks, Maxim
