Maxim Cournoyer <maxim.courno...@gmail.com> skribis: > Ludovic Courtès <l...@gnu.org> writes:
[...] >> There’s no need to have a ‘version’ field in live <boot-parameters> >> records: have the ‘version’ field in the serialized format (the sexp) >> and make sure the deserializer correctly converts to the internal >> representation. >> >> Here, I think you can bump the version number in the serialized form, >> and have ‘read-boot-parameters’ automatically augment ‘kernel-arguments’ >> when VERSION is 0 with “--root=XYZ”. > > I initially went that route, as this was the idea you'd given me > initially. However, 'read-boot-parameters' deals with serializing the > parameters file only; to add 'root', 'gnu.load' and 'gnu.system', the > operating-system object as well as the root device are needed. <boot-parameters> already has ‘root-device’, so that’s fine. But you’re right that the system itself is a problem because that’s self-referential—it’s the thing the “parameters” file is in. Hmm! We could add a substitution mechanism where a literal “$SYSTEM” (say) in the ‘kernel-arguments’ of <boot-parameters> would be substituted by the actual system store file name by ‘read-boot-parameters’, but maybe that’s overkill. So long story short: keeping the ‘version’ field in <boot-parameters> sounds reasonable after all. :-) > The reason 'gnu.load' and 'gnu.system' aren't written to the > parameters file to start with is because they would cause the system > directory to no longer be content-addressable; this is explained in > the docstring of 'operating-system-boot-parameters-file': > > When SYSTEM-KERNEL-ARGUMENTS? is true, add kernel arguments such as 'root' > and 'gnu.load' to the returned file (since the returned file is then > usually > stored into the content-addressed "system" directory, it's usually not a > good idea to give it because the content hash would change by the content > hash > being stored into the "parameters" file). This comment originates in 40fad1c24ce60076e26f6dc8096e4716d31d90c3. I find it a bit misleading because nothing’s “content-addressed”, but I guess it refers to the same problem: that this is self-referential. (There’s only one use of #:system-kernel-arguments? #t. We can remove that keyword parameter from ‘operating-system-boot-parameters-file’ since it’s not used there.) >> Also, you could write the ‘match’ pattern like this: >> >> ('boot-parameters ('version (and version (or 0 1))) >> ('label label) …) > > I think this patch's current form is preferable, as it means future > boot-parameters version bumps won't break older Guices (when > reconfiguring), as long as the version is an exact, non-negative integer > (e.g. when going from 1 to 2). That’s what we want to avoid: bumping the version number means that the new format is not backwards-compatible, and that older Guix versions won’t be able to read it. That’s why I think ‘read-boot-parameters’ needs to be explicit about the version(s) it expects. (A more complete example of this pattern is ‘sexp->manifest’.) Breaking backwards compatibility should be avoided when possible, but it’s not always possible. In ‘read-boot-parameters’, ‘bootloader-name’, ‘bootloader-menu-entries’, ‘kernel’, etc. are handled somewhat weirdly to preserve backwards-compatibility; doing this allowed us to not bump the file format version. Thanks! Ludo’.