guix system delete-generations removes custom boot menu entries

DoneSubmitted by Jesse Gibbons.
Details
4 participants
  • Danny Milosavljevic
  • Jesse Gibbons
  • Ludovic Courtès
  • Jakob L. Kreuze
Owner
unassigned
Severity
important
J
J
Jesse Gibbons wrote on 31 Jul 2019 17:48
(address . bug-guix@gnu.org)
20190731094857.28829b11@gmail.com
I dual-booted Guix with another gnu/linux-libre distro.
My configuration includes the other distro in the grub menu. When I run
"sudo guix system delete-generations" the changes to the grub menu drop
the other distro with the older system generations of guix.

My current work-around for this is to run "guix system reconfigure ..."
which includes the boot menu entries specified in the configuration.
J
J
Jakob L. Kreuze wrote on 5 Aug 2019 18:05
(name . Jesse Gibbons)(address . jgibbons2357@gmail.com)(address . 36876@debbugs.gnu.org)
8736ifzjfe.fsf@sdf.lonestar.org
Hi Jesse,

Jesse Gibbons <jgibbons2357@gmail.com> writes:

Toggle quote (8 lines)
> I dual-booted Guix with another gnu/linux-libre distro.
> My configuration includes the other distro in the grub menu. When I run
> "sudo guix system delete-generations" the changes to the grub menu drop
> the other distro with the older system generations of guix.
>
> My current work-around for this is to run "guix system reconfigure ..."
> which includes the boot menu entries specified in the configuration.

Thanks for reporting this; it's a rather serious issue. The problem lies
in the 'reinstall-bootloader' procedure. Chiefly, it uses the default
bootloader configuration for whatever it can find using
'lookup-bootloader-by-name' and generates menu entries for the
generations reachable from '%system-profile', which is quite a bit
different from how 'guix system reconfigure' produces the bootloader
configuration. It really isn't ideal. To quote a comment in
'system.scm': "[i]t will be enough to allow the system to boot."

I don't think this should be _too_ hard to fix. To me, parsing the
installed Grub configuration to get existing menu entries seems like a
logical step forward.

Thoughts from anyone else?

Regards,
Jakob
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEa1VJLOiXAjQ2BGSm9Qb9Fp2P2VoFAl1IU+YACgkQ9Qb9Fp2P
2Vp/xhAAk/8YsIdUIUoAXsarGj/LJe4AuVhMJKR2uJGsgVUFbB1F825rSj6PTk7L
Oa2l5BvRq3fuULT46zEzgCaE4LHu2xwe0FFjhKGPrsAx9I6AMHp1hItRwj3u57CC
oFQBDOAKOQBqQT8h9ynJ45KdJ+dkzwmOYVXIp5/17FfjIefZOL0XTE7Bmr3ycd5y
YW/n+B0LCrMPtdwVmFy999o0in/Z5BPh2RLLo/NW36viT+kafqSBAk9YymYqEbe4
H/uF7n5nb9Z/zWOlwj3gpDJ/TGU797TSVYSwZOFkO+dekYsw7PlesqQApXX3qfGe
VgRSwSn22NwzwjyCgLGtvjgxEloV8pihodceznRZXi25Ld1N9DUQndN6Ryk65JiO
RzLVGn/vfCq8yR5OOiWcQApd7Xz8XPXxe9fX9mkG/KJW6U4g8BjfjjwAvELsrKmg
+6OFg0bUdn+Gr9OVZKAEVlgtThhxdGIpVFxXChBJdRK/K/QyZeY2DQarRhgkm3h5
vzKwBncEOOj+sQfDs09nT/1Bz/DjpQJpLT+FTsqRVGzfknzYVamKiKamm59RdxJS
jI9EUTFLT/oqk254utSc8lhXBhemynKN+GiBV6uW45BqYKMtUgdRxFSyRn8XRThB
BBebjQQhxGk9gliCsZMIJhWs26BsGrgEUwfAciJJfNypZWUoHbk=
=ZPUK
-----END PGP SIGNATURE-----

