Subject: Re: bug#27007: [PATCH 1/2] bootloader: Use menu-entry to define custom bootloader entries.

From: [email-masked] (Ludovic Courtès)
Date: Mon, 05 Jun 2017 12:23:33 +0200

Hi Mathieu,

Mathieu Othacehe <[email-masked]> skribis:

> * gnu/bootloader.scm (<menu-entry>): New variable. Export associated getters,
> This record is extracted from grub module.
> * gnu/bootloader/extlinux.scm (extlinux-configuration-file): Use
>   menu-entry->boot-parameters to convert menu-entry records to
>   boot-parameters.
> * gnu/bootloader/grub.scm (<menu-entry>): Remove.
> (boot-parameters->menu-entry): Remove.
> (grub-configuration-file): Use boot-parameters to create configuration
> entries.
> * gnu/system.scm (menu-entry->boot-parameters): New exported procedure.

Overall LGTM.  Some comments and questions:

> +(define-record-type* <menu-entry>
> +  menu-entry make-menu-entry
> +  menu-entry?
> +  (label           menu-entry-label)
> +  (device          menu-entry-device       ; file system uuid, label, or #f
> +                   (default #f))
> +  (device-mount-point menu-entry-device-mount-point
> +                      (default "/"))
> +  (linux           menu-entry-linux)
> +  (linux-arguments menu-entry-linux-arguments
> +                   (default '()))          ; list of string-valued gexps
> +  (initrd          menu-entry-initrd))     ; file name of the initrd as a gexp

Do we still need ‘device-mount-point’ now?  For the dual-boot use case,
I don’t see how this would be used.

> +(define (menu-entry->boot-parameters menu-entry)
> +  "Convert a <menu-entry> instance to a corresponding <boot-parameters>."
> +  (boot-parameters
> +   (label (menu-entry-label menu-entry))
> +   (root-device #f)
> +   (boot-name 'custom)
> +   (store-device (menu-entry-device menu-entry))
> +   (store-mount-point (menu-entry-device-mount-point menu-entry))
> +   (kernel (menu-entry-linux menu-entry))
> +   (kernel-arguments (menu-entry-linux-arguments menu-entry))
> +   (initrd (menu-entry-initrd menu-entry))))

It’s weird to set ‘store-device’ and ‘store-mount-point’ here since
there’s no store (at least when defining a menu entry for another distro
or OS).  Should we set them to #f?

BTW, this is unrelated to this patch series, but I think ‘boot-name’
should be renamed to ‘bootloader-name’ since that’s what it is.

Thank you for working on this!

Ludo’.