File name of initrd and kernel image in 'menu-entry' should not be forced

Done

Details

4 participants

Chris Marusich
Ludovic Courtès
Tomáš ?ech
Tomáš ?ech

Owner: unassigned

Submitted by: Tomáš ?ech

Severity: normal

Tomáš ?ech wrote on 9 Mar 2015 21:34

fix interpretation of grub configuration

Recipients:(address . bug-guix@gnu.org)

Message-ID:20150309203443.GA3438@venom

Grub configuration interpretes `linux' as directory where is located

bzImage. If I enter file name instead, result configuration will be

wrong.

Example of system configuration:

(bootloader (grub-configuration

(device "/dev/sda")

(menu-entries

(list

(menu-entry

(label "Gentoo")

(linux "/vmlinuz-gentoo") ; vmlinuz-gentoo is file

(linux-arguments (list

"root=/dev/venom/gentoo"

"init=/usr/lib/systemd/systemd"))

(initrd "/initramfs-gentoo")

)))))

Result part of grub.cfg:

menuentry "Gentoo" {

# Set 'root' to the partition that contains the kernel.

search --file --set /vmlinuz-gentoo/bzImage

linux /vmlinuz-gentoo/bzImage root=/dev/venom/gentoo init=/usr/lib/systemd/systemd

initrd /initramfs-gentoo

}

It would be nice if the the string would be simply copied into

grub.cfg, so I could use even `(hd0,msdos1)/vmlinuz'.

-----BEGIN PGP SIGNATURE-----

Version: GnuPG v2

iEYEARECAAYFAlT+A+MACgkQ37XrCapiVCNyJwCggCY6aE0uJGYSks1+lFYZWxMm

z2UAoKVeetSSuCplssvFRBY9pzQrgQZH

=z1Q/

-----END PGP SIGNATURE-----

Ludovic Courtès wrote on 23 Feb 2016 14:39

retitle

Recipients:(address . request@debbugs.gnu.org)

Message-ID:87fuwjk0lf.fsf@gnu.org

retitle 20067 File name of initrd and kernel image in 'menu-entry' should not be forced

thanks

Ludovic Courtès wrote on 23 Feb 2016 14:45

Re: bug#20067: fix interpretation of grub configuration

Recipients:(address . 20067@debbugs.gnu.org)

Message-ID:874mczk0an.fsf@gnu.org

Tomáš ?ech <sleep_walker@suse.cz> skribis:

Toggle quote (4 lines)

> Grub configuration interpretes `linux' as directory where is located

> bzImage. If I enter file name instead, result configuration will be

> wrong.

The solution will be to not automatically append “/bzImage” (and

likewise for the initrd.)

We could change places where ‘menu-entry’ is instantiated to:

#~(string-append #$kernel "/bzImage")

However, there’s the problem that the image name appears in the

‘parameters’ file of the system (as seen in the output of ‘guix system

build foo.scm’), where it is unevaluated. If we use ‘string-append’ as

above, a raw (string-append …) sexp will appear in there, which is not

nice.

To address this, an idea is to add “expanders” for gexps: gexps already

have “compilers”, and expanders would be similar except that they would

produce something possibly different from just the derivation’s output

file name. For instance, we could write:

(file-append kernel "/bzImage")

and that would expand directly to:

"/gnu/store/…/bzImage"

Ludo’.

Ludovic Courtès wrote on 3 Aug 2016 18:52

Re: [PATCH] system: grub: Introduce foreign-menu-entry.

Recipients:(name . Tomáš ?ech)(address . sleep_walker@gnu.org)

Message-ID:87lh0d23sf.fsf@gnu.org

Hi!

Tomáš ?ech <sleep_walker@gnu.org> skribis:

Toggle quote (6 lines)

> * 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:

Toggle diff (17 lines)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?

Thanks,

Ludo’.

Chris Marusich wrote on 4 Aug 2016 07:38

Re: bug#20067: [PATCH] system: grub: Introduce foreign-menu-entry.

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:871t25jdpr.fsf@gmail.com

ludo@gnu.org (Ludovic Courtès) writes:

Toggle quote (37 lines)> 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

Ludovic Courtès wrote on 10 Sep 2016 00:03

Re: bug#20067: fix interpretation of grub configuration

Recipients:(address . 20067@debbugs.gnu.org)

Message-ID:87vay4rarp.fsf@gnu.org

Good news!

ludo@gnu.org (Ludovic Courtès) skribis:

Toggle quote (30 lines)> Tomáš ?ech <sleep_walker@suse.cz> skribis:
>
>> Grub configuration interpretes `linux' as directory where is located
>> bzImage. If I enter file name instead, result configuration will be
>> wrong.
>
> The solution will be to not automatically append “/bzImage” (and
> likewise for the initrd.)
>
> We could change places where ‘menu-entry’ is instantiated to:
>
>   #~(string-append #$kernel "/bzImage")
>
> However, there’s the problem that the image name appears in the
> ‘parameters’ file of the system (as seen in the output of ‘guix system
> build foo.scm’), where it is unevaluated.  If we use ‘string-append’ as
> above, a raw (string-append …) sexp will appear in there, which is not
> nice.
>
> To address this, an idea is to add “expanders” for gexps: gexps already
> have “compilers”, and expanders would be similar except that they would
> produce something possibly different from just the derivation’s output
> file name.  For instance, we could write:
>
>   (file-append kernel "/bzImage")
>
> and that would expand directly to:
>
>   "/gnu/store/…/bzImage"