J
J
Jesse Gibbons wrote on 6 Aug 2019 05:12
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)(address . 36876@debbugs.gnu.org)
e5d7a9a83b44c739794a597411ae3ee090bac61d.camel@gmail.com
On Mon, 2019-08-05 at 12:05 -0400, Jakob L. Kreuze wrote:
Toggle quote (12 lines)
> ...
>
> I don't think this should be _too_ hard to fix. To me, parsing the
> installed Grub configuration to get existing menu entries seems like
> a
> logical step forward.
>
> Thoughts from anyone else?
>
> Regards,
> Jakob

Alternatively, we could save in the store a derivation for the the grub
config generated from the bootloader of the configuration. When the
user calls "guix system delete-generations", the derivation can be run,
and the remaining system generations (if any) can be appended in a menu
like when the user calls "guix system reconfigure". (Although it does
not work for me right now, I'm assuming "guix system delete-generations
2m" as described in the manual will be implemented in the future.)

The immediate downsides I see to my solution:

- It would take space in the store per generation, which can add up if
the user does not often call "guix system delete-generations" and calls
"guix system reconfigure" on a healthy basis. The user could just be
reminded to call "guix system delete-generations" occasionally, and any
official service that automatically updates the system via "guix
system reconfigure" can (and considering how large a generation with a
lot of updated system packages can get, probably should) also be
configured to call "guix system delete-generations".

- If someone hand-edits the grub config the changes would be lost. This
is the case as it is right now, and grub options can be edited in the
configuration, so I'm not too concerned about this.

-It would be much simpler to identify menu entries generated by guix
that are no longer in the store and remove them, and remove all empty
submenus. Parsing would make hand-editing grub.cfg more dangerous than
a solution that simply scraps the hand-made changes and rebuilds as I
propose, because the user doing the hand-editing would not necessarily
be aware what patterns the parser checks. It would also be inconsitent:
edits to grub.cfg being scrapped when "guix system reconfigure" is
called, but not when 'guix system delete-generations" is called looks
to me like a good way to introduce a bug to the more adventurous
"Murphy's Law"-type users down the road.


These are just my thoughts. I would love to hear other downsides to my
solution.

--
-Jesse
J
J
Jakob L. Kreuze wrote on 6 Aug 2019 15:22
(name . Jesse Gibbons)(address . jgibbons2357@gmail.com)(address . 36876@debbugs.gnu.org)
87y306xwb0.fsf@sdf.lonestar.org
Jesse Gibbons <jgibbons2357@gmail.com> writes:

Toggle quote (34 lines)
> Alternatively, we could save in the store a derivation for the the grub
> config generated from the bootloader of the configuration. When the
> user calls "guix system delete-generations", the derivation can be run,
> and the remaining system generations (if any) can be appended in a menu
> like when the user calls "guix system reconfigure". (Although it does
> not work for me right now, I'm assuming "guix system delete-generations
> 2m" as described in the manual will be implemented in the future.)
>
> The immediate downsides I see to my solution:
>
> - It would take space in the store per generation, which can add up if
> the user does not often call "guix system delete-generations" and calls
> "guix system reconfigure" on a healthy basis. The user could just be
> reminded to call "guix system delete-generations" occasionally, and any
> official service that automatically updates the system via "guix
> system reconfigure" can (and considering how large a generation with a
> lot of updated system packages can get, probably should) also be
> configured to call "guix system delete-generations".
>
> - If someone hand-edits the grub config the changes would be lost. This
> is the case as it is right now, and grub options can be edited in the
> configuration, so I'm not too concerned about this.
>
> -It would be much simpler to identify menu entries generated by guix
> that are no longer in the store and remove them, and remove all empty
> submenus. Parsing would make hand-editing grub.cfg more dangerous than
> a solution that simply scraps the hand-made changes and rebuilds as I
> propose, because the user doing the hand-editing would not necessarily
> be aware what patterns the parser checks. It would also be
> inconsitent: edits to grub.cfg being scrapped when "guix system
> reconfigure" is called, but not when 'guix system delete-generations"
> is called looks to me like a good way to introduce a bug to the more
> adventurous "Murphy's Law"-type users down the road.

