From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: guix bootloader selection - wip patch Date: Thu, 28 Jul 2016 14:34:38 +0200 Message-ID: <874m7aj601.fsf@gnu.org> References: <20160721223501.3a989d55@scratchpost.org> <877fc815wg.fsf@gnu.org> <20160727113248.2ee9087c@scratchpost.org> <20160727222924.07209026@scratchpost.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:39071) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bSkWW-0006tx-1r for guix-devel@gnu.org; Thu, 28 Jul 2016 08:34:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bSkWQ-0003Ds-4B for guix-devel@gnu.org; Thu, 28 Jul 2016 08:34:47 -0400 In-Reply-To: <20160727222924.07209026@scratchpost.org> (Danny Milosavljevic's message of "Wed, 27 Jul 2016 22:29:24 +0200") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Danny Milosavljevic Cc: guix-devel Danny Milosavljevic skribis: > so far I came up with the patch to Guix below for the actual bootloader s= election. Nice! > Some places are still broken. Search for "FIXME" below. > > For example I need a way to find out what the bootloader config file is s= upposed to be called in the new routine 'install-bootloader . It will get (= derivation->output-path bootloader-configuration-file) as argument. Given i= t, can I still find out whether the filename is "grub.cfg" or "extlinux.con= f"? Is that safe enough? [...] > +(define* (install-u-boot extlinux.conf device mount-point) > + "Install U-Boot with EXTLINUX.CONF on DEVICE, which is assumed to be m= ounted on > +MOUNT-POINT. FIXME is that correct?" > + (install-bootloader-config extlinux.conf > + (string-append mount-point > + "/extlinux.conf")) > + (unless (zero? (system* "u-boot-install" > + (string-append "--boot-directory=3D" mount-p= oint) > + device)) > + (error "failed to install U-Boot"))) Really, installing U-Boot is as simple as this? If it is, that=E2=80=99s perfect. :-) > -(define* (operating-system-grub.cfg os #:optional (old-entries '())) > - "Return the GRUB configuration file for OS. Use OLD-ENTRIES to popula= te the > +(define (bootloader-configuration-device bootloader-configuration) > + (match bootloader-configuration > + (($ config) > + (grub-configuration-device config)) > + (($ config) > + (u-boot-configuration-device config)))) Very good, this is where the bootloader selection should occur. It seems to me that it=E2=80=99s enough to find out whether to call =E2=80=98i= nstall-grub=E2=80=99 or =E2=80=98install-u-boot=E2=80=99, no? I would write it as: (match bootloader-configuration ((? grub-configuration? config) (grub-configuration-device config)) ((? u-boot-configuration? config) (u-boot-configuration-device config))) which does the same thing but allows us to avoid exporting and (better to keep them private so that external code doesn=E2=80=99t rely on the structure layout.) I see three separate things here: 1. Replacing =E2=80=9Cgrub=E2=80=9D by =E2=80=9Cbootloader=E2=80=9D in th= e API (cosmetic change); 2. Adding the build-side code to install U-Boot; 3. Adding the host-side code to handle and do the right thing. To facilitate review, could you separate these three things? Also, not critical, but could you send patches as =E2=80=98text/x-patch=E2= =80=99 MIME attachments so that my email client can perform color highlighting? :-) I=E2=80=99m happy to see fast progress on this, thank you! Ludo=E2=80=99.