From debbugs-submit-bounces@debbugs.gnu.org Fri Mar 24 10:56:32 2023 Received: (at 62298) by debbugs.gnu.org; 24 Mar 2023 14:56:32 +0000 Received: from localhost ([127.0.0.1]:41137 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pfiqF-00045z-H5 for submit@debbugs.gnu.org; Fri, 24 Mar 2023 10:56:32 -0400 Received: from mail-vs1-f45.google.com ([209.85.217.45]:46035) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pfiqD-00045k-8m for 62298@debbugs.gnu.org; Fri, 24 Mar 2023 10:56:29 -0400 Received: by mail-vs1-f45.google.com with SMTP id c10so1700914vsh.12 for <62298@debbugs.gnu.org>; Fri, 24 Mar 2023 07:56:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679669778; h=content-transfer-encoding:mime-version:user-agent:message-id :in-reply-to:date:references:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=lWB1O5uQg+xVlJd8vCmCM8XQRn1Y5oVC4URjecw+CGE=; b=FJsvIp7ol1R2WNo8j9/O7kxwJHHgo4WC00bWmuP4pcw/RL/VfB6kEM6eCmFRuyVyla 2g1Et6WrEXMYsPvxLfVcJ9LhHPEjhzNExoZ+lxNhW2S6Oq7VRxdsrkYypl8zONp/9nLo ytR6ms1da+h7fha8srRgeFZtmmqNO0oalPtPYiEflBmMG3aWPwfhB73QlNFNwYIqQPSS gYhI00df2rEaVA2P/UvpYq27ptauQ7fdkafLepc8TFvr6nWtJlRDu7jqqtKY9OIphBAJ jMx7Qwtm5dG5w/iKbUDHa/xmJc/iqlXe3T74N6Tp8FIwttdCHkmEzN0nfYn9LD9wVXrs vNBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679669778; h=content-transfer-encoding:mime-version:user-agent:message-id :in-reply-to:date:references:subject:cc:to:from:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=lWB1O5uQg+xVlJd8vCmCM8XQRn1Y5oVC4URjecw+CGE=; b=Bj13+BiFV5kKYhn+wS6hVYlndR3evN0vfA5BpANWhTduFy17+mW376t71R9WfvzVOb pimlAMmt8bjbOSgHRVZh8+ZxjihJHQVjflPHROgokto5KPKogq66/5FK78Lld6tF343b WhteMWldOUUecthGkZrz6NYR8f3zZtd2Sl2AfR6Wu9I8GgNuGco04TixQUDo0mkXH2gc TIMop9/xj+D++Y6rYZR2Y1eGHHLNB1HPzqyWgGLK5Png4mSWBDlaE/CMndcHpjKak81P /itby2JhoUheefYZU05BJ9dybnmPXgtMiXgfUdNCwxaMd5tcafvNy4CWf5h6ebuFqxjw uoFg== X-Gm-Message-State: AAQBX9dZOSefzgxK16GaZAsUSSVAg9yCxHzXOahKRandFROrLX1mwGuP e7XZdcQtbVC2Xr0evafshDHl3GYBMzVppA== X-Google-Smtp-Source: AKy350ZFcRSNdCWxJcwGOYBKIFiUSxkTfsHrl9zZbX6Bjw2DUbpTCKEYuY2OUjOa3L14lFUAIL9lAA== X-Received: by 2002:a05:6214:500e:b0:5ba:168d:d3fc with SMTP id jo14-20020a056214500e00b005ba168dd3fcmr5475844qvb.4.1679667939660; Fri, 24 Mar 2023 07:25:39 -0700 (PDT) Received: from hurd (dsl-155-54.b2b2c.ca. [66.158.155.54]) by smtp.gmail.com with ESMTPSA id m64-20020a375843000000b0073b8512d2dbsm14158059qkb.72.2023.03.24.07.25.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Mar 2023 07:25:39 -0700 (PDT) From: Maxim Cournoyer To: Bruno Victal Subject: Re: [PATCH v2 1/8] services: configuration: Add user-defined sanitizer support. References: Date: Fri, 24 Mar 2023 10:25:37 -0400 In-Reply-To: (Bruno Victal's message of "Thu, 23 Mar 2023 15:02:11 +0000") Message-ID: <87r0tez0ny.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 62298 Cc: 62298@debbugs.gnu.org, ludo@gnu.org, liliana.prikler@gmail.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Hello! Bruno Victal writes: > This changes the 'custom-serializer' field into a generic > 'extra-args' field that can be extended to support new literals. > With this mechanism, the literals 'sanitizer' allow for user-defined > sanitizer procedures while the 'serializer' literal is used for > custom serializer procedures. > The 'empty-serializer' was also added as a 'literal' and can be used > just like it was previously. > > With the repurposing of 'custom-serializer' into 'extra-args', to prevent > intolerable confusion, the custom serializer procedures should be > specified using the new 'literals' approach, with the previous =E2=80=9Cs= tyle=E2=80=9D > being considered deprecated. > > * gnu/services/configuration.scm (define-configuration-helper): > Rename 'custom-serializer' to 'extra-args'. > Add support for literals 'sanitizer', 'serializer' and 'empty-serializer'. > Rename procedure 'field-sanitizer' to 'default-field-sanitizer' to avoid = syntax clash. > Only define default field sanitizers if user-defined ones are absent. > (normalize-extra-args): New procedure. > ()[sanitizer]: New field. > > * doc/guix.texi (Complex Configurations): Document the newly added litera= ls. > > * tests/services/configuration.scm: Add tests for the new literals. Interesting work! > --- > doc/guix.texi | 30 ++++- > gnu/services/configuration.scm | 91 +++++++++++---- > tests/services/configuration.scm | 185 ++++++++++++++++++++++++++++++- > 3 files changed, 280 insertions(+), 26 deletions(-) > > diff --git a/doc/guix.texi b/doc/guix.texi > index dfdb26103a..1609e508ef 100644 > --- a/doc/guix.texi > +++ b/doc/guix.texi > @@ -41216,7 +41216,7 @@ Complex Configurations > (@var{field-name} > (@var{type} @var{default-value}) > @var{documentation} > - @var{serializer}) > + (serializer @var{serializer})) >=20=20 > (@var{field-name} > (@var{type}) > @@ -41225,7 +41225,18 @@ Complex Configurations > (@var{field-name} > (@var{type}) > @var{documentation} > - @var{serializer}) > + (serializer @var{serializer})) > + > +(@var{field-name} > + (@var{type}) > + @var{documentation} > + (sanitizer @var{sanitizer}) > + > +(@var{field-name} > + (@var{type}) > + @var{documentation} > + (sanitizer @var{sanitizer}) > + (serializer @var{serializer})) > @end example >=20=20 > @var{field-name} is an identifier that denotes the name of the field in > @@ -41248,6 +41259,21 @@ Complex Configurations > @var{documentation} is a string formatted with Texinfo syntax which > should provide a description of what setting this field does. >=20=20 > +@var{sanitizer} is the name of a procedure which takes one argument, > +a user-supplied value, and returns a ``sanitized'' value for the field. > +If none is specified, the predicate @code{@var{type}?} is automatically > +used to construct an internal sanitizer that asserts the type correctness > +of the field value. > + > +An example of a sanitizer for a field that accepts both strings and > +symbols looks like this: > +@lisp > +(define (sanitize-foo value) > + (cond ((string? value) value) > + ((symbol? value) (symbol->string value)) > + (else (throw 'bad! value)))) > +@end lisp I'd use the more conventional (error "bad value" value) in the example. > @var{serializer} is the name of a procedure which takes two arguments, > the first is the name of the field, and the second is the value > corresponding to the field. The procedure should return a string or > diff --git a/gnu/services/configuration.scm b/gnu/services/configuration.= scm > index 174c2f20d2..409d4cef00 100644 > --- a/gnu/services/configuration.scm > +++ b/gnu/services/configuration.scm > @@ -6,6 +6,7 @@ > ;;; Copyright =C2=A9 2021, 2022 Maxim Cournoyer > ;;; Copyright =C2=A9 2021 Andrew Tropin > ;;; Copyright =C2=A9 2022 Maxime Devos > +;;; Copyright =C2=A9 2023 Bruno Victal > ;;; > ;;; This file is part of GNU Guix. > ;;; > @@ -28,7 +29,8 @@ (define-module (gnu services configuration) > #:use-module (guix gexp) > #:use-module ((guix utils) #:select (source-properties->location)) > #:use-module ((guix diagnostics) > - #:select (formatted-message location-file &error-locatio= n)) > + #:select (formatted-message location-file &error-location > + warning)) > #:use-module ((guix modules) #:select (file-name->module-name)) > #:use-module (guix i18n) > #:autoload (texinfo) (texi-fragment->stexi) > @@ -37,6 +39,7 @@ (define-module (gnu services configuration) > #:use-module (ice-9 format) > #:use-module (ice-9 match) > #:use-module (srfi srfi-1) > + #:use-module (srfi srfi-26) > #:use-module (srfi srfi-34) > #:use-module (srfi srfi-35) > #:export (configuration-field > @@ -44,6 +47,7 @@ (define-module (gnu services configuration) > configuration-field-type > configuration-missing-field > configuration-field-error > + configuration-field-sanitizer > configuration-field-serializer > configuration-field-getter > configuration-field-default-value-thunk > @@ -116,6 +120,7 @@ (define-record-type* > (type configuration-field-type) > (getter configuration-field-getter) > (predicate configuration-field-predicate) > + (sanitizer configuration-field-sanitizer) > (serializer configuration-field-serializer) > (default-value-thunk configuration-field-default-value-thunk) > (documentation configuration-field-documentation)) > @@ -181,11 +186,45 @@ (define (normalize-field-type+def s) > (values #'(field-type %unset-value))))) >=20=20 > (define (define-configuration-helper serialize? serializer-prefix syn) > + > + (define (normalize-extra-args s) A docstring would help here, explaining what this helper is for. > + (let loop ((s s) > + (sanitizer* %unset-value) > + (serializer* %unset-value)) > + (syntax-case s (sanitizer serializer empty-serializer) > + (((sanitizer proc) tail ...) > + (if (maybe-value-set? sanitizer*) > + (syntax-violation 'sanitizer "duplicate entry" > + #'proc) > + (loop #'(tail ...) #'proc serializer*))) > + (((serializer proc) tail ...) > + (if (maybe-value-set? serializer*) > + (syntax-violation 'serializer "duplicate or conflicting ent= ry" > + #'proc) > + (loop #'(tail ...) sanitizer* #'proc))) > + ((empty-serializer tail ...) > + (if (maybe-value-set? serializer*) > + (syntax-violation 'empty-serializer > + "duplicate or conflicting entry" #f) > + (loop #'(tail ...) sanitizer* #'empty-serializer))) > + (() ; stop condition > + (values (list sanitizer* serializer*))) > + ((proc) ; TODO: deprecated, to be removed > + (every (cut eq? <> #f) > + (map maybe-value-set? > + (list sanitizer* serializer*))) This 'every' call result is not acted upon. Was it supposed to raise a syntax violation? If it's useful to keep it (and act on it), I'd use something like: --8<---------------cut here---------------start------------->8--- (unless (null? (filter-map maybe-value-set? (list sanitizer* serializer*))) (syntax-violation ...)) --8<---------------cut here---------------end--------------->8--- > + (begin I think the 'begin' is unnecessary. > + (warning #f (G_ "specifying serializers after documentation i= s \ > +deprecated, use (serializer ~a) instead~%") (syntax->datum #'proc)) I wonder, instead of #f, would it be possible to fill in the correct source location? That'd be useful, and it's done elsewhere, though I'm not too familiar with the mechanism (I think it relies on Guile's source properties). [...] > diff --git a/tests/services/configuration.scm b/tests/services/configurat= ion.scm > index 4f8a74dc8a..64b7bb1543 100644 > --- a/tests/services/configuration.scm > +++ b/tests/services/configuration.scm [...] > +(test-group "empty-serializer as literal/procedure tests" > + ;; empty-serializer as literal Nitpick; all stand-alone comments starting with ;; should be properly punctuated, complete sentences. The rest LGTM, but I'll leave it on the tracker for another week or so to allow for others to comment since it's a close-to-core change. --=20 Thanks, Maxim