I haven't tried it yet, but 'menuentry' seems as though it would be a
fairly simple structure to parse.

Toggle quote (3 lines)
> These are just my thoughts. I would love to hear other downsides to my
> solution.

I prefer your suggestion to mine and think that the benefits may
outweigh the costs in this case. This seems to fit into the Guix idea of
purity a bit better than parsing a grub.cfg that may have been
overwritten by another system.

Regards,
Jakob
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEa1VJLOiXAjQ2BGSm9Qb9Fp2P2VoFAl1JfzMACgkQ9Qb9Fp2P
2VrANQ//bnil91UBb5UGNEs2l/MPS2pLXGnnGuTbfFFBsvFDkL3YCnzLnAce1uRV
/rYdU1zGu1HHM3dpN0AGuR5xXX9NKlCZTZcWKFGUBkaS0Tc7r3n7AKFbhTbal2M+
ViGAot6yWuSlghLN2xaBzFR+wqPtghOVAPR5GOZoW6XUpnMSiBtRNRlitR6rettG
xm9clTCOkR8f2Cbrgwn/zq9TjXFlcQWhzDB8GSkFgzB+cgJhiuCXsNvYdXhCzNa0
CNxZRS1rxIaSMztWnicTHghN29AxzYNqJ22p3jk6X3WevNoxC4Yr/vIk5BHY1KLJ
hl63bj4xTtrN9+8vpgE2UoLIGXGquxyLq5KIi08hu8aXa/C11QnPPiNZlirnghl2
EN7V8Psf2qvZYeXKC0+VbUuiZdHCllERA6+PabI+s6v+IwGWlRjQi/7Yk+e55DOW
Y0leADS8HILIK3cMRvIxf/6WGktD/3zxsHMEy7nSePXqyyozfbPhbmAViOTDvh4f
Mp4mjAbOER8RbkAXCP8CMlcvBYAWFxyBNflufCuHYEORe0ByY+zhDn3gzqPkuIoO
8f0W2aq/b4nCPIBMojgkVtR8ST88XTiNLVBA7cbAFbwBSRZASGWvyHliAu5F2goS
XqyRydKPtnTagkkmRadokAEC5AniJKQ3lV5FQp+X7CPr26ToZzI=
=vlA6
-----END PGP SIGNATURE-----

D
D
Danny Milosavljevic wrote on 6 Aug 2019 18:32
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)
20190806183234.5e7714c2@scratchpost.org
Hi Jakob,

keep in mind that switching back to a previous generation could also entail a
switch of bootloader projects. It does you no good to parse the grub config to
find menu entries and then append them to u-boot.

Fundamentally, it would be best if either the bootloader is part of the state
that guix manages (and each time uses), or it isn't; not both.

Current situation:

* If you select a previous generation in the boot menu, then the state that is
selected does NOT include the bootloader (i.e. it doesn't change the bootloader
or the bootloader config).

* If you do "guix system reconfigure", the state that is saved includes only
the bootloader in the boot sector (simplified) but not the bootloader installer
or the bootloader derivation.

* If you do "guix system delete-generations", the state that is restored does
not include the bootloader installer and previous bootloader configuration.

Clearly, it's not nice to have these different things happen.

It would be better if we retained the generation's bootloader installer
and bootloader config and reinstalled it on each of those.

So, I would suggest to retain the following for each system generation:

* The bootloader package derivation
* The bootloader config derivation
* The bootloader installer derivation

And to install the bootloader each time using the latter.

This introduces cycles--not sure whether that's a problem or not.
For the GC I think it's not a problem.
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl1Jq6IACgkQ5xo1VCww
uqX+MAf/SuzaBCxBH2Shxkh3c3JUJYnVPc9oW+MVvCou/sAwjeUkgI9LVz4lgaXA
aLmknvllkHpWWi3sofBXqWQXqHSpu6PlJPMSSOadcPKTYc2J1oK0rMK4TWmLHqZk
eqGdy9pAN06Fr56Vcl/OEqnEwCodFJ/Xv9kxGj5vS+6WtpDbdLDSmvQs2YkPAcc/
KaoNZBwYneMjAJV25EAq0i5jhCh3TmQkibl3FDdt8veURo+oAVf6B1gpXU12XRiS
/PF2x6SERRMObz0jQXyGlzi6anFwXkRj9NKozfoKzwfncbqrTux/3EIDeUTQYvZS
bkZ0ZDqXk6Lu40Byi54Oekn3XGnumg==
=x0pE
-----END PGP SIGNATURE-----


