bug-guix
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#40998: [PATCH 1/4] system: Add a version field to the <boot-paramete


From: Maxim Cournoyer
Subject: bug#40998: [PATCH 1/4] system: Add a version field to the <boot-parameters> record.
Date: Tue, 01 Mar 2022 09:40:30 -0500
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux)

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi!
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> This version field exposes the (already present) version information of a 
>> boot
>> parameters file.
>>
>> * gnu/system.scm (%boot-parameters-version): New variable.
>> (<boot-parameters>)[version]: New field.
>> (read-boot-parameters): Use it.
>> (operating-system-boot-parameters-file): Likewise.
>> * tests/boot-parameters.scm (test-read-boot-parameters): Use
>> %boot-parameters-version as the default version value in the template.
>
> [...]
>
>> +  ;; New versions are not backward-compatible, so only accept past and 
>> current
>> +  ;; versions, not future ones.
>> +  (define (version? n)
>> +    (member n (iota (1+ %boot-parameters-version))))
>> +
>>    (match (read port)
>> -    (('boot-parameters ('version 0)
>> +    (('boot-parameters ('version (? version? version))
>
> I still have a preference for an explicit list right here, for clarity,
> and so that we don’t unwillingly find ourselves treating any past
> version in the same way in the future.

OK.  I prefer that we can bump %boot-parameters-version at one place and
have the rest of the code base do the right thing instead of having to
manually remember to adjust bits left and right.  I've added a comment
next to %boot-parameters-version to mention it should be incremented by
1 when bumping it.

> I think I wasn’t clear about it (sorry!) but I wonder if we could,
> instead of bumping the version, use something like:
>
>   (find (cut string-prefix? "gnu.load=") kernel-arguments)
>
> to determine whether we’re dealing with an “old-style” “parameters”
> file.
>
> If that’s not possible, then what this patch is doing SGTM.

That's not possible, because the parameters file doesn't include the
gnu.load nor gnu.system parameters because of their self-referential
nature, so we don't have such information to look at.

I'll be looking toward pushing this series today.

Thank you for the review!

Maxim





reply via email to

[Prev in Thread] Current Thread [Next in Thread]