GNU bug report logs - #43064
[PATCH] gexp: computed-file: Prevent mistakenly overriding default option values.

Previous Next

Package: guix-patches;

Reported by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

Date: Thu, 27 Aug 2020 05:15:01 UTC

Severity: normal

Tags: patch

Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#43064: closed ([PATCH] gexp: computed-file: Prevent
 mistakenly overriding default option values.)
Date: Tue, 01 Sep 2020 13:01:01 +0000
[Message part 1 (text/plain, inline)]
Your message dated Tue, 01 Sep 2020 09:00:12 -0400
with message-id <87imcxzm77.fsf <at> gmail.com>
and subject line Re: [PATCH v2] gexp: computed-file: Prevent mistakenly overriding default option values.
has caused the debbugs.gnu.org bug report #43064,
regarding [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values.
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)


-- 
43064: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=43064
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH] gexp: computed-file: Prevent mistakenly overriding default
 option values.
Date: Thu, 27 Aug 2020 01:12:26 -0400
Because the options are passed as a list rather than actual keyword arguments,
omitting to repeat the default values in that list easily causes the default
values to be lost.

* guix/gexp.scm (%computed-file-default-options): New variable.
(alist->plist): New procedure.
(computed-file-combine-options-with-defaults): New procedure.
(computed-file): Use the above procedures to form the default OPTIONS value.
Update doc.  Use the COMPUTED-FILE-COMBINE-OPTIONS-WITH-DEFAULTS procedure to
combine the user options with the default options, when they aren't
overridden.
* tests/gexp.scm ("computed-file options defaults honored")
("computed-file options defaults overridden"): Add tests.
---
 guix/gexp.scm  | 42 +++++++++++++++++++++++++++++++++++++++---
 tests/gexp.scm | 12 ++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/guix/gexp.scm b/guix/gexp.scm
index 67b6121313..14e07e8fe6 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2018 Clément Lassieur <clement <at> lassieur.org>
 ;;; Copyright © 2018 Jan Nieuwenhuizen <janneke <at> gnu.org>
 ;;; Copyright © 2019, 2020 Mathieu Othacehe <m.othacehe <at> gmail.com>
+;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -503,14 +504,49 @@ This is the declarative counterpart of 'text-file'."
   (guile      computed-file-guile)                ;<package>
   (options    computed-file-options))             ;list of arguments
 
+;;; Alist containing the default options for computed-file.
+(define %computed-file-default-options '((#:local-build? . #t)))
+
+(define (alist->plist alist)
+  "Transform an association list into a property list."
+  (fold (lambda (current acc)
+          (match current
+            ((x . y)
+             (append acc (list x y)))))
+        '()
+        alist))
+
+(define (computed-file-combine-options-with-defaults options)
+
+  (define alist->keys
+    (match-lambda
+      (((key . value) ...)
+       key)))
+
+  (define (plist->keys plist)
+    (filter keyword? plist))
+
+  (define (default-keys->plist keys)
+    (append-map (lambda (key)
+                  (list key (assq-ref %computed-file-default-options key)))
+                keys))
+
+  (let ((default-keys (lset-difference
+                       eq?
+                       (alist->keys %computed-file-default-options)
+                       (plist->keys options))))
+    (append options (default-keys->plist default-keys))))
+
 (define* (computed-file name gexp
-                        #:key guile (options '(#:local-build? #t)))
+                        #:key guile (options (alist->plist
+                                              %computed-file-default-options)))
   "Return an object representing the store item NAME, a file or directory
 computed by GEXP.  OPTIONS is a list of additional arguments to pass
-to 'gexp->derivation'.
+to 'gexp->derivation', which defaults to %COMPUTED-FILE-DEFAULT-OPTIONS.
 
 This is the declarative counterpart of 'gexp->derivation'."
-  (%computed-file name gexp guile options))
+  (let ((options* (computed-file-combine-options-with-defaults options)))
+    (%computed-file name gexp guile options*)))
 
 (define-gexp-compiler (computed-file-compiler (file <computed-file>)
                                               system target)
diff --git a/tests/gexp.scm b/tests/gexp.scm
index 1beeb67c21..350065b58d 100644
--- a/tests/gexp.scm
+++ b/tests/gexp.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -62,6 +63,9 @@
                                      #:target target)
                   #:guile-for-build (%guile-for-build)))
 
+(define computed-file-combine-options-with-defaults
+  (@@ (guix gexp) computed-file-combine-options-with-defaults))
+
 (define %extension-package
   ;; Example of a package to use when testing 'with-extensions'.
   (dummy-package "extension"
@@ -1367,6 +1371,14 @@
     (return (and (derivation? drv1) (derivation? drv2)
                  (store-path? item)))))
 
+(test-equal "computed-file options defaults honored"
+  '(#:substitutable? #t #:local-build? #t)
+  (computed-file-combine-options-with-defaults '(#:substitutable? #t)))
+
+(test-equal "computed-file options defaults overridden"
+  '(#:local-build? #f)
+  (computed-file-combine-options-with-defaults '(#:local-build? #f)))
+
 (test-assertm "lower-object, computed-file"
   (let* ((text     (plain-file "foo" "Hello!"))
          (exp      #~(begin
-- 
2.27.0



[Message part 3 (message/rfc822, inline)]
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 43064-done <at> debbugs.gnu.org
Subject: Re: [PATCH v2] gexp: computed-file: Prevent mistakenly overriding
 default option values.
Date: Tue, 01 Sep 2020 09:00:12 -0400
Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> In order to do so, default to an empty options list, and expose options whose
>> default values are sensitive directly as keyword arguments.
>>
>> * guix/gexp.scm (computed-file): Extract the LOCAL-BUILD? parameter from the
>> OPTIONS parameter to make it a stand-alone keyword argument.  Introduce an
>> OPTIONS* binding which is obtained by combining the LOCAL-BUILD? keyword and
>> its value with OPTIONS.
>>
>> Suggested-by: Ludovic Courtès <ludo <at> gnu.org>

[...]

> Please update doc/guix.texi as well.
>
> Otherwise LGTM, thanks!
>
> Ludo’.

Done!

Thank you,

Maxim


This bug report was last modified 4 years and 261 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.