D
D
Danny Milosavljevic wrote on 6 Aug 2019 18:35
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)
20190806183558.7d863a82@scratchpost.org
Addition:

* If you do "guix system switch-generation", the state that is restored does
not include the bootloader installer and previous bootloader configuration.
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl1JrG4ACgkQ5xo1VCww
uqXwuAf/YBhjEfeoVHpKvG3yWhXyuSgXYVA823vL/ovJzOCNrJ0gOwN/oax8zYE4
RENwreE7cp0vvfWR7L5BC7Q3knU6+oXcfjvemLwmzYYE2vBwt7EuxQGy+Y7pyU3U
ei16tooXaQPkU/fDXYLdYxGYAg3qYdpPmSE/nGB33LFTybiLt+Q/aDm5nFn9oWML
XQ/8j+i+3MC1Ympx/jZvH7wPcmZ+XAKI5pf8I5JGpKGL61f8FfZKVeIxV1qBYxgY
A2GrMa6kWXcyYocg4PwN2BpK/Q5fIpJTUHN/nsbkn2Qs9/Yal4fEFIjvluJr5xG/
BHMXMoc0XASl9gEKjqXRwIi0F45gpA==
=2XFY
-----END PGP SIGNATURE-----


J
J
Jakob L. Kreuze wrote on 6 Aug 2019 20:27
(name . Danny Milosavljevic)(address . dannym@scratchpost.org)
87k1bqdu8s.fsf@sdf.lonestar.org
Hi Danny,

Danny Milosavljevic <dannym@scratchpost.org> writes:

Toggle quote (38 lines)
> Hi Jakob,
>
> keep in mind that switching back to a previous generation could also entail a
> switch of bootloader projects. It does you no good to parse the grub config to
> find menu entries and then append them to u-boot.
>
> Fundamentally, it would be best if either the bootloader is part of the state
> that guix manages (and each time uses), or it isn't; not both.
>
> Current situation:
>
> * If you select a previous generation in the boot menu, then the state that is
> selected does NOT include the bootloader (i.e. it doesn't change the bootloader
> or the bootloader config).
>
> * If you do "guix system reconfigure", the state that is saved includes only
> the bootloader in the boot sector (simplified) but not the bootloader installer
> or the bootloader derivation.
>
> * If you do "guix system delete-generations", the state that is restored does
> not include the bootloader installer and previous bootloader configuration.
>
> Clearly, it's not nice to have these different things happen.
>
> It would be better if we retained the generation's bootloader installer
> and bootloader config and reinstalled it on each of those.
>
> So, I would suggest to retain the following for each system generation:
>
> * The bootloader package derivation
> * The bootloader config derivation
> * The bootloader installer derivation
>
> And to install the bootloader each time using the latter.
>
> This introduces cycles--not sure whether that's a problem or not.
> For the GC I think it's not a problem.

Thank you for your comments -- after reading this and Jesse's
suggestion, I'm quite confident that this is the way to go about it.