AFAICS this is finally fixed!

expanders in commit ebdfd776f4504c456d383ee8afa59fc6fdfc6756

‘file-append’ in commit a9e5e92f940381e3a4ee828c6d8ff22a73067e17

kernel file name in commit 44d5f54e31039d78f156bd9562dca293124eaa76

Please let me know how it goes! In particular, does it work for the

dual-boot scenario you were interested in?

Thanks,

Ludo’.

Ludovic Courtès wrote on 10 Sep 2016 00:03

control message for bug #20067

Recipients:(address . control@debbugs.gnu.org)

Message-ID:87twdorarb.fsf@gnu.org

tags 20067 fixed

close 20067 0.11.1

Tomáš ?ech wrote on 14 Sep 2016 18:16

Re: bug#20067: fix interpretation of grub configuration

Recipients:(address . 20067@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID:20160914161558.yqhld3ragclyra2h@venom

On Sat, Sep 10, 2016 at 12:03:38AM +0200, Ludovic Courtès wrote:

Toggle quote (2 lines)

>Good news!

Good news indeed!

Toggle quote (42 lines)>
>ludo@gnu.org (Ludovic Courtès) skribis:
>
>> Tomáš ?ech <sleep_walker@suse.cz> skribis:
>>
>>> Grub configuration interpretes `linux' as directory where is located
>>> bzImage. If I enter file name instead, result configuration will be
>>> wrong.
>>
>> The solution will be to not automatically append “/bzImage” (and
>> likewise for the initrd.)
>>
>> We could change places where ‘menu-entry’ is instantiated to:
>>
>>   #~(string-append #$kernel "/bzImage")
>>
>> However, there’s the problem that the image name appears in the
>> ‘parameters’ file of the system (as seen in the output of ‘guix system
>> build foo.scm’), where it is unevaluated.  If we use ‘string-append’ as
>> above, a raw (string-append …) sexp will appear in there, which is not
>> nice.
>>
>> To address this, an idea is to add “expanders” for gexps: gexps already
>> have “compilers”, and expanders would be similar except that they would
>> produce something possibly different from just the derivation’s output
>> file name.  For instance, we could write:
>>
>>   (file-append kernel "/bzImage")
>>
>> and that would expand directly to:
>>
>>   "/gnu/store/…/bzImage"
>
>AFAICS this is finally fixed!
>
>  expanders in commit ebdfd776f4504c456d383ee8afa59fc6fdfc6756
>  ‘file-append’ in commit a9e5e92f940381e3a4ee828c6d8ff22a73067e17
>  kernel file name in commit 44d5f54e31039d78f156bd9562dca293124eaa76
>
>Please let me know how it goes!  In particular, does it work for the
>dual-boot scenario you were interested in?

It is almost perfect.

Configuration excerpt...

(bootloader (grub-configuration

(device "/dev/sda")

(menu-entries

(list

(menu-entry

(label "openSUSE")

(linux "(hd0,msdos1)/vmlinuz")

(linux-arguments (list

"root=/dev/venom/opensuse"

"init=/usr/lib/systemd/systemd"))

(initrd "(hd0,msdos1)/initrd"))))))

...transforms into

menuentry "openSUSE" {

search --file --set (hd0,msdos1)/vmlinuz

linux (hd0,msdos1)/vmlinuz root=/dev/venom/opensuse init=/usr/lib/systemd/systemd

initrd (hd0,msdos1)/initrd

}

I think that if linux contains prefix '(.*)/', there should be no

search for kernel.

Thank you very much for fixing this bug (especially when I wasn't

able). I believe that fixing this bug is big step in more friendly

behavior to other OS.

Best regards,

S_W

Ludovic Courtès wrote on 25 Sep 2016 17:56

Recipients:(name . Tomáš ?ech)(address . tcech@suse.com)(address . 20067@debbugs.gnu.org)

Message-ID:87eg48otv8.fsf@gnu.org

Hi,

Tomáš ?ech <tcech@suse.com> skribis:

Toggle quote (26 lines)> Configuration excerpt...
>
> (bootloader (grub-configuration
>              (device "/dev/sda")
>              (menu-entries
>               (list
>                (menu-entry
>                 (label "openSUSE")
>                 (linux "(hd0,msdos1)/vmlinuz")
>                 (linux-arguments (list
>                                   "root=/dev/venom/opensuse"
>                                   "init=/usr/lib/systemd/systemd"))
>                 (initrd "(hd0,msdos1)/initrd"))))))
>
>
> ...transforms into
>
> menuentry "openSUSE" {
>   search --file --set (hd0,msdos1)/vmlinuz
>   linux (hd0,msdos1)/vmlinuz root=/dev/venom/opensuse init=/usr/lib/systemd/systemd
>   initrd (hd0,msdos1)/initrd
> }
>
> I think that if linux contains prefix '(.*)/', there should be no
> search for kernel.

Oh, right. I believe this is fixed by

5babe521c8adc722c2411b255cbeeef308339d06.

Please let me know if anything’s missing now.

Thanks!

Ludo’.

Your comment

This issue is archived.

To comment on this conversation send an email to 20067@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it

mumi current 20067

Then, you may apply the latest patchset in this issue (with sign off)

mumi am -- -s

Or, compose a reply to this issue

mumi compose

Or, send patches to this issue

mumi send-email *.patch

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date