Danny Milosavljevic <dan...@scratchpost.org> writes: > Hi Marius, > > On Mon, 07 Nov 2016 11:36:51 +0000 > Marius Bakke <mba...@fastmail.com> wrote: > >> That's it! This makes it pick up (bootloader (grub-configuration (grub >> grub-efi))) from my config.scm: >> >> making '/gnu/store/kgk9rrawq9fxh1g2j6121gl3lcz47395-system' the current >> system... >> Installing for x86_64-efi platform. >> Installation finished. No error reported. >> >> Even though I'm now working on multi-platform grub, I think we should >> have this anyway so that the "grub" argument works as expected. WDYT? > > Yes, I agree that grub should not be magically picked up but rather be read > from the configuration - as you do here. This makes alternative bootloaders > possible and is also less surprising in any case. > > Also in guix/scripts/system.scm in perform-action there's a (setenv "PATH" > ...) form. I think that one should be replaced, too - for much the same > reasons. It would be better to just pass grub to install-grub* (which would > need its parameter list adapted) instead of mucking with PATH :P > > In this way the package variable would flow from the os configuration all the > way to the actual "grub-install" invocation call without magical environment > variables, packages that just happen to be pulled in from some imported > module etc. > > If you want, you can also fix this one up, too. (If not, I'll wait until your > stuff is merged and fix it myself - no worries) > > A first test whether it was enough is to remove the #:use-module (gnu > packages grub) from guix/scripts/system.scm and see whether it still works > (it should). For clarity I would make the final patch remove it, too.
Hi Danny! You raise some very good points. The patch I just sent indeed works without #:use-module (gnu packages grub), so that should be included. Passing the grub object to grub-install seems like it is better suited for a separate patch. I have a couple of other things on my list before ready to hack on grub-install (need to pass "--efi-directory" somehow), but happy to review any work on it. Attached is the same patch without loading the grub module. Would be great to clear this out of the patch queue. Thanks a lot! :)
signature.asc
Description: PGP signature
>From 5e31312aeae87d63ab2c64e92835231b59c804db Mon Sep 17 00:00:00 2001
From: Marius Bakke <mba...@fastmail.com>
Date: Mon, 7 Nov 2016 11:56:52 +0000
Subject: [PATCH] system: Use grub from bootloader configuration.
* gnu/system/grub.scm (gnu): Export grub-configuration-grub.
* guix/scripts/system.scm (perform-action): Use it.
(define-module): Don't import (gnu packages grub).
Co-authored-by: Danny Milosavljevic <dan...@scratchpost.org>
---
gnu/system/grub.scm | 1 +
guix/scripts/system.scm | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 5c9d0f1..4657b06 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -51,6 +51,7 @@
grub-configuration
grub-configuration?
grub-configuration-device
+ grub-configuration-grub
menu-entry
menu-entry?
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index df9b37d..71ddccf 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -44,7 +44,6 @@
#:use-module (gnu services)
#:use-module (gnu services shepherd)
#:use-module (gnu services herd)
- #:use-module (gnu packages grub)
#:use-module (srfi srfi-1)
#:use-module (srfi srfi-11)
#:use-module (srfi srfi-19)
@@ -617,7 +616,8 @@ building anything."
#:image-size image-size
#:full-boot? full-boot?
#:mappings mappings))
- (grub (package->derivation grub))
+ (grub (package->derivation (grub-configuration-grub
+ (operating-system-bootloader os))))
(grub.cfg (if (eq? 'container action)
(return #f)
(operating-system-grub.cfg os
--
2.10.2