Regards,
Jakob
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEa1VJLOiXAjQ2BGSm9Qb9Fp2P2VoFAl1JxqMACgkQ9Qb9Fp2P
2Vqb8g//ZObyAhO6cNTFAHob4iC/ygNuKL3qVKKpe5fGOrsZaAgQFxRON+1pgUem
RFMLdXaAqVbd26nthZSGZWTA1QYbEAEhwlapFv+1ZDIm9BAk0r+hz1TJ5Awz8AZW
v4BM4MrzG6iZMJ8MBZ6t2II29H60J6hPk29DIYU67FW8NlsqKUdQOfl5jOAh99lq
LDwjByhuCWzn02gZkfv7Sp8CRcYRxB67jy4byN2oH6VGpCqAr0MPOkP3vUIg7j5p
onvmT3Pt5MSa60wwSo92EiNmmZYd/QyV/B1Rg7ug7+nIu3Mbu9r/MgpcJUFMMWRU
/MPbZ8LU8D+YnI5D4y8tfQnGjw9qb1o/GtCDklu5CKNxJOV7115UyCMAy62/oXXI
N5NfkygfM4KRbPk5FAV1RSVncwVaxPIb3xe+EY1717HJavA38MdB++cVMF+Mzb2k
fAt/NL/5NYrAbi00Y1STZDvOjmQOndGemoeb8g0H3OC9X0yQH1zX9bf8oc/3Qcd2
4TSso5mBC06/rZAkTYkpaMyWVfwV5fpBop4fTdI4q5tn8hcB1dHAayfKItmbwBOk
iCC97LsJUjGH8fDxz/yrq+qrk90nTunZdK0Bg9iCJJvKDAYrRQ8rw2j98Jv4TdTX
gJ+anlbTc9e9IdN+xVI4foLNCRhAw2JLB/Jbdk8qWt79ezOOsJc=
=wCog
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 23 Aug 2019 14:28
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)
87sgps6p6c.fsf@gnu.org
Hello!

zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis:

Toggle quote (23 lines)
> Jesse Gibbons <jgibbons2357@gmail.com> writes:
>
>> I dual-booted Guix with another gnu/linux-libre distro.
>> My configuration includes the other distro in the grub menu. When I run
>> "sudo guix system delete-generations" the changes to the grub menu drop
>> the other distro with the older system generations of guix.
>>
>> My current work-around for this is to run "guix system reconfigure ..."
>> which includes the boot menu entries specified in the configuration.
>
> Thanks for reporting this; it's a rather serious issue. The problem lies
> in the 'reinstall-bootloader' procedure. Chiefly, it uses the default
> bootloader configuration for whatever it can find using
> 'lookup-bootloader-by-name' and generates menu entries for the
> generations reachable from '%system-profile', which is quite a bit
> different from how 'guix system reconfigure' produces the bootloader
> configuration. It really isn't ideal. To quote a comment in
> 'system.scm': "[i]t will be enough to allow the system to boot."
>
> I don't think this should be _too_ hard to fix. To me, parsing the
> installed Grub configuration to get existing menu entries seems like a
> logical step forward.

I agree with Danny here that parsing the GRUB config wouldn’t be great.

We have information about the user’s extra menu entries. The issue, as
I see it, as that this information is lost once the system is
instantiated.

But! We have the <boot-parameters> structure, that gets serialized with
the system, and which we could extend with those extra menu entries.
That way, the info would be preserved, and we can restore them upon
‘delete-generations’. <menu-entry> records are bootloader-independent,
which is good.

How does that sound?

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 23 Aug 2019 14:29
control message for bug #36876
(address . control@debbugs.gnu.org)
87r25c6p4y.fsf@gnu.org
severity 36876 important
quit
J
J
Jakob L. Kreuze wrote on 28 Aug 2019 17:38
Re: bug#36876: guix system delete-generations removes custom boot menu entries
(name . Ludovic Courtès)(address . ludo@gnu.org)
87lfvdpaey.fsf@sdf.lonestar.org
Hi Ludo,

Ludovic Courtès <ludo@gnu.org> writes:

Toggle quote (14 lines)
> I agree with Danny here that parsing the GRUB config wouldn’t be great.
>
> We have information about the user’s extra menu entries. The issue, as
> I see it, as that this information is lost once the system is
> instantiated.
>
> But! We have the <boot-parameters> structure, that gets serialized with
> the system, and which we could extend with those extra menu entries.
> That way, the info would be preserved, and we can restore them upon
> ‘delete-generations’. <menu-entry> records are bootloader-independent,
> which is good.
>
> How does that sound?

