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

[Maxim Cournoyer]

Hi Ludovic,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

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

I've now pushed it as 6d9d616113cf051d80567b584a5b0a6489ddc065.  Thanks
for the review!

Maxim