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 CAIvK4DTlGCpmwAAgWs5BA (envelope-from ) for ; Fri, 07 May 2021 07:43:28 +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 eB/JJoDTlGDjZQAAB5/wlQ (envelope-from ) for ; Fri, 07 May 2021 05:43:28 +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 488D3DD9F for ; Fri, 7 May 2021 07:43:28 +0200 (CEST) Received: from localhost ([::1]:58986 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1letGo-0004BV-VZ for larch@yhetil.org; Fri, 07 May 2021 01:43:27 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37662) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1letGR-0004BO-1N for guix-devel@gnu.org; Fri, 07 May 2021 01:43:03 -0400 Received: from mail-qk1-x731.google.com ([2607:f8b0:4864:20::731]:39737) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1letGO-0004P9-OR; Fri, 07 May 2021 01:43:02 -0400 Received: by mail-qk1-x731.google.com with SMTP id k127so7399146qkc.6; Thu, 06 May 2021 22:42:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=vFurenGJ96INL/ubgaImzd0e+PszHkEH/KC1k7Q2IFU=; b=ZgLiq2GF8/yaui1+jRnby4oaB1i/JaLc71NN7dit45KKLW21rM9719BwkZB1JH8myG eg2pcX1YvlHKvPcToLQlGOn8U+wsftzZd6sM00T855p6mWLLjOrGk7RIEUZFGGO+1eVL 1sEA0HgJdauvw1SUvi7pnpWDp9foqDqhTdJnZBlVCyB/F0EzalvkY/siHz3NPEJywDa7 9x1cjwfIV13THd4fGUgTON7ykADpBGAlT9V2mAdSKbuQgbFcK9hr0IdHd4RenKTu8Fp1 fpIFvEK5FJwp7WHheQ1NLLTaDpvx5yEE8gcw+pbk+KMFtBTV0lS0eLo52tXYMxI+Eqdy Hksw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=vFurenGJ96INL/ubgaImzd0e+PszHkEH/KC1k7Q2IFU=; b=D+ylMLkVFKvFMqoAQOdEoaSq8y1Hv/9qLOEITkXyGiBFTCYuAMkLMQ15/ZsHWciORg Ke0FHsaenkT6y1rQlX2nNO4eR304gPAd6BGEojYp9bl0EJ4xy4RbSAsG3p9PU22hO5pL QWLovI0rjxyDQiG6k7tJcQg/rKpyx3s1pP+I7Z234qWY5z7evQ3Zl/ned995dypa6rQ3 P1dgl/odvdExqip2MpBC79Ztq3I584zTp1B6ENO7AyTcFWF2O7dFptk5JnWvU0PTH8Cs g2Cy+cMOfgJmu8FYfV/4+PLXHurKfeHbO8juBUcdxOUfpyOKHZTtOMmgnsRrML8iDpQX hibA== X-Gm-Message-State: AOAM532i4bWtuwTM0AIvjqUvixueS/ccu+FD0pnMVNRbYnVGuwTxc29B Bdt7xcasbVS0N0j/b2QQb5Oqu0zx4jik7w== X-Google-Smtp-Source: ABdhPJx5V4YfrWpKLm5EEOYXz/gDliUKrk286dtg+lgQ67cc51ApWjmZGdlP8QYaxIXU82UjB0yPyQ== X-Received: by 2002:a37:a8c6:: with SMTP id r189mr7776343qke.446.1620366178024; Thu, 06 May 2021 22:42:58 -0700 (PDT) Received: from hurd (dsl-10-131-245.b2b2c.ca. [72.10.131.245]) by smtp.gmail.com with ESMTPSA id 11sm4201003qkk.31.2021.05.06.22.42.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 May 2021 22:42:57 -0700 (PDT) From: Maxim Cournoyer To: Xinglu Chen Subject: Re: Add a way to disable serialization support to (guix services configuration) 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> Date: Fri, 07 May 2021 01:42:56 -0400 In-Reply-To: <87k0oik6sy.fsf@yoctocell.xyz> (Xinglu Chen's message of "Sat, 01 May 2021 13:54:53 +0200") Message-ID: <87wnsbgkv3.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Received-SPF: pass client-ip=2607:f8b0:4864:20::731; envelope-from=maxim.cournoyer@gmail.com; helo=mail-qk1-x731.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 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, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham 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=1620366208; 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:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=vFurenGJ96INL/ubgaImzd0e+PszHkEH/KC1k7Q2IFU=; b=j2z76hL5tRhmhTYIFMYmAFDawYEyTxnm/lsvozzUKPiF9P+t2Vi2jG/l8lqUu6gy1R1daa bQDZs66psefD53OBOJ9HvsDV8lNBNByOMwoCwkKf4DZ4Csa8dh2Rrp3cJ91bUcsfh95GFX YEEvoL8iOxeNxWGl4pLOt+SHbpC+Q9bATWZtqMAUZD0p+vSaI62x+VMaZN4SFaPGe25N4t qNoCMMyx70u578yoOQcEzdkFKT4C7Yx4QejZoEJ8z76YmtNnTsqFDLLEGIbBMkYMlzdHMJ BpF7guHoN2H1bL9e8I+5WpmsuD1Eh6JBlchw3zRpj/zeroDrzscoBB6YjZE/Xg== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1620366208; a=rsa-sha256; cv=none; b=VgIqUjfZ9Bz3uvFKEWSYZLhSecxHtHW69HBZntb5DCOiGG4REZKwrL05BqbizHN2jroA9g rhP3sLLtzUtDnN0yeTAytTJreDwhjtLzqPIyGGLW40vU+I1WuC25nF2vCc/RHzyDVnWbVY SOZg5suqvPFGLlipPBgpenFv2oePm5MF93hozEPd3pyFxOPBoRi3S4aB3l0fLE4c5BNaRO C9imiGN7iD3Y+pXDzF42fOCQpuKjf+3J2Io/iAks4ZAz8eF0UA2fJXOQOcdmCn9LD2qz2g jvwYA40FoUR0EbYnyxvdB3gDWiOrFrgDuZyryekhhB7w0SfnTtEaXSDMESSLdQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20161025 header.b=ZgLiq2GF; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none); 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: 0.14 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20161025 header.b=ZgLiq2GF; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none); 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: 488D3DD9F X-Spam-Score: 0.14 X-Migadu-Scanner: scn0.migadu.com X-TUID: 7vvVlXpvyWFS 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 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. > + (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; modified gnu/services/configuration.scm @@ -66,7 +66,8 @@ (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))) + (format #f "The field `~a' of the `~a' configuration record \ +does not have a default value" field kind))) (define-record-type* configuration-field make-configuration-field configuration-field? @@ -116,7 +117,7 @@ (define-syntax define-configuration (lambda (stx) (syntax-case stx () - ((_ stem (field (field-type properties ...) doc) ...) + ((_ stem (field (field-type def ...) doc) ...) (with-syntax (((field-getter ...) (map (lambda (field) (id #'stem #'stem #'- field)) @@ -127,11 +128,13 @@ #'(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-type default-value) + default-value) + ((field-type) + ;; Quote `undefined' to prevent a possibly + ;; unbound warning. + (syntax 'undefined))) + #'((field-type def ...) ...))) ((field-serializer ...) (map (lambda (type) (id #'stem #'serialize- type)) @@ -146,7 +149,7 @@ source-properties->location)) (innate)) #,@(map (lambda (name getter def) - (if (equal? (syntax->datum def) (quote 'disabled)) + (if (eq? (syntax->datum def) (quote 'undefined)) #`(#,name #,getter) #`(#,name #,getter (default #,def)))) #'(field ...) @@ -159,12 +162,11 @@ (getter field-getter) (predicate field-predicate) (serializer field-serializer) - ;; TODO: What if there is no default value? (default-value-thunk (lambda () (display '#,(id #'stem #'% #'stem)) - (if (equal? (syntax->datum field-default) - (quote 'disabled)) + (if (eq? (syntax->datum field-default) + 'undefined) (configuration-no-default-value '#,(id #'stem #'% #'stem) 'field) field-default))) I'll attempt to review patch 2/2 shortly! Thanks a lot for this neat improvement! Maxim