Would that involve appending an additional field to <boot-parameters>
for storing the previous entries? I think that would have the pleasant
side-effect of making the code for deployment/reconfiguration simpler :)

Regards,
Jakob
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEa1VJLOiXAjQ2BGSm9Qb9Fp2P2VoFAl1moAUACgkQ9Qb9Fp2P
2VqHBg/+I9HKnOOKbVcDscLbE+OFIPGnVdtYBXrUV1e26Yu1pplaK/pI1SL7rV14
5Xl1v+rWKczR597yIglbqllgd4ERs+u+EU6Ke1EDzymAFg2J+qRqcacRecwneXFw
OdMC+LtDc0m8nQRf2b2nnsC3BvUc4jF0c30sQGL0+Z/p/yGA+5q5UDBv2zPQ1258
L6MebpfNrYbR6HRObO+chDFM9CYEQNsS+ITfHJO+wAlKP9yEu3f8QjEyf08+t8Uv
KiLcB/wNixPtA6bwOeyWx/TqDeYF0KoLHX5LXq/dqBbXXRt1A5eWgBQorcA9iQA1
LoyJOSYNdKNxA7SyZqTdxAyluDnhAJXX3Fv5SziGKxmoJEPSub243/LMqdiqOF3W
cA681WGRc2FarK94kce5V43Xg/OBL7X9kWZETK58iRimJkpxXktMf8wtnMu2z2aw
eNny6pWKVDLcWZAjb5LnrOo/yMJI+vL+4r1Y24Wfqr/0kEyHmzDJtcMPjJlOI9B8
Mf9WhnqgtC0sWNSTo3+MnToVqE9d3afaMNt20cgd5gRp/OlUiE/lUs2O6sTbyKpk
3TqIXVh2SJ40hdUMzw/91Bwd3p33aZwFABfcyv7FyY4sAqwavS+1YGPcyiy4y1dX
m6uTgQ5y+jW1RMOn7SPX3KYg0dZGSrSQXPIUEZ+7q5JDmsf4ilE=
=oEDu
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 28 Aug 2019 23:39
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)
87r255dl5g.fsf@gnu.org
Hello,

zerodaysfordays@sdf.lonestar.org (Jakob L. Kreuze) skribis:

Toggle quote (20 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> I agree with Danny here that parsing the GRUB config wouldn’t be great.
>>
>> We have information about the user’s extra menu entries. The issue, as
>> I see it, as that this information is lost once the system is
>> instantiated.
>>
>> But! We have the <boot-parameters> structure, that gets serialized with
>> the system, and which we could extend with those extra menu entries.
>> That way, the info would be preserved, and we can restore them upon
>> ‘delete-generations’. <menu-entry> records are bootloader-independent,
>> which is good.
>>
>> How does that sound?
>
> Would that involve appending an additional field to <boot-parameters>
> for storing the previous entries? I think that would have the pleasant
> side-effect of making the code for deployment/reconfiguration simpler :)

Oh that’d be nice.

The attached patches should fix this. I’ve successfully deleted a
generation on my system. :-) I don’t have extra menu entries though, so
it’d be great if you could give it a try.

Thanks,
Ludo’.
From 0735c71707a5ea14e43e9adf6125801afa7db1ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Wed, 28 Aug 2019 23:27:20 +0200
Subject: [PATCH 1/2] system: Add 'bootloader-menu-entries' field to
<boot-parameters>.

This allows us to keep track of the extra menu entries specified in the
OS configuration.

* gnu/system.scm (<boot-parameters>)[bootloader-menu-entries]: New field.
(read-boot-parameters): Initialize it.
(operating-system-boot-parameters): Likewise.
(operating-system-boot-parameters-file): Serialize it.
* gnu/bootloader.scm (menu-entry->sexp, sexp->menu-entry): New
procedures.
---
gnu/bootloader.scm | 34 ++++++++++++++++++++++++++++++++++
gnu/system.scm | 15 +++++++++++++++
2 files changed, 49 insertions(+)

