Re: Bugs in parsing build options

[Alex Kost]

Ludovic Courtès (2015-02-25 01:45 +0300) wrote:

> Alex Kost <address@hidden> 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