* [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values.
@ 2020-08-27 5:12 Maxim Cournoyer
2020-08-27 5:21 ` [bug#43064] [PATCH] Revert "system: image: Do not offload image files." Maxim Cournoyer
2020-08-30 19:41 ` [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values Ludovic Courtès
0 siblings, 2 replies; 6+ messages in thread
From: Maxim Cournoyer @ 2020-08-27 5:12 UTC (permalink / raw)
To: 43064; +Cc: Maxim Cournoyer
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=yes, Size: 4708 bytes --]
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@lassieur.org>
;;; Copyright © 2018 Jan Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2019, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@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@gnu.org>
+;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [bug#43064] [PATCH] Revert "system: image: Do not offload image files."
2020-08-27 5:12 [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values Maxim Cournoyer
@ 2020-08-27 5:21 ` Maxim Cournoyer
2020-08-30 19:41 ` [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values Ludovic Courtès
1 sibling, 0 replies; 6+ messages in thread
From: Maxim Cournoyer @ 2020-08-27 5:21 UTC (permalink / raw)
To: 43064; +Cc: Maxim Cournoyer
This reverts commit 6a9581741e4ee81226aeb2f1c997df76670a6aab, which is
obsoleted by the previous commit.
---
gnu/system/image.scm | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index c1a718d607..36f56e237d 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -266,8 +266,7 @@ used in the image."
#$output
image-root)))))
(computed-file "partition.img" image-builder
- #:options `(#:local-build? #t ;typically large file
- #:references-graphs ,inputs))))
+ #:options `(#:references-graphs ,inputs))))
(define (partition->config partition)
;; Return the genimage partition configuration for PARTITION.
@@ -325,8 +324,7 @@ image ~a {
#~(symlink
(string-append #$image-dir "/" #$genimage-name)
#$output)
- #:options `(#:local-build? #t ;typically large file
- #:substitutable? ,substitutable?))))
+ #:options `(#:substitutable? ,substitutable?))))
\f
;;
@@ -403,8 +401,7 @@ used in the image. "
#:volume-id #$root-label
#:volume-uuid #$root-uuid)))))
(computed-file name builder
- #:options `(#:local-build? #t ;typically large file
- #:references-graphs ,inputs
+ #:options `(#:references-graphs ,inputs
#:substitutable? ,substitutable?))))
\f
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values.
2020-08-27 5:12 [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values Maxim Cournoyer
2020-08-27 5:21 ` [bug#43064] [PATCH] Revert "system: image: Do not offload image files." Maxim Cournoyer
@ 2020-08-30 19:41 ` Ludovic Courtès
2020-08-31 4:37 ` [bug#43064] [PATCH v2] " Maxim Cournoyer
1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2020-08-30 19:41 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 43064
Hi,
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> 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.
How about exposing some of the relevant options as keywords? We can
keep #:options as an “escape hatch” and add things like like
#:local-build? etc. That would be simpler and more idiomatic.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug#43064] [PATCH v2] gexp: computed-file: Prevent mistakenly overriding default option values.
2020-08-30 19:41 ` [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values Ludovic Courtès
@ 2020-08-31 4:37 ` Maxim Cournoyer
2020-08-31 13:34 ` Ludovic Courtès
0 siblings, 1 reply; 6+ messages in thread
From: Maxim Cournoyer @ 2020-08-31 4:37 UTC (permalink / raw)
To: 43064; +Cc: Ludovic Courtès, Maxim Cournoyer
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@gnu.org>
---
guix/gexp.scm | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/guix/gexp.scm b/guix/gexp.scm
index 67b6121313..9d3c52e783 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -3,6 +3,7 @@
;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;; Copyright © 2018 Jan Nieuwenhuizen <janneke@gnu.org>
;;; Copyright © 2019, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -504,13 +505,15 @@ This is the declarative counterpart of 'text-file'."
(options computed-file-options)) ;list of arguments
(define* (computed-file name gexp
- #:key guile (options '(#:local-build? #t)))
+ #:key guile (local-build? #t) (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'.
+computed by GEXP. When LOCAL-BUILD? is #t (the default), it ensures the
+corresponding derivation is built locally. OPTIONS may be used to pass
+additional arguments to 'gexp->derivation'.
This is the declarative counterpart of 'gexp->derivation'."
- (%computed-file name gexp guile options))
+ (let ((options* `(#:local-build? ,local-build? ,@options)))
+ (%computed-file name gexp guile options*)))
(define-gexp-compiler (computed-file-compiler (file <computed-file>)
system target)
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [bug#43064] [PATCH v2] gexp: computed-file: Prevent mistakenly overriding default option values.
2020-08-31 4:37 ` [bug#43064] [PATCH v2] " Maxim Cournoyer
@ 2020-08-31 13:34 ` Ludovic Courtès
2020-09-01 13:00 ` bug#43064: " Maxim Cournoyer
0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2020-08-31 13:34 UTC (permalink / raw)
To: Maxim Cournoyer; +Cc: 43064
Hi,
Maxim Cournoyer <maxim.cournoyer@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@gnu.org>
[...]
> (define* (computed-file name gexp
> - #:key guile (options '(#:local-build? #t)))
> + #:key guile (local-build? #t) (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'.
> +computed by GEXP. When LOCAL-BUILD? is #t (the default), it ensures the
> +corresponding derivation is built locally. OPTIONS may be used to pass
> +additional arguments to 'gexp->derivation'.
>
> This is the declarative counterpart of 'gexp->derivation'."
Please update doc/guix.texi as well.
Otherwise LGTM, thanks!
Ludo’.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#43064: [PATCH v2] gexp: computed-file: Prevent mistakenly overriding default option values.
2020-08-31 13:34 ` Ludovic Courtès
@ 2020-09-01 13:00 ` Maxim Cournoyer
0 siblings, 0 replies; 6+ messages in thread
From: Maxim Cournoyer @ 2020-09-01 13:00 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 43064-done
Ludovic Courtès <ludo@gnu.org> writes:
> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@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@gnu.org>
[...]
> Please update doc/guix.texi as well.
>
> Otherwise LGTM, thanks!
>
> Ludo’.
Done!
Thank you,
Maxim
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-09-01 13:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-27 5:12 [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values Maxim Cournoyer
2020-08-27 5:21 ` [bug#43064] [PATCH] Revert "system: image: Do not offload image files." Maxim Cournoyer
2020-08-30 19:41 ` [bug#43064] [PATCH] gexp: computed-file: Prevent mistakenly overriding default option values Ludovic Courtès
2020-08-31 4:37 ` [bug#43064] [PATCH v2] " Maxim Cournoyer
2020-08-31 13:34 ` Ludovic Courtès
2020-09-01 13:00 ` bug#43064: " Maxim Cournoyer
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).