Toggle diff (122 lines)
diff --git a/gnu/bootloader.scm b/gnu/bootloader.scm
index a381f67145..03616c12ab 100644
--- a/gnu/bootloader.scm
+++ b/gnu/bootloader.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2017 David Craven <david@craven.ch>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
+;;; Copyright © 2019 Ludovic Courtès <ludo@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -23,6 +24,7 @@
   #:use-module (guix records)
   #:use-module (guix ui)
   #:use-module (srfi srfi-1)
+  #:use-module (ice-9 match)
   #:export (menu-entry
             menu-entry?
             menu-entry-label
@@ -32,6 +34,9 @@
             menu-entry-initrd
             menu-entry-device-mount-point
 
+            menu-entry->sexp
+            sexp->menu-entry
+
             bootloader
             bootloader?
             bootloader-name
@@ -76,6 +81,35 @@
                    (default '()))          ; list of string-valued gexps
   (initrd          menu-entry-initrd))     ; file name of the initrd as a gexp
 
+(define (menu-entry->sexp entry)
+  "Return ENTRY serialized as an sexp."
+  (match entry
+    (($ <menu-entry> label device mount-point linux linux-arguments initrd)
+     `(menu-entry (version 0)
+                  (label ,label)
+                  (device ,device)
+                  (device-mount-point ,mount-point)
+                  (linux ,linux)
+                  (linux-arguments ,linux-arguments)
+                  (initrd ,initrd)))))
+
+(define (sexp->menu-entry sexp)
+  "Turn SEXP, an sexp as returned by 'menu-entry->sexp', into a <menu-entry>
+record."
+  (match sexp
+    (('menu-entry ('version 0)
+                  ('label label) ('device device)
+                  ('device-mount-point mount-point)
+                  ('linux linux) ('linux-arguments linux-arguments)
+                  ('initrd initrd) _ ...)
+     (menu-entry
+      (label label)
+      (device device)
+      (device-mount-point mount-point)
+      (linux linux)
+      (linux-arguments linux-arguments)
+      (initrd initrd)))))
+
 
 ;;;
 ;;; Bootloader record.
diff --git a/gnu/system.scm b/gnu/system.scm
index 01be1243fe..a599ba2750 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -116,6 +116,7 @@
             boot-parameters-label
             boot-parameters-root-device
             boot-parameters-bootloader-name
+            boot-parameters-bootloader-menu-entries
             boot-parameters-store-device
             boot-parameters-store-mount-point
             boot-parameters-kernel
@@ -251,6 +252,8 @@ directly by the user."
   ;; OS's root file system, so it might be a device path like "/dev/sda3".
   (root-device      boot-parameters-root-device)
   (bootloader-name  boot-parameters-bootloader-name)
+  (bootloader-menu-entries                        ;list of <menu-entry>
+   boot-parameters-bootloader-menu-entries)
   (store-device     boot-parameters-store-device)
   (store-mount-point boot-parameters-store-mount-point)
   (kernel           boot-parameters-kernel)
@@ -297,6 +300,11 @@ file system labels."
          ((_ args) args)
          (#f       'grub))) ; for compatibility reasons.
 
+      (bootloader-menu-entries
+       (match (assq 'bootloader-menu-entries rest)
+         ((_ . entries) (map sexp->menu-entry entries))
+         (#f            '())))
+
       ;; In the past, we would store the directory name of the kernel instead
       ;; of the absolute file name of its image.  Detect that and correct it.
       (kernel (if (string=? linux (direct-store-path linux))
@@ -1005,6 +1013,8 @@ such as '--root' and '--load' to <boot-parameters>."
           (operating-system-user-kernel-arguments os)))
      (initrd initrd)
      (bootloader-name bootloader-name)
+     (bootloader-menu-entries
+      (bootloader-configuration-menu-entries (operating-system-bootloader os)))
      (store-device (ensure-not-/dev (file-system-device store)))
      (store-mount-point (file-system-mount-point store)))))
 
@@ -1046,6 +1056,11 @@ being stored into the \"parameters\" file)."
                      #$(boot-parameters-kernel-arguments params))
                     (initrd #$(boot-parameters-initrd params))
                     (bootloader-name #$(boot-parameters-bootloader-name params))
+                    (bootloader-menu-entries
+                     #$(map menu-entry->sexp
+                            (or (and=> (operating-system-bootloader os)
+                                       bootloader-configuration-menu-entries)
+                                '())))
                     (store
                      (device
                       #$(device->sexp (boot-parameters-store-device params)))
-- 
2.23.0
From f0c5d8f1479d899ce5e646f7a157c571c9b6d80c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Wed, 28 Aug 2019 23:31:28 +0200
Subject: [PATCH 2/2] guix system: Reinstalling the bootloader preserves extra
menu entries.

Reported by Jesse Gibbons <jgibbons2357@gmail.com>.

Previously 'guix system delete-generations' or 'switch-generation' would
lose the extra bootloader menu entries specified in the current system's
configuration. This fixes that.

* guix/scripts/system.scm (reinstall-bootloader): Turn PARAMS into a
single <boot-parameters>. Adjust ENTRIES to include extra menu entries
specified in PARAMS.
---
guix/scripts/system.scm | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Toggle diff (23 lines)
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 9fc3a10e98..27b014db68 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -384,12 +384,14 @@ STORE is an open connection to the store."
                              (bootloader bootloader)))
 
          ;; Make the specified system generation the default entry.
-         (params (profile-boot-parameters %system-profile (list number)))
+         (params (first (profile-boot-parameters %system-profile
+                                                 (list number))))
          (old-generations
           (delv number (reverse (generation-numbers %system-profile))))
          (old-params (profile-boot-parameters
                        %system-profile old-generations))
-         (entries (map boot-parameters->menu-entry params))
+         (entries (cons (boot-parameters->menu-entry params)
+                        (boot-parameters-bootloader-menu-entries params)))
          (old-entries (map boot-parameters->menu-entry old-params)))
     (run-with-store store
       (mlet* %store-monad
-- 
2.23.0
L
L
Ludovic Courtès wrote on 29 Aug 2019 10:02
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)
877e6wl7py.fsf@gnu.org
Hi there!

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (4 lines)
> The attached patches should fix this. I’ve successfully deleted a
> generation on my system. :-) I don’t have extra menu entries though, so
> it’d be great if you could give it a try.

So I tried it on my laptop: I added a dummy menu entry, reconfigured,
rebooted, then ran “guix system delete-generations N”, and I confirm
that it preserves the extra menu entry.

There was a typo though:

Toggle quote (26 lines)
> --- a/gnu/system.scm
> +++ b/gnu/system.scm
> @@ -116,6 +116,7 @@
> boot-parameters-label
> boot-parameters-root-device
> boot-parameters-bootloader-name
> + boot-parameters-bootloader-menu-entries
> boot-parameters-store-device
> boot-parameters-store-mount-point
> boot-parameters-kernel
> @@ -251,6 +252,8 @@ directly by the user."
> ;; OS's root file system, so it might be a device path like "/dev/sda3".
> (root-device boot-parameters-root-device)
> (bootloader-name boot-parameters-bootloader-name)
> + (bootloader-menu-entries ;list of <menu-entry>
> + boot-parameters-bootloader-menu-entries)
> (store-device boot-parameters-store-device)
> (store-mount-point boot-parameters-store-mount-point)
> (kernel boot-parameters-kernel)
> @@ -297,6 +300,11 @@ file system labels."
> ((_ args) args)
> (#f 'grub))) ; for compatibility reasons.
>
> + (bootloader-menu-entries
> + (match (assq 'bootloader-menu-entries rest)
> + ((_ . entries) (map sexp->menu-entry entries))
^
There shouldn’t be a dot here.

I’ll go ahead and push these patches (with this correction) if there are
no objections.

Thanks,
Ludo’.
L
L
Ludovic Courtès wrote on 30 Aug 2019 01:35
(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)
87a7brh7eu.fsf@gnu.org
Pushed as c3e59de9b1340f1a0ef7e30dd2e4e7bf7b484ee9.

Let me know if anything’s still not quite as expected!

Ludo’.
Closed
?
Your comment

This issue is archived.

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