Subject: Re: [bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs.

From: Mathieu Othacehe <[email-masked]>
Date: Sun, 10 May 2020 10:20:28 +0200

Hello Stefan,

This patch does not apply here. Could you rebase it on top of master?

> -
> -      ;; Intel and EFI systems need to be switched into graphics mode, whereas
> -      ;; most other modern architectures have no other mode and therefore
> -      ;; don't need to be switched.
> -
> -      ;; XXX: Do we really need to restrict to x86 systems?  We could imitate
> -      ;; what the GRUB default configuration does and decide based on whether
> -      ;; a user provided 'gfxterm' in the terminal-outputs field of their
> -      ;; bootloader-configuration record.
> -      (if (string-match "^(x86_64|i[3-6]86)-" system)
> -          (format #f "
> +    (format #f "
>    set gfxmode=~a
>    insmod all_video
> -  insmod gfxterm~%" gfxmode)
> -          "")))
> +  insmod gfxterm~%"
> +            (string-join
> +             (grub-gfxmode (bootloader-theme config))
> +             ";")))

Why not enable graphic mode only if 'gfxterm' is provided in
terminal-outputs fields, like suggested by the comment?

> +         (efi-bootloader-link (string-append "boot"
> +                                       (match arch
> +                                         ("i686" "ia32")
> +                                         ("x86_64" "x64")
> +                                         ("armhf" "arm")

If cross-building for "arm-linux-gnueabihf", arch will be "arm" and
won't match anything here.

> +          (catch 'system-error
> +            (lambda () (delete-file efi-bootloader-link))
> +            (lambda _ #f))

You can use "false-if-exception" here I think.

> +          (symlink #$efi-bootloader
> +                   efi-bootloader-link)
> +          (catch 'system-error
> +            (lambda () (delete-file store-link))
> +            (lambda _ #f))

Same here.

> +(define* (grub-efi-net-bootloader #:key (target #f) (efi-subdir #f))

#f if implicit if omitted.

> +  (let ((target (or target "boot"))
> +        (efi-subdir (or efi-subdir "efi/boot")))

It would be better to keep grub-efi-net-bootloader as a variable, like
all other bootloaders. You could default configuration-file to
"boot/efi/boot/grub.cfg" instead?

Thanks,

Mathieu