Subject: bug#26339: [PATCH v4 1/7] bootloader: Add extlinux support.

From: Danny Milosavljevic <[email-masked]>
Date: Sun, 14 May 2017 10:43:47 +0200

Hi Mathieu,

>+++ b/gnu/bootloader.scm
[...]
>+(define-record-type* <bootloader-configuration>
> +  (menu-entries                    bootloader-configuration-menu-entries   ; list of <boot-parameters>
> +                                   (default '()))

At first I thought this could be confused with grub menu-entries - but on second thought they are similar enough.

> +  (timeout                         bootloader-configuration-timeout        ; integer
> +                                   (default 5))

Can you add a comment about the unit?

> +(define (lookup-bootloader-by-name name)
> +  "Return the bootloader called NAME."
> +  (or (find (lambda (bootloader)
> +              (format #t "~a\n" (eq? name (bootloader-name bootloader)))

Left-over debugging message?

>      (append (map boot-parameters->menu-entry entries)
> -            (grub-configuration-menu-entries config)))
> +            (bootloader-configuration-menu-entries config)))

So bootloader-configuration-menu-entries are grub menu-entries, right?  Earlier the code said they were boot-parameters (see quoted text above).

>+++ b/gnu/bootloader/grub.scm
> +(define-syntax-rule (grub-configuration fields ...)
> +  (bootloader-configuration
> +   (bootloader grub-bootloader)
> +   fields ...))

Will these be able to resolve the symbols used?  Because there are new modules now etc - do old configuration files which don't import those modules still work?

> -       (entry (operating-system-boot-parameters os system root-device)))
> -    ((module-ref (resolve-interface '(gnu system grub))
> -                 'grub-configuration-file)
> -     (operating-system-bootloader os)
> -     (list entry)
> -     #:old-entries old-entries)))
> +       (entry (operating-system-boot-parameters os system root-device))
> +       (bootloader-conf -> (operating-system-bootloader os)))
> +    ((bootloader-configuration-file-generator
> +      (bootloader-configuration-bootloader bootloader-conf))
> +     bootloader-conf (list entry) #:old-entries old-entries)))

Yay my band-aid is gone :)

Remainder of this patch LGTM!