From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id TWptHBj0VF87HAAA0tVLHw (envelope-from ) for ; Sun, 06 Sep 2020 14:37:12 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id +M8QGBj0VF/4RAAAbx9fmQ (envelope-from ) for ; Sun, 06 Sep 2020 14:37:12 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 798F194062D for ; Sun, 6 Sep 2020 14:37:11 +0000 (UTC) Received: from localhost ([::1]:47918 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kEvn3-0003md-SC for larch@yhetil.org; Sun, 06 Sep 2020 10:37:09 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52904) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kEvmw-0003li-BC for guix-patches@gnu.org; Sun, 06 Sep 2020 10:37:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:35199) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kEvmw-0003Eu-1E for guix-patches@gnu.org; Sun, 06 Sep 2020 10:37:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kEvmv-0001OK-Uy for guix-patches@gnu.org; Sun, 06 Sep 2020 10:37:01 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#41011] [PATCH] gnu: grub: Support for network boot via tftp/nfs. Resent-From: Danny Milosavljevic Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 06 Sep 2020 14:37:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 41011 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Stefan Cc: 41011@debbugs.gnu.org Received: via spool by 41011-submit@debbugs.gnu.org id=B41011.15994029705290 (code B ref 41011); Sun, 06 Sep 2020 14:37:01 +0000 Received: (at 41011) by debbugs.gnu.org; 6 Sep 2020 14:36:10 +0000 Received: from localhost ([127.0.0.1]:46745 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kEvm5-0001NE-M0 for submit@debbugs.gnu.org; Sun, 06 Sep 2020 10:36:10 -0400 Received: from dd26836.kasserver.com ([85.13.145.193]:39676) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kEvm0-0001N2-FU for 41011@debbugs.gnu.org; Sun, 06 Sep 2020 10:36:08 -0400 Received: from localhost (80-110-126-103.cgn.dynamic.surfer.at [80.110.126.103]) by dd26836.kasserver.com (Postfix) with ESMTPSA id DD86C3363FB1; Sun, 6 Sep 2020 16:36:01 +0200 (CEST) Date: Sun, 6 Sep 2020 16:35:59 +0200 From: Danny Milosavljevic Message-ID: <20200906163559.1b56c36f@scratchpost.org> In-Reply-To: References: <9AAFEFF4-8ACE-4C95-975F-67C3F4FDAF81@vodafonemail.de> X-Mailer: Claws Mail 3.17.5 (GTK+ 2.24.32; x86_64-unknown-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/signed; boundary="Sig_/l9vaRW05C86OpKk/hmO26wf"; protocol="application/pgp-signature"; micalg=pgp-sha512 X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -1.7 (-) X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Spam-Score: -1.11 X-TUID: dB2YbfkBV+Rc --Sig_/l9vaRW05C86OpKk/hmO26wf Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Hi Stefan, I think this looks good in general. I'd like to do some nitpicking on the names--especially since the procedure= is exported and thus presumably can't have its signature modified later without breaking backward compatibility. In this case, the man page grub-mknetdir(8) mentions "netboot" ? Do you think "net" or "netboot" is a better name for this functionality ? On Sat, 5 Sep 2020 13:25:24 +0200 Stefan wrote: > + install-grub-net I'm fine with whatever--but the man page says "netboot". If that's the usu= al name, let's use it. If "net"'s the usual name, let's use that. > + (let* ((arch (car (string-split (or (%current-target-system) > + (%current-system)) > + #\-))) Let's not use arcane Scheme anachronisms like "car". I know most Scheme programmers probably know what it does--but still, better not to use names of registers of a machine no one uses anymore. Better something like this: (let* ((system-parts (string-split (or (%current-target-system) (%current-system)) #\-))) > + (efi-bootloader-link (string-append "/boot" > + (match arch > + ("i686" "ia32") > + ("x86_64" "x64") > + ("arm" "arm") > + ("armhf" "arm") > + ("aarch64" "aa64") > + ("riscv" "riscv32") > + ("riscv64" "riscv64")) > + ".efi")) Also, I have a slight preference for greppable file names even when it's a little more redundant, so more like that: (match system-parts (("i686" _ ...) "ia32.efi") (("x86_64" _ ...) "x64.efi") (("arm" _ ...) "arm.efi") (("armhf" _ ...) "arm.efi") (("aarch64" _ ...) "aa64.efi") (("riscv" _ ...) "riscv32.efi") (("riscv64" _ ...) "riscv64.efi")) > + (efi-bootloader (string-append (match arch > + ("i686" "i386") > + ("x86_64" "x86_64") > + ("arm" "arm") > + ("armhf" "arm") > + ("aarch64" "arm64") > + ("riscv" "riscv32") > + ("riscv64" "riscv64")) > + "-efi/core.efi"))) Likewise: (efi-bootloader (match system-parts (("i686" _ ...) "i386-efi/core.efi") (("x86_64" _ ...) "x86_64-efi/core.efi") (("arm" _ ...) "arm-efi/core.efi") (("armhf" _ ...) "arm-efi/core.efi") (("aarch64" _ ...) "arm64-efi/core.efi") (("riscv" _ ...) "riscv32-efi/core.efi") (("riscv64" _ ...) "riscv64-efi/core.efi")))) > + #~(lambda (bootloader target mount-point) > + "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into > +SUBDIR, which is usually \"efi/boot\" or \"efi/Guix\" below the director= y TARGET > +for the system whose root is mounted at MOUNT-POINT." I think you mean: > + "Install GRUB as e.g. \"bootx64.efi\" or \"bootarm64.efi\" \"into > +SUBDIR (which is usually \"efi/boot\" or \"efi/Guix\") below the directo= ry TARGET > +for the system whose root is mounted at MOUNT-POINT." > + (let* (;; Use target-depth and subdir-depth to construct links to > + ;; "../gnu" and "../../../boot/grub/grub.cfg" with the co= rrect > + ;; number of "../". Note: This doesn't consider ".." or "= .", > + ;; which may appear inside target or subdir. Uhhhh... that could use some more explanationary comments in the source code of why it is done in the first place. Also, is TARGET itself assumed to be an absolute path or is it relative to something else ? According to the rest of the patch it's relative to MOUNT-POINT--but please state this explicitly in the docstring. > + (target-depth (length (delete "" (string-split target #\/= )))) > + (subdir-depth (length (delete "" (string-split #$subdir #= \/)))) > + (up1 (string-join (make-list target-depth "..") "/" 'suff= ix)) Maybe better name: escape-target or something. > + (up2 (string-join (make-list subdir-depth "..") "/" 'suff= ix)) Maybe better name: escape-subdir or something. So this is in order to get out of (string-append TARGET "/" SUBDIR), correc= t? Does the (string-append TARGET "/" SUBDIR) have an official name ? If not, fine. > + (net-dir (string-append mount-point target "/")) So TARGET is relative to MOUNT-POINT ? And MOUNT-POINT is assumed to have a slash at the end ? > + (store-name (car (delete "" (string-split bootloader #\/)= ))) Maybe use match. Also isn't there an official way to find out how the store is called ? (%store-prefix) ? > + (store (string-append up1 store-name)) (string-append escape-target store-name) > + (store-link (string-append net-dir store-name)) *mumbles to self* (string-append MOUNT-POINT TARGET) is net-dir. So it tries to get to (string-append MOUNT-POINT "/gnu"). I vaguely remember our docker pack adding some serious plumbing to support symlinks like that. I'll try to find it. I just wanted to send this E-Mail because of the foll= owing: > ;;; > ;;; Bootloader definitions. > ;;; > +;;; For all these grub-bootloader variables the path to /boot/grub/grub.= cfg > +;;; is fixed. Inheriting and overwriting the field 'configuration-file'= will > +;;; break 'guix system delete-generations', 'guix system switch-generati= on', > +;;; and 'guix system roll-back'. I've added that comment to the source code in an extra commit 3f2bd9df410e85795ec656052f44d2cddec2a060 in guix master. Thank you very much for it. > -(define* grub-minimal-bootloader > +(define grub-minimal-bootloader > (bootloader > -(define* grub-efi-bootloader > +(define grub-efi-bootloader > (bootloader > -(define* grub-mkrescue-bootloader > +(define grub-mkrescue-bootloader I've applied this hunk to guix master as commit 8664c35d6d7fd6e9ce1ca8adefa8070a8e556db4. Thanks. --Sig_/l9vaRW05C86OpKk/hmO26wf Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl9U888ACgkQ5xo1VCww uqXJJAf/ZctzWQXWb20y+oL9pJncfsY9wWhvi8ru1vZ8ft2VvuoqEXjd9sAArASU sBgEtSJfcbygCD2CPRdr9xWxqxR3E/ADl/GKAou+QmlHceLnQLTx8+lVgs/TH9hv fNnvBG0yM9D9/1B4B4KjRURU0KhbXdGRrf/toWgXEW7DFtC7plQv7DCgv55gnV77 TvWg2rGBrRnJAegdKnAVrIGjo89wDwIZ2hmZRZiw57o7WlXigSfZGxJyXbgqkbDD RiYTUcrd5ih4OzdBhBY0rPhE0JStnBxlaK3jBlWL38/RwAf01xPn8YBXExUgGOBH C/s8pDSMqH2H/6UOQ82mAxDkAJOXQg== =KE80 -----END PGP SIGNATURE----- --Sig_/l9vaRW05C86OpKk/hmO26wf--