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

["Ludovic Courtès" <ludo@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]

Hello,

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

> 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 #2: 0001-system-Add-bootloader-menu-entries-field-to-boot-par.patch --]
[-- Type: text/x-patch, Size: 5639 bytes --]

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(+)

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)))))
+
 \f
 ;;;
 ;;; 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-guix-system-Reinstalling-the-bootloader-preserves-ex.patch --]
[-- Type: text/x-patch, Size: 1879 bytes --]

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.

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(-)

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