From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id 6GZjDMNIlWA1fwEAgWs5BA (envelope-from ) for ; Fri, 07 May 2021 16:03:47 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:bcc0::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id AKoHCMNIlWBhSgAAB5/wlQ (envelope-from ) for ; Fri, 07 May 2021 14:03:47 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id A3B6315E94 for ; Fri, 7 May 2021 16:03:46 +0200 (CEST) Received: from localhost ([::1]:35182 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lf14z-0002HM-5R for larch@yhetil.org; Fri, 07 May 2021 10:03:45 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:56600) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lf14o-0002HF-6O for guix-devel@gnu.org; Fri, 07 May 2021 10:03:34 -0400 Received: from h87-96-130-155.cust.a3fiber.se ([87.96.130.155]:46650 helo=mail.yoctocell.xyz) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lf14l-0007z0-Hc for guix-devel@gnu.org; Fri, 07 May 2021 10:03:33 -0400 From: Xinglu Chen DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=yoctocell.xyz; s=mail; t=1620396203; bh=fSWGwJyN0RHWsyHqhK7hHAJCNIicUSYZHg320mH6HdI=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=uyqGWHZ1bZcyVuEeAEP/UzrQL1hNK+qNDJOV8XskfQIsL5X2Ehgyryac+sXuSdj2R 0cXLt1dW1v1Q4bCVHLBOzl1BFd94cTOh+m7Hy6idRfSxazNKeGpWDJuQMkL38gSYUH 9grGGVgsshH85uPZp52bS/y6WKKHWb2A3urDIIcQ= To: Maxim Cournoyer Subject: Re: Add a way to disable serialization support to (guix services configuration) In-Reply-To: <87wnsbgkv3.fsf@gmail.com> References: <878s5ncjaw.fsf@gmail.com> <87fszoswmc.fsf@gnu.org> <87k0ovd4ok.fsf@gmail.com> <87mttqt0ms.fsf@gnu.org> <871rb15y6k.fsf@yoctocell.xyz> <87k0oik6sy.fsf@yoctocell.xyz> <87wnsbgkv3.fsf@gmail.com> Date: Fri, 07 May 2021 16:03:22 +0200 Message-ID: <87pmy24p5h.fsf@yoctocell.xyz> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Received-SPF: pass client-ip=87.96.130.155; envelope-from=public@yoctocell.xyz; helo=mail.yoctocell.xyz X-Spam_score_int: 14 X-Spam_score: 1.4 X-Spam_bar: + X-Spam_report: (1.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FROM_SUSPICIOUS_NTLD=0.499, PDS_OTHER_BAD_TLD=1.999, RDNS_DYNAMIC=0.982, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: guix-devel Errors-To: guix-devel-bounces+larch=yhetil.org@gnu.org Sender: "Guix-devel" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1620396226; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=Lpi4Z26jBLRyt/iyWmT1SnEzoC66eUV/lsVvZX0hvRY=; b=bA5okEpe1ZLcM/ANpVmQQH2bzdLTNEcmC01jSfPOYyPgUEn93q4tMqrdu6wcD1PnDFuAP7 hYtLQ8/3prolI3PrC9ItBTBVXECSZWASKefBIdlfX1Qo521cIB/HH6r9ty4uehR1CkYStq wyPAGN3mHrKTzGK5VdwzVmJvDVBa7tcR5gy8Mu5iwdjYtg9lf5zt9z5zM4VqDKcZRK8vtK yU9cKo6BA7YY2hjHQTXPUxHKF+fEhLn8F/E5A5LGAR4HFpzSU6L5JbFaihndZZO4PCXLae yc5MhLdURvL/Tpr/M5aVda994dcqSbI39CkaAxsOySnr27qbBZfn32mAr6UAgQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1620396226; a=rsa-sha256; cv=none; b=imVUnsg6vrll9d6sfT5BqhoWhby4+MEJunQe8IFDeDCoothzPZ0gvCoSyPOZFIydIyzJW+ GppVTAtJCOlqfrH59Ge/Tnl26hBnDAVxShUdOHZKnHgYO3Lle/T2qOE49FTnFHWq1HMvLh QL/mTI/gK/KdXRqf+JtY8+7jGVMvLW/JlES1aBO2tiEPOSS3daJ1r6e/ochAm0qtiq8POa FXnqh9gJxUzSo+0ec4aLwGDx5JS0HEJERHJb8ZATrILMT7EKnLwsgN5Byth1PGPAOJtwmb Ev5rbPh+eitRl8lUGOoz4SpNNyGgaK6ge7mzFG7R3B/Mx+wiJuydLLJc/ikfXA== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=yoctocell.xyz header.s=mail header.b=uyqGWHZ1; dmarc=pass (policy=none) header.from=yoctocell.xyz; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Spam-Score: -1.65 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=yoctocell.xyz header.s=mail header.b=uyqGWHZ1; dmarc=pass (policy=none) header.from=yoctocell.xyz; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Queue-Id: A3B6315E94 X-Spam-Score: -1.65 X-Migadu-Scanner: scn0.migadu.com X-TUID: pa5v/0aFvH9I 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 =E2=80=98properties=E2=80=99, an= yway... >> (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=E2=80=99s 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=3D> (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 #'-field= s)) >> - conf)))))))) >> + #`(begin >> + (define-record-type* #,(id #'stem #'< #'stem #'>) >> + #,(id #'stem #'% #'stem) >> + #,(id #'stem #'make- #'stem) >> + #,(id #'stem #'stem #'?) >> + (%location #,(id #'stem #'-location) >> + (default (and=3D> (current-source-location) >> + source-properties->location)) >> + (innate)) >> + #,@(map (lambda (name getter def) >> + (if (equal? (syntax->datum def) (quote 'disabl= ed)) > > 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 taken care of already :-). > >> + (default-value-thunk >> + (lambda () >> + (display '#,(id #'stem #'% #'stem)) >> + (if (equal? (syntax->datum field-default) >> + (quote 'disabled)) > > Like above (eq? would do). More importantly (and confusingly), here the > 'disabled expected value must *not* be quoted. I haven't investigated > why but it seems one level of quote got striped at that point. Hmm, I am still a little confused about how quotation works with syntax-case & friends. >> + (configuration-no-default-value >> + '#,(id #'stem #'% #'stem) 'field) >> + field-default))) >> + (documentation doc)) >> + ...)) >> + (define-syntax-rule (stem arg (... ...)) >> + (let ((conf (#,(id #'stem #'% #'stem) arg (... ...)))) >> + (validate-configuration conf >> + #,(id #'stem #'stem #'-fields)) >> + conf)))))))) > > The following patch implements the above comments; > > [...] > > I'll attempt to review patch 2/2 shortly! > > Thanks a lot for this neat improvement! Thank you for the review and the improvements!=20=20