diff mbox

How to use custom grub?

Message ID 87bmxr8f1s.fsf@kirby.i-did-not-set--mail-host-address--so-tickle-me
State New
Headers show

Commit Message

Marius Bakke Nov. 7, 2016, 3:50 p.m. UTC 
Danny Milosavljevic <dannym@scratchpost.org> writes:

> Hi Marius,
>
> On Mon, 07 Nov 2016 11:36:51 +0000
> Marius Bakke <mbakke@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! :)

Comments

Danny Milosavljevic Nov. 7, 2016, 10:26 p.m. UTC | #1 
Looks good to me!

I also invoked make in the guix directory - which didn't print any new warnings -, reconfigured the system (for non-uefi grub) and successfully booted with it. So all good!
Ludovic Courtès Nov. 8, 2016, 12:40 p.m. UTC | #2 
Marius Bakke <mbakke@fastmail.com> skribis:

> From 5e31312aeae87d63ab2c64e92835231b59c804db Mon Sep 17 00:00:00 2001
> From: Marius Bakke <mbakke@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 <dannym@scratchpost.org>

Also an “LGTM” from me.  Thanks to both of you!

Ludo’.
Marius Bakke Nov. 8, 2016, 2:36 p.m. UTC | #3 
Ludovic Courtès <ludo@gnu.org> writes:

> Marius Bakke <mbakke@fastmail.com> skribis:
>
>> From 5e31312aeae87d63ab2c64e92835231b59c804db Mon Sep 17 00:00:00 2001
>> From: Marius Bakke <mbakke@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 <dannym@scratchpost.org>
>
> Also an “LGTM” from me.  Thanks to both of you!

Cool! I've pushed this as 81bf2ccbc408fc2e959d3f5ab019938dad2ce616.
diff mbox

Patch

From 5e31312aeae87d63ab2c64e92835231b59c804db Mon Sep 17 00:00:00 2001
From: Marius Bakke <mbakke@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 <dannym@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