* [PATCH] guix: scripts: Fix GUIX_BUILD_OPTIONS handling.
[not found] <CAJ41eeyECrUPfEhS+9puU3y0iczEeCs8JgKB8RnTRofVNOW_4w@mail.gmail.com>
@ 2014-12-05 0:53 ` Deck Pickard
2014-12-06 15:04 ` Ludovic Courtès
0 siblings, 1 reply; 10+ messages in thread
From: Deck Pickard @ 2014-12-05 0:53 UTC (permalink / raw)
To: guix-devel
[-- Attachment #1.1: Type: text/plain, Size: 100 bytes --]
Perhaps, now it WILL twerk.
Drp,
--
(or ((,\ (x) `(,x x)) '(,\ (x) `(,x x))) (smth (that 'like)))
[-- Attachment #1.2: Type: text/html, Size: 149 bytes --]
[-- Attachment #2: 0001-guix-scripts-Fix-GUIX_BUILD_OPTIONS-handling.patch --]
[-- Type: application/octet-stream, Size: 4903 bytes --]
From 9c27d995e1a622de8457209d40031b392538e0f8 Mon Sep 17 00:00:00 2001
From: nebuli <nebu@kipple>
Date: Fri, 5 Dec 2014 01:28:12 +0100
Subject: [PATCH] guix: scripts: Fix GUIX_BUILD_OPTIONS handling.
Appending to "raw" args broke optional parameters in 'guix package -I'
and 'guix package -A', and possibly other places. Therefore, switch to
parsing each set of options on its own and append resulting alists
together afterwards.
* guix/scripts/archive.scm (parse-options-from): Rename from
(parse-options) and add explicit argument. New form of (parse-options)
using its old algorithm via -from function.
* guix/scripts/build.scm: Ditto.
* guix/scripts/environment.scm: Ditto.
* guix/scripts/package.scm: Ditto.
* guix/scripts/system.scm: Ditto.
---
guix/scripts/archive.scm | 8 ++++++--
guix/scripts/build.scm | 8 ++++++--
guix/scripts/environment.scm | 9 +++++++--
guix/scripts/package.scm | 8 ++++++--
guix/scripts/system.scm | 8 ++++++--
5 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/guix/scripts/archive.scm b/guix/scripts/archive.scm
index 29a3ad1..781ffc5 100644
--- a/guix/scripts/archive.scm
+++ b/guix/scripts/archive.scm
@@ -293,8 +293,12 @@ the input port."
(define (guix-archive . args)
(define (parse-options)
;; Return the alist of option values.
- (args-fold* (append args (environment-build-options))
- %options
+ (append (parse-options-from args)
+ (parse-options-from (environment-build-options))))
+
+ (define (parse-options-from args)
+ ;; Actual parsing takes place here.
+ (args-fold* args %options
(lambda (opt name arg result)
(leave (_ "~A: unrecognized option~%") name))
(lambda (arg result)
diff --git a/guix/scripts/build.scm b/guix/scripts/build.scm
index 76a743f..26e9f42 100644
--- a/guix/scripts/build.scm
+++ b/guix/scripts/build.scm
@@ -401,8 +401,12 @@ arguments with packages that use the specified source."
(define (guix-build . args)
(define (parse-options)
;; Return the alist of option values.
- (args-fold* (append args (environment-build-options))
- %options
+ (append (parse-options-from args)
+ (parse-options-from (environment-build-options))))
+
+ (define (parse-options-from args)
+ ;; Actual parsing takes place here.
+ (args-fold* args %options
(lambda (opt name arg result)
(leave (_ "~A: unrecognized option~%") name))
(lambda (arg result)
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index a309dfa..c388b0c 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -213,8 +213,13 @@ packages."
;; Entry point.
(define (guix-environment . args)
(define (parse-options)
- (args-fold* (append args (environment-build-options))
- %options
+ ;; Return the alist of option values.
+ (append (parse-options-from args)
+ (parse-options-from (environment-build-options))))
+
+ (define (parse-options-from args)
+ ;; Actual parsing takes place here.
+ (args-fold* args %options
(lambda (opt name arg result)
(leave (_ "~A: unrecognized option~%") name))
(lambda (arg result)
diff --git a/guix/scripts/package.scm b/guix/scripts/package.scm
index 9ff4d17..21dc66c 100644
--- a/guix/scripts/package.scm
+++ b/guix/scripts/package.scm
@@ -668,8 +668,12 @@ removed from MANIFEST."
(define (guix-package . args)
(define (parse-options)
;; Return the alist of option values.
- (args-fold* (append args (environment-build-options))
- %options
+ (append (parse-options-from args)
+ (parse-options-from (environment-build-options))))
+
+ (define (parse-options-from args)
+ ;; Actual parsing takes place here.
+ (args-fold* args %options
(lambda (opt name arg result arg-handler)
(leave (_ "~A: unrecognized option~%") name))
(lambda (arg result arg-handler)
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 8e049a4..eb987e1 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -471,8 +471,12 @@ Build the operating system declared in FILE according to ACTION.\n"))
(define (guix-system . args)
(define (parse-options)
;; Return the alist of option values.
- (args-fold* (append args (environment-build-options))
- %options
+ (append (parse-options-from args)
+ (parse-options-from (environment-build-options))))
+
+ (define (parse-options-from args)
+ ;; Actual parsing takes place here.
+ (args-fold* args %options
(lambda (opt name arg result)
(leave (_ "~A: unrecognized option~%") name))
(lambda (arg result)
--
2.1.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] guix: scripts: Fix GUIX_BUILD_OPTIONS handling.
2014-12-05 0:53 ` [PATCH] guix: scripts: Fix GUIX_BUILD_OPTIONS handling Deck Pickard
@ 2014-12-06 15:04 ` Ludovic Courtès
2015-02-20 20:47 ` Bugs in parsing build options Alex Kost
0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2014-12-06 15:04 UTC (permalink / raw)
To: Deck Pickard; +Cc: guix-devel
Deck Pickard <deck.r.pickard@gmail.com> skribis:
> From 9c27d995e1a622de8457209d40031b392538e0f8 Mon Sep 17 00:00:00 2001
> From: nebuli <nebu@kipple>
> Date: Fri, 5 Dec 2014 01:28:12 +0100
> Subject: [PATCH] guix: scripts: Fix GUIX_BUILD_OPTIONS handling.
>
> Appending to "raw" args broke optional parameters in 'guix package -I'
> and 'guix package -A', and possibly other places. Therefore, switch to
> parsing each set of options on its own and append resulting alists
> together afterwards.
>
> * guix/scripts/archive.scm (parse-options-from): Rename from
> (parse-options) and add explicit argument. New form of (parse-options)
> using its old algorithm via -from function.
> * guix/scripts/build.scm: Ditto.
> * guix/scripts/environment.scm: Ditto.
> * guix/scripts/package.scm: Ditto.
> * guix/scripts/system.scm: Ditto.
Good catch. I’ve applied it along with a test case that reproduces the
problem and a clarification in the doc.
Thanks!
Ludo’.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Bugs in parsing build options
2014-12-06 15:04 ` Ludovic Courtès
@ 2015-02-20 20:47 ` Alex Kost
2015-02-24 22:45 ` Ludovic Courtès
0 siblings, 1 reply; 10+ messages in thread
From: Alex Kost @ 2015-02-20 20:47 UTC (permalink / raw)
To: guix-devel
Ludovic Courtès (2014-12-06 18:04 +0300) wrote:
> Deck Pickard <deck.r.pickard@gmail.com> skribis:
>
>> From 9c27d995e1a622de8457209d40031b392538e0f8 Mon Sep 17 00:00:00 2001
>> From: nebuli <nebu@kipple>
>> Date: Fri, 5 Dec 2014 01:28:12 +0100
>> Subject: [PATCH] guix: scripts: Fix GUIX_BUILD_OPTIONS handling.
>>
>> Appending to "raw" args broke optional parameters in 'guix package -I'
>> and 'guix package -A', and possibly other places. Therefore, switch to
>> parsing each set of options on its own and append resulting alists
>> together afterwards.
>>
>> * guix/scripts/archive.scm (parse-options-from): Rename from
>> (parse-options) and add explicit argument. New form of (parse-options)
>> using its old algorithm via -from function.
>> * guix/scripts/build.scm: Ditto.
>> * guix/scripts/environment.scm: Ditto.
>> * guix/scripts/package.scm: Ditto.
>> * guix/scripts/system.scm: Ditto.
>
> Good catch. I’ve applied it along with a test case that reproduces the
> problem and a clarification in the doc.
Hello, I think I found a problem with this commit.
I noticed that "--no-grub" option has no effect. For example,
"guix system --no-grub reconfigure" installs GRUB anyway.
Further investigation showed that there is a problem in parsing build
options. After that commit (847391f) we have the following code:
(append (parse-options-from args)
(parse-options-from (environment-build-options)))
in several places. But 'parse-options-from' returns default values for
unspecified options, e.g. if you didn't set GUIX_BUILD_OPTIONS, then
(parse-options-from '()) will return an alist of default options
(including ‘(install-grub? . #t)’).
So build commands will just ignore such options as "--no-grub" or
"--no-substitutes" if a user didn't tweak GUIX_BUILD_OPTIONS.
--
Alex
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Bugs in parsing build options
2015-02-20 20:47 ` Bugs in parsing build options Alex Kost
@ 2015-02-24 22:45 ` Ludovic Courtès
2015-02-25 14:38 ` Alex Kost
0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2015-02-24 22:45 UTC (permalink / raw)
To: Alex Kost; +Cc: guix-devel
Alex Kost <alezost@gmail.com> skribis:
> I noticed that "--no-grub" option has no effect. For example,
> "guix system --no-grub reconfigure" installs GRUB anyway.
>
> Further investigation showed that there is a problem in parsing build
> options. After that commit (847391f) we have the following code:
>
> (append (parse-options-from args)
> (parse-options-from (environment-build-options)))
>
> in several places. But 'parse-options-from' returns default values for
> unspecified options, e.g. if you didn't set GUIX_BUILD_OPTIONS, then
> (parse-options-from '()) will return an alist of default options
> (including ‘(install-grub? . #t)’).
Indeed, good catch. Commit 6e1a7d1 fixes it.
> So build commands will just ignore such options as "--no-grub" or
> "--no-substitutes" if a user didn't tweak GUIX_BUILD_OPTIONS.
The problem was in fact specific to the option handler for --no-grub;
the one for --no-substitutes (and all the others I checked) always
conses, so there’s no problem.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Bugs in parsing build options
2015-02-24 22:45 ` Ludovic Courtès
@ 2015-02-25 14:38 ` Alex Kost
2015-02-25 23:05 ` Ludovic Courtès
0 siblings, 1 reply; 10+ messages in thread
From: Alex Kost @ 2015-02-25 14:38 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
Ludovic Courtès (2015-02-25 01:45 +0300) wrote:
> Alex Kost <alezost@gmail.com> skribis:
>
>> I noticed that "--no-grub" option has no effect. For example,
>> "guix system --no-grub reconfigure" installs GRUB anyway.
>>
>> Further investigation showed that there is a problem in parsing build
>> options. After that commit (847391f) we have the following code:
>>
>> (append (parse-options-from args)
>> (parse-options-from (environment-build-options)))
>>
>> in several places. But 'parse-options-from' returns default values for
>> unspecified options, e.g. if you didn't set GUIX_BUILD_OPTIONS, then
>> (parse-options-from '()) will return an alist of default options
>> (including ‘(install-grub? . #t)’).
>
> Indeed, good catch. Commit 6e1a7d1 fixes it.
Not really, I'm afraid.
>> So build commands will just ignore such options as "--no-grub" or
>> "--no-substitutes" if a user didn't tweak GUIX_BUILD_OPTIONS.
>
> The problem was in fact specific to the option handler for --no-grub;
> the one for --no-substitutes (and all the others I checked) always
> conses, so there’s no problem.
Yes, I realized that, but I believe the real problem lies deeper – it is
in the current way of parsing build options in general (sorry, I should
have written more about that initially).
Suppose a user specifies "--no-substitutes" in his GUIX_BUILD_OPTIONS
and then he calls "guix system reconfigure". What would happen?
Substitutes _will be used_ anyway, because:
‘(parse-options-from args)’ will contain ‘(substitutes? . #t)’ among
other things and it will shadow the false value for substitutes returned
by ‘(parse-options-from (environment-build-options))’.
Now (after your patch) the same will happen with "--no-grub":
“export GUIX_BUILD_OPTIONS=--no-grub” will not be honored unless a user
explicitly specifies "--no-grub" option one more time in a "guix system"
command.
That's why I didn't propose a patch initially: I don't really know what
would be the best way to fix all that.
--
Alex
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Bugs in parsing build options
2015-02-25 14:38 ` Alex Kost
@ 2015-02-25 23:05 ` Ludovic Courtès
2015-02-26 21:08 ` Alex Kost
2015-05-30 16:26 ` Alex Kost
0 siblings, 2 replies; 10+ messages in thread
From: Ludovic Courtès @ 2015-02-25 23:05 UTC (permalink / raw)
To: Alex Kost; +Cc: guix-devel
Alex Kost <alezost@gmail.com> skribis:
> Suppose a user specifies "--no-substitutes" in his GUIX_BUILD_OPTIONS
> and then he calls "guix system reconfigure". What would happen?
> Substitutes _will be used_ anyway, because:
> ‘(parse-options-from args)’ will contain ‘(substitutes? . #t)’ among
> other things and it will shadow the false value for substitutes returned
> by ‘(parse-options-from (environment-build-options))’.
>
> Now (after your patch) the same will happen with "--no-grub":
> “export GUIX_BUILD_OPTIONS=--no-grub” will not be honored unless a user
> explicitly specifies "--no-grub" option one more time in a "guix system"
> command.
Oh, that’s right. AFAICS, commit cf6ce3e fixes it.
It was a good opportunity to factorize all that and to add tests.
Thanks!
Ludo’.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Bugs in parsing build options
2015-02-25 23:05 ` Ludovic Courtès
@ 2015-02-26 21:08 ` Alex Kost
2015-05-30 16:26 ` Alex Kost
1 sibling, 0 replies; 10+ messages in thread
From: Alex Kost @ 2015-02-26 21:08 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
Ludovic Courtès (2015-02-26 02:05 +0300) wrote:
> Alex Kost <alezost@gmail.com> skribis:
>
>> Suppose a user specifies "--no-substitutes" in his GUIX_BUILD_OPTIONS
>> and then he calls "guix system reconfigure". What would happen?
>> Substitutes _will be used_ anyway, because:
>> ‘(parse-options-from args)’ will contain ‘(substitutes? . #t)’ among
>> other things and it will shadow the false value for substitutes returned
>> by ‘(parse-options-from (environment-build-options))’.
>>
>> Now (after your patch) the same will happen with "--no-grub":
>> “export GUIX_BUILD_OPTIONS=--no-grub” will not be honored unless a user
>> explicitly specifies "--no-grub" option one more time in a "guix system"
>> command.
>
> Oh, that’s right. AFAICS, commit cf6ce3e fixes it.
Yes, thank you, now I'm satisfied :-)
> It was a good opportunity to factorize all that and to add tests.
>
> Thanks!
>
> Ludo’.
Thank YOU for being so productive!
--
Alex
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Bugs in parsing build options
2015-02-25 23:05 ` Ludovic Courtès
2015-02-26 21:08 ` Alex Kost
@ 2015-05-30 16:26 ` Alex Kost
2015-05-31 19:32 ` Ludovic Courtès
1 sibling, 1 reply; 10+ messages in thread
From: Alex Kost @ 2015-05-30 16:26 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
Ludovic Courtès (2015-02-26 02:05 +0300) wrote:
> Alex Kost <alezost@gmail.com> skribis:
>
>> Suppose a user specifies "--no-substitutes" in his GUIX_BUILD_OPTIONS
>> and then he calls "guix system reconfigure". What would happen?
>> Substitutes _will be used_ anyway, because:
>> ‘(parse-options-from args)’ will contain ‘(substitutes? . #t)’ among
>> other things and it will shadow the false value for substitutes returned
>> by ‘(parse-options-from (environment-build-options))’.
>>
>> Now (after your patch) the same will happen with "--no-grub":
>> “export GUIX_BUILD_OPTIONS=--no-grub” will not be honored unless a user
>> explicitly specifies "--no-grub" option one more time in a "guix system"
>> command.
>
> Oh, that’s right. AFAICS, commit cf6ce3e fixes it.
>
> It was a good opportunity to factorize all that and to add tests.
I think I've found that there are still some problems in parsing options.
1. The following command:
GUIX_BUILD_OPTIONS="--no-grub" guix package --help
gives the following error:
guix package: error: no-grub: unrecognized option
So it makes impossible to use GUIX_BUILD_OPTIONS env var or did I miss
anything?
2. The manual says that ‘guix package’ «supports all the common build
options that ‘guix build’ supports» and the following command works
perfectly:
guix build --system=i686-linux hello
However, the following command:
guix package --system=i686-linux --install hello
gives an error again:
guix package: error: system=i686-linux: unrecognized option
--
Alex
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Bugs in parsing build options
2015-05-30 16:26 ` Alex Kost
@ 2015-05-31 19:32 ` Ludovic Courtès
2015-06-01 6:52 ` Alex Kost
0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2015-05-31 19:32 UTC (permalink / raw)
To: Alex Kost; +Cc: guix-devel
Alex Kost <alezost@gmail.com> skribis:
> 1. The following command:
>
> GUIX_BUILD_OPTIONS="--no-grub" guix package --help
>
> gives the following error:
>
> guix package: error: no-grub: unrecognized option
>
> So it makes impossible to use GUIX_BUILD_OPTIONS env var or did I miss
> anything?
GUIX_BUILD_OPTIONS is intended for options that related to building,
namely those listed in ‘%standard-build-options’ in (guix scripts
build).
Conversely, ‘--no-grub’ is specific to ‘guix system’.
> 2. The manual says that ‘guix package’ «supports all the common build
> options that ‘guix build’ supports» and the following command works
> perfectly:
>
> guix build --system=i686-linux hello
>
> However, the following command:
>
> guix package --system=i686-linux --install hello
>
> gives an error again:
>
> guix package: error: system=i686-linux: unrecognized option
Same issue: the manual is somewhat vague, but ‘--system’ is not in
‘%standard-build-options’.
‘--system’ was intentionally not added to ‘guix package’ on the grounds
that it wouldn’t be very useful to install binaries built for another
system in a profile.
Now, I agree that the situation is different on “multi-personality”
systems, so maybe we should revise that choice?
Thoughts?
Ludo’.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Bugs in parsing build options
2015-05-31 19:32 ` Ludovic Courtès
@ 2015-06-01 6:52 ` Alex Kost
0 siblings, 0 replies; 10+ messages in thread
From: Alex Kost @ 2015-06-01 6:52 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
Ludovic Courtès (2015-05-31 22:32 +0300) wrote:
> Alex Kost <alezost@gmail.com> skribis:
>
>> 1. The following command:
>>
>> GUIX_BUILD_OPTIONS="--no-grub" guix package --help
>>
>> gives the following error:
>>
>> guix package: error: no-grub: unrecognized option
>>
>> So it makes impossible to use GUIX_BUILD_OPTIONS env var or did I miss
>> anything?
>
> GUIX_BUILD_OPTIONS is intended for options that related to building,
> namely those listed in ‘%standard-build-options’ in (guix scripts
> build).
>
> Conversely, ‘--no-grub’ is specific to ‘guix system’.
Ah, indeed! I missed that GUIX_BUILD_OPTIONS is intended to have only
‘build’ options, not ‘system’ ones, sorry.
>> 2. The manual says that ‘guix package’ «supports all the common build
>> options that ‘guix build’ supports» and the following command works
>> perfectly:
>>
>> guix build --system=i686-linux hello
>>
>> However, the following command:
>>
>> guix package --system=i686-linux --install hello
>>
>> gives an error again:
>>
>> guix package: error: system=i686-linux: unrecognized option
>
> Same issue: the manual is somewhat vague, but ‘--system’ is not in
> ‘%standard-build-options’.
>
> ‘--system’ was intentionally not added to ‘guix package’ on the grounds
> that it wouldn’t be very useful to install binaries built for another
> system in a profile.
>
> Now, I agree that the situation is different on “multi-personality”
> systems, so maybe we should revise that choice?
>
> Thoughts?
I believe that ‘guix package’ should support ‘--system’ option. At
least I have a need to install some packages of another system to my
profile.
--
Alex
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-06-01 6:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAJ41eeyECrUPfEhS+9puU3y0iczEeCs8JgKB8RnTRofVNOW_4w@mail.gmail.com>
2014-12-05 0:53 ` [PATCH] guix: scripts: Fix GUIX_BUILD_OPTIONS handling Deck Pickard
2014-12-06 15:04 ` Ludovic Courtès
2015-02-20 20:47 ` Bugs in parsing build options Alex Kost
2015-02-24 22:45 ` Ludovic Courtès
2015-02-25 14:38 ` Alex Kost
2015-02-25 23:05 ` Ludovic Courtès
2015-02-26 21:08 ` Alex Kost
2015-05-30 16:26 ` Alex Kost
2015-05-31 19:32 ` Ludovic Courtès
2015-06-01 6:52 ` Alex Kost
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.