From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35972) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d7ovy-00041r-Dv for guix-patches@gnu.org; Mon, 08 May 2017 16:07:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d7ovv-00086D-7l for guix-patches@gnu.org; Mon, 08 May 2017 16:07:06 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:57551) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1d7ovv-00085y-3u for guix-patches@gnu.org; Mon, 08 May 2017 16:07:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1d7ovu-0005T2-BW for guix-patches@gnu.org; Mon, 08 May 2017 16:07:02 -0400 Subject: bug#26339: [PATCH 02/18] system: Add extlinux support. Resent-Message-ID: From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) References: <20170402135242.2958-1-m.othacehe@gmail.com> <20170402135242.2958-2-m.othacehe@gmail.com> Date: Mon, 08 May 2017 22:06:18 +0200 In-Reply-To: <20170402135242.2958-2-m.othacehe@gmail.com> (Mathieu Othacehe's message of "Sun, 2 Apr 2017 15:52:26 +0200") Message-ID: <87mvanjepx.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Mathieu Othacehe Cc: 26339@debbugs.gnu.org, David Craven Hello! Sorry for the delay, and thanks for reminding me of this patch. Mathieu Othacehe skribis: > From: David Craven > > * gnu/system.scm (operating-system): Add default bootloader. > (operating-system-grub.cfg): Use bootloader-configuration-file-procedur= e. > * gnu/system/grub.scm (bootloader-configuration->grub-configuration): New > variable. > (grub-configuration-file): Use bootloader-configuration->grub-configura= tion. > * guix/scripts/system.scm (profile-grub-entries): Rename system->grub-ent= ry to > system->boot-parameters and adjust accordingly. > (perform-action): Make bootloader optional. Use > bootloader-configuration-device. > * gnu/system/bootloader.scm: New file. > * gnu/local.mk (GNU_SYSTEM_MODULES): Add it. > * tests/system.scm: Adjust operating-system to new API. > * tests/guix-system.sh: Adjust operating-system to new API. [...] > +(define-record-type* > + bootloader-configuration make-bootloader-configuration > + bootloader-configuration? > + (bootloader bootloader-configuration-bootloader = ; package > + (default #f)) > + (device bootloader-configuration-device = ; string > + (default #f)) > + (menu-entries bootloader-configuration-menu-entries= ; list of > + (default '())) > + (default-entry bootloader-configuration-default-entr= y ; integer > + (default 0)) > + (timeout bootloader-configuration-timeout = ; integer > + (default 5)) > + (configuration-file-location bootloader-configuration-file-location > + (default #f)) > + (configuration-file-procedure bootloader-configuration-file-procedu= re ; procedure > + (default #f)) > + (install-procedure bootloader-configuration-install-proc= edure ; procedure > + (default #f)) > + (additional-configuration bootloader-configuration-additional-c= onfiguration ; record > + (default #f))) > + To avoid mistakes, I think we should remove default values for fields that really need to be set. For example, =E2=80=98configuration-file-locat= ion=E2=80=99 and =E2=80=98install-procedure=E2=80=99 should probably not have a default = value. I would rename =E2=80=98configuration-file-location=E2=80=99 to =E2=80=98co= nfiguration-file=E2=80=99. I=E2=80=99m fine with =E2=80=98configuration-file-procedure=E2=80=99, but = =E2=80=98generator=E2=80=99 is OK too. I would make =E2=80=98bootloader=E2=80=99 a delayed field (see the =E2=80= =98patches=E2=80=99 field of for an example of that.) That would avoid loading the package modules upfront. > +;;; > +;;; Bootloader configurations. > +;;; > + > +(define* (extlinux-configuration #:optional (config (bootloader-configur= ation))) > + (bootloader-configuration > + (inherit config) > + (configuration-file-location "/boot/extlinux/extlinux.conf") > + (configuration-file-procedure extlinux-configuration-file))) > + > +(define* (grub-configuration #:optional (config (bootloader-configuratio= n))) > + (bootloader-configuration > + (inherit config) > + (bootloader (@ (gnu packages bootloaders) grub)) > + (configuration-file-location "/boot/grub/grub.cfg") > + (configuration-file-procedure grub-configuration-file) > + (install-procedure install-grub) > + (additional-configuration > + (let ((additional-config (bootloader-configuration-additional-config= uration config))) > + (if additional-config additional-config %default-theme))))) > + > +(define* (grub-efi-configuration #:optional (config (bootloader-configur= ation))) > + (bootloader-configuration > + (inherit (grub-configuration config)) > + (bootloader (@ (gnu packages bootloaders) grub-efi)))) > + > +(define* (syslinux-configuration #:optional (config (bootloader-configur= ation))) > + (bootloader-configuration > + (inherit (extlinux-configuration config)) > + (bootloader (@ (gnu packages bootloaders) syslinux)) > + (install-procedure install-syslinux))) So it looks like this is the reason for all the default values. What about distinguishing two things: bootloader configuration, and bootloader implementation? This would be akin to the relation between and . We=E2=80=99d have , but without the =E2=80=98boot= loader=E2=80=99, =E2=80=98install-procedure=E2=80=99, =E2=80=98configuration-file-procedure= =E2=80=99, and =E2=80=98configuration-file=E2=80=99 fields. Instead of those 4 fields, we= =E2=80=99d have a single =E2=80=98bootloader=E2=80=99 field whose value would be a record. The record would contain these 4 fields (package, install-procedure, configuration-file, configuration-file-procedure), maybe along with a =E2=80=98name=E2=80=99 field to aid debugging. The record would define the implementation (GRUB, extlinux, u-boot, etc.) while would be purely configuration. How does that sound? We could do: (define grub-bootloader (bootloader (package grub) =E2=80=A6)) in (gnu bootloader grub). Likewise we=E2=80=99d define =E2=80=98extlinux-bootloader=E2=80=99 in (gnu = bootloader extlinux). The (gnu system) module would include (gnu bootloader), but it would not include (gnu bootloader grub) nor (gnu bootloader extlinux). Users would include one of these in their config file instead. > --- a/tests/system.scm > +++ b/tests/system.scm > @@ -36,7 +36,6 @@ > (host-name "komputilo") > (timezone "Europe/Berlin") > (locale "en_US.utf8") > - (bootloader (grub-configuration (device "/dev/sdX"))) > (file-systems (cons %root-fs %base-file-systems)) In gnu/system.scm, there=E2=80=99s: > + (bootloader operating-system-bootloader ; > + (default (extlinux-configuration))) If there was a default value here, I think it=E2=80=99d rather be GRUB ;-),= but probably we=E2=80=99d better not have a default at all. How would the installation device be found? Thanks a lot, and sorry for the loooong delay! Ludo=E2=80=99.