Hi Ludovic, Ludovic Courtès writes: > Hi! > > Maxim Cournoyer skribis: > >> This version field exposes the (already present) version information of a boot >> parameters file. >> >> * gnu/system.scm (%boot-parameters-version): New variable. >> ()[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