* [PATCH] system: grub: Introduce foreign-menu-entry.
@ 2016-08-03 6:42 Tomáš Čech
2016-08-03 8:48 ` Chris Marusich
2016-08-03 16:52 ` Ludovic Courtès
0 siblings, 2 replies; 6+ messages in thread
From: Tomáš Čech @ 2016-08-03 6:42 UTC (permalink / raw)
To: guix-devel
* gnu/system/grub(foreign-menu-entry): New record type.
menu-entry type is suitable for kernel and initrd from GuixSD as it is looking
for menu-entry-linux/bzImage for kernel in every case which makes pasing any
other form impossible.
foreign-menu-entry is very similar but is intended for entering other
distributions so it doesn't limit the name and allows kernel and initrd to be
placed in different device (in Grub syntax).
---
gnu/system/grub.scm | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/gnu/system/grub.scm b/gnu/system/grub.scm
index 45b46ca..9b60dc8 100644
--- a/gnu/system/grub.scm
+++ b/gnu/system/grub.scm
@@ -54,6 +54,9 @@
menu-entry
menu-entry?
+ foreign-menu-entry
+ foreign-menu-entry?
+
grub-configuration-file))
;;; Commentary:
@@ -116,6 +119,16 @@
(default '())) ; list of string-valued gexps
(initrd menu-entry-initrd)) ; file name of the initrd as a gexp
+(define-record-type* <foreign-menu-entry>
+ foreign-menu-entry make-foreign-menu-entry
+ foreign-menu-entry?
+ (label foreign-menu-entry-label)
+ (device foreign-menu-entry-device (default ""))
+ (linux foreign-menu-entry-linux)
+ (linux-arguments foreign-menu-entry-linux-arguments
+ (default '())) ; list of string-valued gexps
+ (initrd foreign-menu-entry-initrd)) ; file name of the initrd as a gexp
+
\f
;;;
;;; Background image & themes.
@@ -264,7 +277,16 @@ corresponding to old generations of the system."
#~(string-append #$linux "/"
#$linux-image-name))
#$linux #$linux-image-name (string-join (list #$@arguments))
- #$initrd))))
+ #$initrd))
+ (($ <foreign-menu-entry> label device linux arguments initrd)
+ #~(format port "menuentry ~s {
+ linux ~a ~a
+ initrd ~a
+}~%"
+ #$label
+ (string-append #$device #$linux)
+ (string-join (list #$@arguments))
+ (string-append #$device #$initrd)))))
(mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
(define builder
--
2.9.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] system: grub: Introduce foreign-menu-entry.
2016-08-03 6:42 [PATCH] system: grub: Introduce foreign-menu-entry Tomáš Čech
@ 2016-08-03 8:48 ` Chris Marusich
2016-08-03 9:05 ` Tomáš Čech
2016-08-03 16:52 ` Ludovic Courtès
1 sibling, 1 reply; 6+ messages in thread
From: Chris Marusich @ 2016-08-03 8:48 UTC (permalink / raw)
To: Tomáš Čech; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 2712 bytes --]
Hi Tomáš,
Tomáš Čech <sleep_walker@gnu.org> writes:
> foreign-menu-entry is very similar but is intended for entering other
> distributions so it doesn't limit the name and allows kernel and initrd to be
> placed in different device (in Grub syntax).
What is the use case which motivates this change? How do you intend to
use this after making the proposed changes?
> +(define-record-type* <foreign-menu-entry>
> + foreign-menu-entry make-foreign-menu-entry
> + foreign-menu-entry?
> + (label foreign-menu-entry-label)
> + (device foreign-menu-entry-device (default ""))
> + (linux foreign-menu-entry-linux)
> + (linux-arguments foreign-menu-entry-linux-arguments
> + (default '())) ; list of string-valued gexps
> + (initrd foreign-menu-entry-initrd)) ; file name of the initrd as a gexp
> +
This is identical to the existing <menu-entry> record type; however, it
adds the "device" parameter:
--8<---------------cut here---------------start------------->8---
(define-record-type* <menu-entry>
menu-entry make-menu-entry
menu-entry?
(label menu-entry-label)
(linux menu-entry-linux)
(linux-arguments menu-entry-linux-arguments
(default '())) ; list of string-valued gexps
(initrd menu-entry-initrd)) ; file name of the initrd as a gexp
--8<---------------cut here---------------end--------------->8---
Would it be possible to accomplish the same thing by adding a "device"
parameter to the existing <menu-entry> record type? That way, we
wouldn't have to add a new record type which seems largely redundant.
> #~(string-append #$linux "/"
> #$linux-image-name))
> #$linux #$linux-image-name (string-join (list #$@arguments))
> - #$initrd))))
> + #$initrd))
> + (($ <foreign-menu-entry> label device linux arguments initrd)
> + #~(format port "menuentry ~s {
> + linux ~a ~a
> + initrd ~a
> +}~%"
> + #$label
> + (string-append #$device #$linux)
Don't we need to insert a separator of some kind between #$device and
#$linux? Also, don't we need to include the linux image name (e.g.,
"bzImage")?
> + (string-join (list #$@arguments))
> + (string-append #$device #$initrd)))))
>
> (mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
> (define builder
Isn't a separator of some kind needed between #$device and #$initrd?
--
Chris
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] system: grub: Introduce foreign-menu-entry.
2016-08-03 8:48 ` Chris Marusich
@ 2016-08-03 9:05 ` Tomáš Čech
2016-08-04 6:43 ` Chris Marusich
0 siblings, 1 reply; 6+ messages in thread
From: Tomáš Čech @ 2016-08-03 9:05 UTC (permalink / raw)
To: Chris Marusich; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 4495 bytes --]
On Wed, Aug 03, 2016 at 01:48:36AM -0700, Chris Marusich wrote:
>Hi Tomáš,
>
>Tomáš Čech <sleep_walker@gnu.org> writes:
>
>> foreign-menu-entry is very similar but is intended for entering other
>> distributions so it doesn't limit the name and allows kernel and initrd to be
>> placed in different device (in Grub syntax).
>
>What is the use case which motivates this change? How do you intend to
>use this after making the proposed changes?
I'd like to store configuration of other linux distribution and so far
I haven't found how to override hardcoded bzImage suffix.
With this change I can have configuration:
(bootloader (grub-configuration
(device "/dev/sda")
(menu-entries
(list
(foreign-menu-entry
(label "openSUSE")
(device "(hd0,1)")
(linux "/vmlinuz")
(linux-arguments (list
"root=/dev/venom/opensuse"
"init=/usr/lib/systemd/systemd"))
(initrd "/initrd")
)))))
which will transform to:
menuentry "openSUSE" {
linux (hd0,1)/vmlinuz root=/dev/venom/opensuse init=/usr/lib/systemd/systemd
initrd (hd0,1)/initrd
}
I asked on IRC how can I achieve adding such entry but never got answer. So I made a patch.
>
>> +(define-record-type* <foreign-menu-entry>
>> + foreign-menu-entry make-foreign-menu-entry
>> + foreign-menu-entry?
>> + (label foreign-menu-entry-label)
>> + (device foreign-menu-entry-device (default ""))
>> + (linux foreign-menu-entry-linux)
>> + (linux-arguments foreign-menu-entry-linux-arguments
>> + (default '())) ; list of string-valued gexps
>> + (initrd foreign-menu-entry-initrd)) ; file name of the initrd as a gexp
>> +
>
>This is identical to the existing <menu-entry> record type; however, it
>adds the "device" parameter:
>
>--8<---------------cut here---------------start------------->8---
>
>(define-record-type* <menu-entry>
> menu-entry make-menu-entry
> menu-entry?
> (label menu-entry-label)
> (linux menu-entry-linux)
> (linux-arguments menu-entry-linux-arguments
> (default '())) ; list of string-valued gexps
> (initrd menu-entry-initrd)) ; file name of the initrd as a gexp
>--8<---------------cut here---------------end--------------->8---
Your observation is correct. My intention was to introduce new record
type to distinguish between menu-entry and foreign-menu-entry later in
the code. Different type, different interpretation.
>Would it be possible to accomplish the same thing by adding a "device"
>parameter to the existing <menu-entry> record type? That way, we
>wouldn't have to add a new record type which seems largely redundant.
>
>> #~(string-append #$linux "/"
>> #$linux-image-name))
>> #$linux #$linux-image-name (string-join (list #$@arguments))
>> - #$initrd))))
>> + #$initrd))
>> + (($ <foreign-menu-entry> label device linux arguments initrd)
>> + #~(format port "menuentry ~s {
>> + linux ~a ~a
>> + initrd ~a
>> +}~%"
>> + #$label
>> + (string-append #$device #$linux)
>
>Don't we need to insert a separator of some kind between #$device and
>#$linux? Also, don't we need to include the linux image name (e.g.,
>"bzImage")?
That is the point. User can specify whatever suits him.
For the case with separate boot partition it can look like
/vmlinuz-4.3.3-5-default
for the case with boot directory on the same partition as different
linux distribution it would look like
/boot/vmlinuz-4.3.3-5-default
Note that installed kernel in other distribution has never name
'bzImage'.
>
>> + (string-join (list #$@arguments))
>> + (string-append #$device #$initrd)))))
>>
>> (mlet %store-monad ((sugar (eye-candy config store-fs system #~port)))
>> (define builder
>
>Isn't a separator of some kind needed between #$device and #$initrd?
This kernel or partition with it is not searched, absolute path is
expected, so no.
Code was using match-lambda so defining new type to distinguish the
cases seemed to me natural. Again, I'm new to Guile and I'd like to
see how to do that right. This worked for me as first solution.
Thanks for your review.
S_W
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] system: grub: Introduce foreign-menu-entry.
2016-08-03 6:42 [PATCH] system: grub: Introduce foreign-menu-entry Tomáš Čech
2016-08-03 8:48 ` Chris Marusich
@ 2016-08-03 16:52 ` Ludovic Courtès
2016-08-04 5:38 ` bug#20067: " Chris Marusich
1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2016-08-03 16:52 UTC (permalink / raw)
To: Tomáš Čech; +Cc: guix-devel, 20067
[-- Attachment #1: Type: text/plain, Size: 975 bytes --]
Hi!
Tomáš Čech <sleep_walker@gnu.org> skribis:
> * gnu/system/grub(foreign-menu-entry): New record type.
>
> menu-entry type is suitable for kernel and initrd from GuixSD as it is looking
> for menu-entry-linux/bzImage for kernel in every case which makes pasing any
> other form impossible.
AIUI, this is a followup to <http://bugs.gnu.org/20067>, and it’s
admittedly a shame that this isn’t fixed!
I still think that the approach proposed at
<http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20067#10> is more
appropriate; ‘menu-entry’ would always work, no duplication would be
necessary.
As a stop-gap measure, I would prefer to (1) allow:
(menu-entry
;; …
(linux #~(string-append #$kernel "/bzImage")))
(2) remove the “/bzImage” assumption and use the above idiom everywhere
in the current code, and (3) and have a hack along these lines to
correctly interpret (string-append …) in the ‘parameters’ file:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 586 bytes --]
diff --git a/gnu/system.scm b/gnu/system.scm
index d6bf6c4..467d907 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -766,7 +766,11 @@ this file is the reconstruction of GRUB menu entries for old configurations."
(boot-parameters
(label label)
(root-device root)
- (kernel linux)
+ (kernel (match linux
+ (('string-append (? string? strings) ...)
+ (string-concatenate strings))
+ (_
+ linux)))
(kernel-arguments
(match (assq 'kernel-arguments rest)
((_ args) args)
[-- Attachment #3: Type: text/plain, Size: 34 bytes --]
Thoughts?
Thanks,
Ludo’.
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: bug#20067: [PATCH] system: grub: Introduce foreign-menu-entry.
2016-08-03 16:52 ` Ludovic Courtès
@ 2016-08-04 5:38 ` Chris Marusich
0 siblings, 0 replies; 6+ messages in thread
From: Chris Marusich @ 2016-08-04 5:38 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel, 20067
[-- Attachment #1: Type: text/plain, Size: 1335 bytes --]
ludo@gnu.org (Ludovic Courtès) writes:
> I still think that the approach proposed at
> <http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20067#10> is more
> appropriate; ‘menu-entry’ would always work, no duplication would be
> necessary.
>
> As a stop-gap measure, I would prefer to (1) allow:
>
> (menu-entry
> ;; …
> (linux #~(string-append #$kernel "/bzImage")))
>
> (2) remove the “/bzImage” assumption and use the above idiom everywhere
> in the current code, and (3) and have a hack along these lines to
> correctly interpret (string-append …) in the ‘parameters’ file:
>
>
> diff --git a/gnu/system.scm b/gnu/system.scm
> index d6bf6c4..467d907 100644
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -766,7 +766,11 @@ this file is the reconstruction of GRUB menu entries for old configurations."
> (boot-parameters
> (label label)
> (root-device root)
> - (kernel linux)
> + (kernel (match linux
> + (('string-append (? string? strings) ...)
> + (string-concatenate strings))
> + (_
> + linux)))
> (kernel-arguments
> (match (assq 'kernel-arguments rest)
> ((_ args) args)
>
>
> Thoughts?
Yes, that approach seems better to me.
--
Chris
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] system: grub: Introduce foreign-menu-entry.
2016-08-03 9:05 ` Tomáš Čech
@ 2016-08-04 6:43 ` Chris Marusich
0 siblings, 0 replies; 6+ messages in thread
From: Chris Marusich @ 2016-08-04 6:43 UTC (permalink / raw)
To: guix-devel
[-- Attachment #1: Type: text/plain, Size: 1631 bytes --]
Tomáš Čech <tcech@suse.com> writes:
> I'd like to store configuration of other linux distribution and so far
> I haven't found how to override hardcoded bzImage suffix.
>
> With this change I can have configuration:
>
> (bootloader (grub-configuration
> (device "/dev/sda")
> (menu-entries
> (list
> (foreign-menu-entry
> (label "openSUSE")
> (device "(hd0,1)")
> (linux "/vmlinuz")
> (linux-arguments (list
> "root=/dev/venom/opensuse"
> "init=/usr/lib/systemd/systemd"))
> (initrd "/initrd")
> )))))
>
> which will transform to:
>
> menuentry "openSUSE" {
> linux (hd0,1)/vmlinuz root=/dev/venom/opensuse init=/usr/lib/systemd/systemd
> initrd (hd0,1)/initrd
> }
I see! Yes, I agree it would be nice to be able to do that.
> My intention was to introduce new record type to distinguish between
> menu-entry and foreign-menu-entry later in the code. Different type,
> different interpretation.
I see what you're saying, but I think the alternative which Ludo
proposed in the following bug report is cleaner because it re-uses the
existing record type:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20067
What do you think?
> Code was using match-lambda so defining new type to distinguish the
> cases seemed to me natural. Again, I'm new to Guile and I'd like to
> see how to do that right. This worked for me as first solution.
That's totally understandable. I'm new to Guile, also, so I might not
always be right, either. Thank you for taking the time to submit a
patch to improve the project!
--
Chris
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-04 6:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-03 6:42 [PATCH] system: grub: Introduce foreign-menu-entry Tomáš Čech
2016-08-03 8:48 ` Chris Marusich
2016-08-03 9:05 ` Tomáš Čech
2016-08-04 6:43 ` Chris Marusich
2016-08-03 16:52 ` Ludovic Courtès
2016-08-04 5:38 ` bug#20067: " Chris Marusich
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).