* [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.