unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#24346: GuixSD: grub.cfg does not support separate /gnu partition
@ 2016-08-31 21:17 Ludovic Courtès
  2016-09-13 11:23 ` bug#24346: [PATCH] Generate grub.cfg with correct paths when store is not in root partition Carlos Sánchez de La Lama
  0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2016-08-31 21:17 UTC (permalink / raw)
  To: 24346

See details at
<https://lists.gnu.org/archive/html/guix-devel/2016-08/msg02082.html>.

Ludo’.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#24346: [PATCH] Generate grub.cfg with correct paths when store is not in root partition
  2016-08-31 21:17 bug#24346: GuixSD: grub.cfg does not support separate /gnu partition Ludovic Courtès
@ 2016-09-13 11:23 ` Carlos Sánchez de La Lama
  0 siblings, 0 replies; 6+ messages in thread
From: Carlos Sánchez de La Lama @ 2016-09-13 11:23 UTC (permalink / raw)
  To: 24346; +Cc: guix-devel

I had sent it to the wrong bug#, sorry about the noise.

Took a while to produce a working patch :S

A little explanation of what it does:

guix/scripts/system.scm: when constructing old entries, we know
"systems" are the system symlinks in /var/guix/profile, and not a store
derivation. No need for gexps then, which is good as resolving the
symlink using "readlink" cannot be done by the builders /var is not in
the chroot).

gnu/system.scm: for the current system, initrd is inside the system
derivattion in the store. Resolve the symlink at build time.

gnu/system/grub.scm: strip the mount point on the paths GRUB will use,
so GRUB modules, background images. kernels and initrds are correcty
loaded.

This is the first time I do something non-trivial in scheme, so there
might be an easier way to achieve the same result. Nonetheless, I have
tested it on my system and seems to work ok.

* guix/scripts/system.scm (previous-grub-entries): resolve initrd
  symlink for old entries (on the host).
* gnu/system.scm (operating-system-grub.cfg): resolve initrd symlink for
  current system (on the container).
* gnu/system/grub.scm: strip mount-point from GRUB filenames.

---
 gnu/system.scm          |  2 +-
 gnu/system/grub.scm     | 29 ++++++++++++++++++++---------
 guix/scripts/system.scm |  6 +++---
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 0802010..1346f49 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -723,7 +723,7 @@ listed in OS.  The C library expects to find it under
                                    #~(string-append "--load=" #$system
                                                     "/boot")
                                    (operating-system-kernel-arguments os)))
