unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Efraim Flashner <efraim@flashner.co.il>
To: Stefan <stefan-guix@vodafonemail.de>
Cc: Danny Milosavljevic <dannym@scratchpost.org>, 41011@debbugs.gnu.org
Subject: [bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP.
Date: Wed, 16 Sep 2020 10:51:17 +0300	[thread overview]
Message-ID: <20200916075117.GE19874@E5400> (raw)
In-Reply-To: <A46A1D1F-EA3F-46EE-8968-ACC14435F667@vodafonemail.de>

[-- Attachment #1: Type: text/plain, Size: 4786 bytes --]

On Tue, Sep 15, 2020 at 10:28:53PM +0200, Stefan wrote:
> 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.
> 

My concern is more for architectures which aren't on the list having
unfortunate errors while doing something unrelated. Another option then
I suppose would be
((_ ...) #f)
It should still fail if you try to use it but there's still a code path
for, say, ppc64el. I like the #f idea better than bootTODO.efi.

> >> +         (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"))

I re-noticed that system is passed above so my code block could be a bit
more contained

((_ ...) (string-append (first
                          (string-split
                            (nix-system->gnu-triplet 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?
> 

The benefit is that there's less chance of typoing a mistake in the
code, either when writing it or when editing it later. There's also the
benefit again of not dismissing architectures which are currently not
listed in the list.

For something greppable, I don't really have a counter argument. Perhaps
a hand-wavey "code correctness" of reusing macros.

For unsupported architectures, ((_ ...) #f) again would work to make
sure there is at least a code path which would definitely fail if
someone tried to use it. That's my primary concern.

> 
> Bye
> 
> Stefan
> 

Thanks

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-09-16  7:52 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-01 20:32 [bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs Stefan
2020-05-10  8:20 ` Mathieu Othacehe
2020-05-10 21:13   ` Stefan
2020-05-18 21:43     ` Stefan
2020-05-21 15:07       ` Stefan
2020-05-21 18:40         ` Stefan
2020-05-23  8:10           ` Mathieu Othacehe
2020-05-24  0:22             ` Stefan
2020-05-23  8:02     ` Mathieu Othacehe
2020-05-24 10:18       ` Stefan
2020-05-24 11:00         ` Danny Milosavljevic
2020-05-24 13:09           ` Stefan
2020-05-24 13:42             ` Danny Milosavljevic
2020-05-24 13:58               ` Danny Milosavljevic
2020-05-24 17:06                 ` Stefan
2020-05-24 16:47               ` Stefan
2020-06-06 13:30         ` Stefan
2020-06-06 13:33           ` Stefan
2020-06-06 17:37             ` Danny Milosavljevic
     [not found]               ` <46CD97B3-9994-4AB7-AA7D-4DE39AB7A238@vodafonemail.de>
2020-06-09 13:44                 ` Danny Milosavljevic
2020-06-09 14:25                   ` Stefan
2020-06-11  4:21                   ` Maxim Cournoyer
2020-06-11 11:36                     ` Stefan
2020-06-11 13:07                       ` Maxim Cournoyer
2020-06-11 13:19                     ` Danny Milosavljevic
2020-06-12 14:41                       ` Stefan
2020-06-14 18:56                       ` Maxim Cournoyer
2020-06-11 23:43                     ` [bug#41820] [PATCH] file-systems: Add record type <nfs-share> for a file system device Stefan
2020-06-20 13:52                       ` Stefan
2020-06-12  0:06                     ` [bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs Stefan
2020-06-14 19:09                       ` Maxim Cournoyer
2020-06-17 13:12                         ` Stefan
2020-09-05 11:25 ` Stefan
2020-09-06 13:07   ` Stefan
2020-09-06 14:35   ` Danny Milosavljevic
2020-09-06 15:14     ` Danny Milosavljevic
2020-09-07 22:59     ` Stefan
2020-09-08 22:37       ` Danny Milosavljevic
2020-09-13 17:46         ` [bug#41011] [PATCH] gnu: grub: Support for network boot via TFTP Stefan
2020-09-14  6:59           ` Efraim Flashner
2020-09-15 20:28             ` Stefan
2020-09-16  7:51               ` Efraim Flashner [this message]
2020-09-19 17:54                 ` Stefan
2020-09-20 11:47                   ` Stefan
2020-09-20 11:56                     ` Stefan
2020-09-26 10:52                       ` Stefan
2020-09-26 10:54                       ` Stefan
2020-09-26 16:13                         ` Danny Milosavljevic
2020-09-27 10:50                           ` Stefan
2020-09-27 10:51                             ` Stefan
2020-09-27 11:47                               ` Danny Milosavljevic
2020-09-14 12:34           ` Danny Milosavljevic
2020-09-15 22:10           ` Danny Milosavljevic
2020-09-27 11:57 ` bug#41011: " Stefan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200916075117.GE19874@E5400 \
    --to=efraim@flashner.co.il \
    --cc=41011@debbugs.gnu.org \
    --cc=dannym@scratchpost.org \
    --cc=stefan-guix@vodafonemail.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).