Hello, Chris Marusich <cmmarus...@gmail.com> skribis:
> l...@gnu.org (Ludovic Courtès) writes: [...] >> Sorry about that! Hopefully we can work around the conflicts. > > I think we can. But I think it will require backwards incompatible > changes to the boot parameters file. Here's why: > > Many of the existing procedures in (gnu system grub) take a "file > system" object as input (e.g. the 'grub-configuration-file' procedure). > However, the boot parameters file does not currently contain all the > information that a "file system" object contains. Good point. This ‘store-fs’ argument was added in response to <http://bugs.gnu.org/22281>. > Here's an example of what it contains today: > > (boot-parameters > (version 0) > (label "GNU with Linux-Libre 4.1.20 (beta)") > (root-device "root") > (kernel > "/gnu/store/zygby8db0adcyj3m6rjflr80jarfy9b5-linux-libre-4.1.20") > (kernel-arguments ()) > (initrd > (string-append > "/gnu/store/hlra3a0g3a14bjvdn3vbagwfvy4nmhn8-base-initrd" > "/initrd"))) > > To avoid backwards-incompatible changes to the structure of the boot > parameters file, I had originally planned to refactor the procedures in > (gnu system grub) so that I could use them with the limited information > that is contained in the version 0 boot parameters file. However, > commit 0f65f54e has modified these procedures in a way that makes it > very awkward to refactor the "file system" object out of them. Now, to > re-use the existing procedures, I believe I will need to add this > missing information (i.e., the information contained in a file system > object) to the boot parameters file, so that I can construct a "file > system" object to pass to those procedures. Does that sound right to > you? Yes, I think so. More precisely, I think we need to add a ‘device’ field to <menu-entry>, which could be the UUID or label of the device where the kernel and initrd are to be found, or #f, in which case grub.cfg would contain a “search --file” command (instead of “search --label” or “search --fs-uuid”). Correspondingly, we’d add a ‘device’ (or ‘boot-device’?) field to <boot-parameters>. The key is that ‘device’ can be different from ‘root-device’ because the store and root devices can be different from one another. That way we could remove the ‘store-fs’ parameter of ‘grub-configuration-file’ since that information would now be contained in each <menu-entry>. > If I do that, then it will probably be a backwards-incompatible change, > so I will do it in the following way. I will simply store an entire > "file system" object in the boot parameters file. I will bump the > version of the boot parameters file from 0 to 1. To ensure that all new > system generations use version 1, I will update commands like > "reconfigure" to always create a version 1 boot parameters file. I will > make the new commands (roll-back and switch-generation) refuse to switch > to any system generation which uses version 0 (because it isn't possible > to construct a complete "file system" object from a version 0 boot > parameters file). I will also update existing commands like > 'list-generations' so that they will gracefully handle both versions. > > Does this sound like the right approach to you? I think we don’t need to bump the version number: ‘read-boot-parameters’ can simply do what it currently does for ‘initrd’ and ‘kernel-arguments’, which is to provide a default value when they’re missing. Here the default value could be ‘root-device’. > I've tried using 'git send-email' on GuixSD before, and it didn't work > for me (because a mail transfer agent is not running on my GuixSD > system). When the new patches are ready, I'll try once more to get it > working. AFAICT an MTA is not needed. >>> - "Return the GRUB configuration file corresponding to CONFIG, a >>> -<grub-configuration> object, and where the store is available at STORE-FS, >>> a >>> -<file-system> object. OLD-ENTRIES is taken to be a list of menu entries >>> -corresponding to old generations of the system." >>> + "Return a derivation which builds the GRUB configuration file >>> corresponding >>> +to CONFIG, a <grub-configuration> object, and where the store is available >>> at >>> +STORE-FS, a <file-system> object. OLD-ENTRIES is taken to be a list of >>> menu >>> +entries corresponding to old generations of the system." >> >> OK, although I often write “Return something” when that really means >> “Return a derivation that builds something”. > > Upon closer inspection, it looks like this procedure, > 'grub-configuration-file', actually returns a monadic value (in the > store monad), which "contains" a derivation, which in turn builds the > grub configuration file. Even in a case like this, where there is so > much indirection, is it appropriate to elide all those details? > > If this is the style we should consistently use in our documentation, > then that's fine, and I will happily follow suit in the name of > consistency. However, as a newcomer to this code base, to gexps, to > derivations, and to monads, in the beginning I was very confused about > how to use this procedure's return value. > > If I can think of a good way to make stuff like this more obvious for > newcomers, I'll let you know. For now, though, I think the best thing > to do is to change my patches to conform to the existing style. I think so. :-) That said, I can understand that the indirections can be confusing, esp. since these parts are not properly documented. That “return a file” really means “return a derivation as a monadic value” is non obvious. We can now avoid monadic procedures by using the declarative counterpart of the monadic API. That is, we could write: (define (grub-configuration-file …) ;normal proc (computed-file "grub.cfg" builder)) instead of: (define (grub-configuration-file …) ;monadic proc (gexp->derivation "grub.cfg" builder)) I would welcome such changes. > Thank you for your patience. I'll send the updated patches when they're > ready. Awesome, thanks! Ludo’.