-                           (initrd #~(string-append #$system "/initrd"))))))
+                           (initrd #~(readlink (string-append #$system "/initrd")))))))
     (grub-configuration-file (operating-system-bootloader os)
                              store-fs entries
                              #:old-entries old-entries)))
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 45b46ca..56cb081 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -62,6 +62,14 @@
 ;;;
 ;;; Code:
 
+(define (strip-mount-point fs file)
+  (let ((mount-point (file-system-mount-point fs)))
+    (if (string=? mount-point "/")
+	file
+	#~(if (string-prefix? #$mount-point #$file)
+	      (substring #$file (string-length #$mount-point))
+	      #$file))))
+
 (define-record-type* <grub-image>
   grub-image make-grub-image
   grub-image?
@@ -183,7 +191,8 @@ the store is.  SYSTEM must be the target system string---e.g.,
                      (symbol->string (assoc-ref colors 'bg)))))
 
   (define font-file
-    #~(string-append #$grub "/share/grub/unicode.pf2"))
+    (strip-mount-point root-fs
+		       #~(string-append #$grub "/share/grub/unicode.pf2")))
 
   (mlet* %store-monad ((image (grub-background-image config)))
     (return (and image
@@ -209,7 +218,7 @@ fi~%"
                            #$(grub-root-search root-fs font-file)
                            #$font-file
 
-                           #$image
+                           #$(strip-mount-point root-fs image)
                            #$(theme-colors grub-theme-color-normal)
                            #$(theme-colors grub-theme-color-highlight))))))
 
@@ -254,17 +263,19 @@ corresponding to old generations of the system."
   (define entry->gexp
     (match-lambda
      (($ <menu-entry> label linux arguments initrd)
-      #~(format port "menuentry ~s {
+      (let ((linux (strip-mount-point store-fs linux))
+            (initrd (strip-mount-point store-fs initrd)))
+        #~(format port "menuentry ~s {
   ~a
   linux ~a/~a ~a
   initrd ~a
 }~%"
-                #$label
-                #$(grub-root-search store-fs
-                                    #~(string-append #$linux "/"
-                                                     #$linux-image-name))
-                #$linux #$linux-image-name (string-join (list #$@arguments))
-                #$initrd))))
+                  #$label
+                  #$(grub-root-search store-fs
+                                      #~(string-append #$linux "/"
+                                                       #$linux-image-name))
+                  #$linux #$linux-image-name (string-join (list #$@arguments))
+                  #$initrd)))))
 
   (mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
     (define builder
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 953c624..5ff98a0 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -391,10 +391,10 @@ it atomically, and then run OS's activation script."
         (linux kernel)
         (linux-arguments
          (cons* (string-append "--root=" root-device)
-                #~(string-append "--system=" #$system)
-                #~(string-append "--load=" #$system "/boot")
+                (string-append "--system=" system)
+                (string-append "--load=" system "/boot")
                 kernel-arguments))
-        (initrd #~(string-append #$system "/initrd"))))))
+        (initrd (readlink (string-append system "/initrd")))))))
 
   (let* ((numbers (generation-numbers profile))
          (systems (map (cut generation-file-name profile <>)
-- 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#24346: [PATCH] Generate grub.cfg with correct paths when store is not in root partition
       [not found] <7tlgyw11k3.fsf@kdpof.com>
@ 2016-09-14 14:13 ` Carlos Sánchez de La Lama
  2016-09-24  8:53   ` Ludovic Courtès
  0 siblings, 1 reply; 6+ messages in thread
From: Carlos Sánchez de La Lama @ 2016-09-14 14:13 UTC (permalink / raw)
  To: 24346

Rebasing to current master (76f31f0b), as new file-append made merge
non-trivial.

* guix/scripts/system.scm (previous-grub-entries): resolve initrd
  symlink for old entries (on the host).
* gnu/system.scm (operating-system-grub.cfg): resolve initrd symlink for
  current system (on the container).
* gnu/system/grub.scm: strip mount-point from GRUB filenames.

---
 gnu/system.scm          |  2 +-
 gnu/system/grub.scm     | 25 ++++++++++++++++++-------
 guix/scripts/system.scm |  6 +++---
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 7edb018..281447f 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -738,7 +738,7 @@ listed in OS.  The C library expects to find it under
                                    #~(string-append "--load=" #$system
                                                     "/boot")
                                    (operating-system-kernel-arguments os)))
-                           (initrd (file-append system "/initrd"))))))
+                           (initrd #~(readlink #$(file-append system "/initrd")))))))
     (grub-configuration-file (operating-system-bootloader os)
                              store-fs entries
                              #:old-entries old-entries)))
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 4592747..a00f568 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -62,6 +62,14 @@
 ;;;
 ;;; Code:
 
+(define (strip-mount-point fs file)
+  (let ((mount-point (file-system-mount-point fs)))
+    (if (string=? mount-point "/")
+	file
+	#~(if (string-prefix? #$mount-point #$file)
+	      (substring #$file (string-length #$mount-point))
+	      #$file))))
+
 (define-record-type* <grub-image>
   grub-image make-grub-image
   grub-image?
@@ -183,7 +191,8 @@ the store is.  SYSTEM must be the target system string---e.g.,
                      (symbol->string (assoc-ref colors 'bg)))))
 
   (define font-file
-    #~(string-append #$grub "/share/grub/unicode.pf2"))
+    (strip-mount-point root-fs
+		       #~(string-append #$grub "/share/grub/unicode.pf2")))
 
   (mlet* %store-monad ((image (grub-background-image config)))
     (return (and image
@@ -209,7 +218,7 @@ fi~%"
                            #$(grub-root-search root-fs font-file)
                            #$font-file
 
-                           #$image
+                           #$(strip-mount-point root-fs image)
                            #$(theme-colors grub-theme-color-normal)
                            #$(theme-colors grub-theme-color-highlight))))))
 
@@ -249,15 +258,17 @@ corresponding to old generations of the system."
   (define entry->gexp
     (match-lambda
      (($ <menu-entry> label linux arguments initrd)
-      #~(format port "menuentry ~s {
+      (let ((linux (strip-mount-point store-fs linux))
+            (initrd (strip-mount-point store-fs initrd)))
+        #~(format port "menuentry ~s {
   ~a
   linux ~a ~a
   initrd ~a
 }~%"
-                #$label
-                #$(grub-root-search store-fs linux)
-                #$linux (string-join (list #$@arguments))
-                #$initrd))))
+                  #$label
+                  #$(grub-root-search store-fs linux)
+                  #$linux (string-join (list #$@arguments))
+                  #$initrd)))))
 
   (mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
     (define builder
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 953c624..5ff98a0 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -391,10 +391,10 @@ it atomically, and then run OS's activation script."
         (linux kernel)
         (linux-arguments
          (cons* (string-append "--root=" root-device)
-                #~(string-append "--system=" #$system)
-                #~(string-append "--load=" #$system "/boot")
+                (string-append "--system=" system)
+                (string-append "--load=" system "/boot")
                 kernel-arguments))
-        (initrd #~(string-append #$system "/initrd"))))))
+        (initrd (readlink (string-append system "/initrd")))))))
 
   (let* ((numbers (generation-numbers profile))
          (systems (map (cut generation-file-name profile <>)
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#24346: [PATCH] Generate grub.cfg with correct paths when store is not in root partition
  2016-09-14 14:13 ` Carlos Sánchez de La Lama
@ 2016-09-24  8:53   ` Ludovic Courtès
  2016-09-26 11:08     ` Carlos Sánchez de La Lama
  0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2016-09-24  8:53 UTC (permalink / raw)
  To: Carlos Sánchez de La Lama; +Cc: 24346

Hi Carlos!

csanchezdll@gmail.com (Carlos Sánchez de La Lama) skribis:

> Rebasing to current master (76f31f0b), as new file-append made merge
> non-trivial.
>
> * guix/scripts/system.scm (previous-grub-entries): resolve initrd
>   symlink for old entries (on the host).
> * gnu/system.scm (operating-system-grub.cfg): resolve initrd symlink for
>   current system (on the container).
> * gnu/system/grub.scm: strip mount-point from GRUB filenames.

I ended up pushing a slightly modified version of this patch as
0f65f54ebd76324653fd5506a7dab42ee44d9255.

Essentially, I was dissatisfied with those ‘readlink’ calls, which do
I/O and could always fail, so I changed them to use the procedures we
have that return the store file name of the initrd.

> +(define (strip-mount-point fs file)

I added a docstring here.

> +  (let ((mount-point (file-system-mount-point fs)))
> +    (if (string=? mount-point "/")
> +	file
> +	#~(if (string-prefix? #$mount-point #$file)
> +	      (substring #$file (string-length #$mount-point))
> +	      #$file))))

I introduced a ‘file’ variable in the staged code, to avoid having that
(string-append …) expression several times in the output.

I think that’s it!

I verified the grub.cfg that ‘guix system reconfigure’ generates and it
seems to work fine.  If you can confirm that it still works for you,
please say so to 24346-done@debbugs.gnu.org.  :-)

Thanks a lot for addressing this bug!

Ludo’.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#24346: [PATCH] Generate grub.cfg with correct paths when store is not in root partition
  2016-09-24  8:53   ` Ludovic Courtès
@ 2016-09-26 11:08     ` Carlos Sánchez de La Lama
  2016-09-26 15:53       ` Ludovic Courtès
  0 siblings, 1 reply; 6+ messages in thread
From: Carlos Sánchez de La Lama @ 2016-09-26 11:08 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 24346

Hi Ludo,

> I ended up pushing a slightly modified version of this patch as
> 0f65f54ebd76324653fd5506a7dab42ee44d9255.

Great! Your version looks definitely better :-)

> I verified the grub.cfg that ‘guix system reconfigure’ generates and it
> seems to work fine.  If you can confirm that it still works for you,
> please say so to 24346-done@debbugs.gnu.org.  :-)

Works on my system which has store on a separate partition. I have
checked grub module and image gets correctly found as well as kernel and
initrd.

Thanks!

Carlos

^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#24346: [PATCH] Generate grub.cfg with correct paths when store is not in root partition
  2016-09-26 11:08     ` Carlos Sánchez de La Lama
@ 2016-09-26 15:53       ` Ludovic Courtès
  0 siblings, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2016-09-26 15:53 UTC (permalink / raw)
  To: Carlos Sánchez de La Lama; +Cc: 24346-done

csanchezdll@gmail.com (Carlos Sánchez de La Lama) skribis:

>> I ended up pushing a slightly modified version of this patch as
>> 0f65f54ebd76324653fd5506a7dab42ee44d9255.
>
> Great! Your version looks definitely better :-)
>
>> I verified the grub.cfg that ‘guix system reconfigure’ generates and it
>> seems to work fine.  If you can confirm that it still works for you,
>> please say so to 24346-done@debbugs.gnu.org.  :-)
>
> Works on my system which has store on a separate partition. I have
> checked grub module and image gets correctly found as well as kernel and
> initrd.

Great!

Ludo’.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-09-26 15:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-31 21:17 bug#24346: GuixSD: grub.cfg does not support separate /gnu partition Ludovic Courtès
2016-09-13 11:23 ` bug#24346: [PATCH] Generate grub.cfg with correct paths when store is not in root partition Carlos Sánchez de La Lama
     [not found] <7tlgyw11k3.fsf@kdpof.com>
2016-09-14 14:13 ` Carlos Sánchez de La Lama
2016-09-24  8:53   ` Ludovic Courtès
2016-09-26 11:08     ` Carlos Sánchez de La Lama
2016-09-26 15:53       ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).