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

  • Done
  • quality assurance status badge
Details
4 participants
  • Chris Marusich
  • Ludovic Courtès
  • Tomáš ?ech
  • Tomáš ?ech
Owner
unassigned
Submitted by
Tomáš ?ech
Severity
normal
T
T
Tomáš ?ech wrote on 9 Mar 2015 21:34
fix interpretation of grub configuration
(address . bug-guix@gnu.org)
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-----


L
L
Ludovic Courtès wrote on 23 Feb 2016 14:39
retitle
(address . request@debbugs.gnu.org)
87fuwjk0lf.fsf@gnu.org
retitle 20067 File name of initrd and kernel image in 'menu-entry' should not be forced
thanks
L
L
Ludovic Courtès wrote on 23 Feb 2016 14:45
Re: bug#20067: fix interpretation of grub configuration
(address . 20067@debbugs.gnu.org)
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’.
L
L
Ludovic Courtès wrote on 3 Aug 2016 18:52
Re: [PATCH] system: grub: Introduce foreign-menu-entry.
(name . Tomáš ?ech)(address . sleep_walker@gnu.org)
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
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’.
C
C
Chris Marusich wrote on 4 Aug 2016 07:38
Re: bug#20067: [PATCH] system: grub: Introduce foreign-menu-entry.
(name . Ludovic Courtès)(address . ludo@gnu.org)
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
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXotTSAAoJEN1AmhXYIkadC84QAL9STzb7OuiCCZQJobm8I64q
qyuVUHGlVuqwD9jNYiMezV1GVxGL3AyUxxH/wFh68nU5+f9598d8ZkKgoHrWw3G9
qOL9wjUtT9wDLFsBk9R+wTc/zy1T7+abG0XcUaLnlZEqgZ/41C41qr4nsm7SVl+K
k7AH6clonaq0a/2F5yNzxz+eIwaaBnG5m1/BN88RvuZxgv1IDoiDytUFtB0EuLTI
S6+dETGZvefsJigUVhhKbacrrckfbnFP/odbSc68NsCWfQPeMwc4N+Ce5Uwb6xPs
OdB67cFVwuf6/P0BcBbalQaXVNQgdyqL2Qpgk1cV3SEYOQbh14HhPXiUDVEmTLBt
WFcVmnV4lTu8KvKQIQmHiHWr/g1C9QXCZH+KRNKAFVp/8BJAnaQ/kkuseaxpS6E0
70DPIgY7KMEtq9AqMLlwZi8aoLYFS1t/Y9oMDXtv9hWVBp8kolNYlaFE+Bxi0Fjw
RY/ymn8Tm0XaSRnGJIufi8/KqBWoOw5oYyKC0wx3ePuwBS5Cceum2iTM5lXIwmHX
FtwxT5DV3kNx9bj/L7JCYAZAsyUJMm/FT9f95xUqHAZlbStjzOrxOZNKE3oPMqh0
14YJ7J5qIgqpCP6udFq0cW02laxdYrauNGe8656009utuUrfDWsReyFGPjyiap8L
SMiVLyJ1cPNjmxhK0P/8
=zEv7
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 10 Sep 2016 00:03
Re: bug#20067: fix interpretation of grub configuration
(address . 20067@debbugs.gnu.org)
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’.
L
L
Ludovic Courtès wrote on 10 Sep 2016 00:03
control message for bug #20067
(address . control@debbugs.gnu.org)
87twdorarb.fsf@gnu.org
tags 20067 fixed
close 20067 0.11.1
T
T
Tomáš ?ech wrote on 14 Sep 2016 18:16
Re: bug#20067: fix interpretation of grub configuration
(address . 20067@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
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
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCAAGBQJX2XfdAAoJEEoj40+gM0NtbAAP/0PBkrVeMBGrUopWmp5mxcTz
3iuz3mpKLjIzGu9XiLr9XymvKoZVvF2N+/Xp6Mqrt6UFsa4z0kA4d6oEq+8kslgL
/RCFi20kvD7lA0MtGJmZl4VX3xiL8aZimfTVgAR2KdSiz4r1XHdnzoNZ68D+qssq
7NWQmlwJ5MIbUame1txbdOdPW6Zb9vikpH/vcFcKt4P96nCyDakOq0h5MlMrq3p3
y/L8vYu3xt2loPHnUUlUUuHr2qIEjbGLv1ePTslFYNKR21wUTXb4R50k8oxW1maY
wu+xB7N0Tj9mrjnrMcAdMRhYWVpvl9UUHtAgKrEeFgtPwyobNJ1RvFpT38LBkLzl
6UC02CN9eYfl0nBlafsOsnTbsM+5Zrx+SMenANPHOZDCRaj3EHcvn5XWjWk2ptnu
dsslrjVyCBffHmvGqgDV4r2wwWBCC5/q9zz5L40AsBzCsM+Si4/YA1BfaKrgP6AX
BNb8wgZZig0jT7wx37geVKVFOVo+NwID6lBU8QDe6eKN/IlE+8UhPL9zao1gusJo
xDUGjoBQ5FgY7KZoGXDsHcioyMMs/uSWSGZKu+rq4lHgnosX8ZYlVfe+nJ09wn4R
VuXYbKYRPIEPVemqbnP85vX7rDeU3Nj9D6gtxFmjJUiWtjU6v1NzrC3bkpPWNzEg
CoGp9sqlENaRLawtHvCy
=O2b3
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 25 Sep 2016 17:56
(name . Tomáš ?ech)(address . tcech@suse.com)(address . 20067@debbugs.gnu.org)
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’.
?