guix system delete-generations removes custom boot menu entries

Done

Details

4 participants

Danny Milosavljevic
Jesse Gibbons
Ludovic Courtès
Jakob L. Kreuze

Owner: unassigned

Submitted by: Jesse Gibbons

Severity: important

Jesse Gibbons wrote on 31 Jul 2019 17:48

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

Message-ID: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.

Jakob L. Kreuze wrote on 5 Aug 2019 18:05

Recipients:(name . Jesse Gibbons)(address . jgibbons2357@gmail.com)(address . 36876@debbugs.gnu.org)

Message-ID: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

Jesse Gibbons wrote on 6 Aug 2019 05:12

Recipients:(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)(address . 36876@debbugs.gnu.org)

Message-ID: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

Jakob L. Kreuze wrote on 6 Aug 2019 15:22

Recipients:(name . Jesse Gibbons)(address . jgibbons2357@gmail.com)(address . 36876@debbugs.gnu.org)

Message-ID: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

Danny Milosavljevic wrote on 6 Aug 2019 18:32

Recipients:(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)

Message-ID: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-----

Danny Milosavljevic wrote on 6 Aug 2019 18:35

Recipients:(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)

Message-ID: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-----

Jakob L. Kreuze wrote on 6 Aug 2019 20:27

Recipients:(name . Danny Milosavljevic)(address . dannym@scratchpost.org)

Message-ID: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

Ludovic Courtès wrote on 23 Aug 2019 14:28

Recipients:(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)

Message-ID: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’.

Ludovic Courtès wrote on 23 Aug 2019 14:29

control message for bug #36876

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

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

severity 36876 important

quit

Jakob L. Kreuze wrote on 28 Aug 2019 17:38

Re: bug#36876: guix system delete-generations removes custom boot menu entries

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

Message-ID: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

Ludovic Courtès wrote on 28 Aug 2019 23:39

Recipients:(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)

Message-ID: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’.

Attachment: 0001-system-Add-bootloader-menu-entries-field-to-boot-par.patch

From f0c5d8f1479d899ce5e646f7a157c571c9b6d80c Mon Sep 17 00:00:00 2001

menu entries.

Fixes https://bugs.gnu.org/36876.

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

Ludovic Courtès wrote on 29 Aug 2019 10:02

Recipients:(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)

Message-ID: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’.

Ludovic Courtès wrote on 30 Aug 2019 01:35

Recipients:(name . Jakob L. Kreuze)(address . zerodaysfordays@sdf.lonestar.org)

Message-ID: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 an email to 36876@debbugs.gnu.org

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