* [PATCH] Generate grub.cfg with correct paths when store is not in root partition
@ 2016-09-13 11:23 Carlos Sánchez de La Lama
2016-09-14 14:13 ` bug#24346: " Carlos Sánchez de La Lama
0 siblings, 1 reply; 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
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
2016-09-13 11:23 [PATCH] Generate grub.cfg with correct paths when store is not in root partition Carlos Sánchez de La Lama
@ 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 ` bug#24346: " 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-09-13 11:23 [PATCH] Generate grub.cfg with correct paths when store is not in root partition Carlos Sánchez de La Lama
2016-09-14 14:13 ` bug#24346: " 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
-- strict thread matches above, loose matches on Subject: below --
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
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.