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

From: Mathieu Othacehe <[email-masked]>
Date: Tue, 16 May 2017 14:46:24 +0200

Hi Danny,

> Can you add a comment about the unit?

Done.

> Left-over debugging message?

Ooop ;)

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

Yes you're right there were a confusion here. I fixed it before applying.
>
>>+++ 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?

Yes the will because most modules included (gnu) which no includes (gnu
bootloader grub). For modules directly including (gnu system grub), this
patch was already ok. See (gnu tests) for instance.

>
>> -       (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 :)

Yop !

>
> Remainder of this patch LGTM!

Thanks for your review !

Mathieu