[bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.

[Stefan]

Hi Efraim!

>> +         (boot-efi-link (match system
>> +                          (("i686" _ ...) "/bootia32.efi")
>> +                          (("x86_64" _ ...) "/bootx64.efi")
>> +                          (("arm" _ ...) "/bootarm.efi")
>> +                          (("armhf" _ ...) "/bootarm.efi")
>> +                          (("aarch64" _ ...) "/bootaa64.efi")
>> +                          (("riscv" _ ...) "/bootriscv32.efi")
>> +                          (("riscv64" _ ...) "/bootriscv64.efi")))
> 
> Don't forget the fall-through case, even if it's just
> ((_ ...) "/bootTODO.efi")

There was a contradicting remark by Danny:

> The major advantage of using "match" is its failure mode.  If the thing 
> matched
> on is not a list (for some unfathomable reason) or if the first element is not
> matched on (!) then you get an exception--which is much better than doing 
> weird
> unknown stuff.

Actually I would prefer an error here as well. Imagine a successful ‘guix 
system init’ silently creating a bootTODO.efi. Booting that system will 
certainly fail and someone will have a hard time to figure out that the 
generated bootTODO.efi is the reason.

>> +         (efi-bootloader (string-append (match system
>> +                                          (("i686" _ ...) "i386-efi")
>> +                                          (("x86_64" _ ...) "x86_64-efi")
>> +                                          (("arm" _ ...) "arm-efi")
>> +                                          (("armhf" _ ...) "arm-efi")
>> +                                          (("aarch64" _ ...) "arm64-efi")
>> +                                          (("riscv" _ ...) "riscv32-efi")
>> +                                          (("riscv64" _ ...) "riscv64-efi"))
>> +                                        "/core.efi")))
> 
> With the fall-through case here, can this be changed to
> (("i686" _ ...) "i386-efi")
> (("aarch64" _ ...) "arm64-efi")
> (("riscv" _ ...) "riscv32-efi")
> ((_ ...) (string-append (first
>                         (string-split
>                           (nix-system->gnu-triplet
>                             (or (%current-system)
>                                 (%current-target-system)))
>                           #\_))
>                        "-efi"))

There was a contradicting remark by Danny, which applies to the first part as 
well:

> Also, I have a slight preference for greppable file names even when it's a
> little more redundant

I understand your point in generating the arch part. I also understand the 
usage of (nix-system->gnu-triplet) to convert armhf to arm. I also think the 
risk is low that the first part raises no error and this part is doing 
something wrong without raising an error, too, leading to a not booting system. 
So having a default here seems fine to me.

However, Danny’s argument is convincing, too. And keeping this block similar to 
the first block eases the understanding, at least mine. My first thought seeing 
your suggestion was: “Why does this block need to be different from the other 
and what is this code doing differently?”

What do you think?


Bye

Stefan