[bug#73202] guix: scripts: Rewrite reinstall-bootloader to use provenance data.

From: Lilah Tascheter <lilah@lunabee.space>

Looking up bootloaders by name is broken because (extlinux) bootloaders
share a name.  Also, bootloader-configuration data is significant to
bootloader installation, so it shouldn't just use the default values.
Installation can rely on the provenance service instead, which should be
present for the vast majority of systems.

* guix/scripts/system.scm (install-bootloader-from-os,
install-bootloader-from-provenance): Add procedures.
(reinstall-bootloader): Remove procedure.
(switch-to-system-generation, process-command): Use
install-bootloader-from-provenance.

Change-Id: I5713a43ad4f9f32a129d980db06d70de16b03f27
---
Hello,

This requires patches from #69343.  #72457 is big and I thought it would
be nice to separately review whatever possible, hence the new issue.  

This is [PATCH v5 01/15] from issue #72457, but with a modified commit
description and the addition of an install-bootloader-from-os procedure,
to reduce nesting and only define local variables when relevant.

The (gnu tests reconfigure) tests all pass, though I myself cannot
roll-back or switch-generations for unrelated reasons.  So please let me
know if this patch creates any trouble with the aformentioned and if you
have ideas for additional (gnu tests reconfigure) tests.

Thanks,
Herman

 gnu/bootloader.scm      |  2 ++
 guix/scripts/system.scm | 72 +++++++++++++++--------------------------
 2 files changed, 28 insertions(+), 46 deletions(-)
  

Hello,

I posted revision v5 of #69343.  If you would like to apply v2 of this
issue on it, use git am -3 and pick the conflicting change made by patch
#5 as is.  Also note that the changes made by patch #14 are now included
in v5 of #69343.

Cheers,
Herman
  

hey herman!

> use git am -3
fails on a clean pull/am due to lack of commit blobs. I'm not quite
sure how to fix that myself, would you be able to just re-send the
rebased patch series?

thanks!
  

Hi Lilah,

I wrote a series of annotated Guile code snippets, regarding an
alternative implementation of bootloader-target, and example user
configuration (with tftp/NFS).  See below, and please let me know what
you think.

Cheers,
Herman

;; In (gnu bootloader grub):
(define (grub-efi-default-targets esp)
  (tree->target ; like arborify
                (bootloader-target
                  (type 'esp)
                  (path esp)
                  (targets ; counterpart of offset
                           (list (bootloader-target
                                   (type 'vendir)
                                   (path "EFI/Guix"))
                                 (bootloader-target
                                   (type 'install)
                                   (path "grub")))))))

;; This is the same as (grub-efi-default-targets "boot").  It could be
;; exported standalone, instead of using bootloader default-targets.
(define %grub-efi-default-targets
  '((esp     . "/boot")
    (install . "/boot/EFI/Guix")
    (vendir  . "/boot/grub")))

;; A simple consequence of this change is that this:
(with-targets %grub-efi-default-targets
  (('install => (path :path))
   ...))
;; becomes:
(let ((install-path (assoc-ref %grub-efi-default-targets 'install)))
  ...)

;; But e.g. device is provided separately (or derived from path):
(with-targets %grub-efi-default-targets
  (('esp => (device :device))
   ...))
;; becomes:
(let* ((path (assoc-ref %grub-efi-default-targets 'esp))
       ;; A single root-device is provided in addition to targets.
       (device (root-device->block-device root-device path)))
  ...)

;; This procedure is in (gnu bootloader).  Something like:
(define (root-device->block-device device path)
  (match device
    ((? block-device?) device) ; string with /dev/ prefix
    ((? uuid?) (find-partition-for-uuid device))
    ((? string?) (find-partition-for-label device))
    ;; This might be necessary, but should not be relied on.
    (_ (mount-source (find-mount path)))))

;; Example user configuration:
(define %grub-efi-bootloader
  (bootloader-configuration
    (bootloader grub-efi-bootloader)
    ;; This is for bootloader configuration (not installation), e.g.
    ;; GRUB search or install-efi disk argument.
    (root-device "UUID, label, or block device.")
    ;; This is complementary to root-device.  It will configure some
    ;; files to be fetched remotely instead of installed.
    (tftp "Varies for UEFI/GRUB/U-Boot.")))

(operating-system
  (bootloader (list %grub-efi-bootloader))
  ;; This is shared between bootloaders.  Ideally, it does not affect
  ;; which files are installed or their contents, but only the location.
  (bootloader-targets (grub-efi-default-targets "boot")))
  

hey herman!

> ;; In (gnu bootloader grub):
> (define (grub-efi-default-targets esp)
>   (tree->target ; like arborify
>                 (bootloader-target
>                   (type 'esp)
>                   (path esp)
>                   (targets ; counterpart of offset
>                            (list (bootloader-target
>                                    (type 'vendir)
>                                    (path "EFI/Guix"))
>                                  (bootloader-target
>                                    (type 'install)
>                                    (path "grub")))))))
how would this handle root offsets, eg by guix system init? is
everything assumed to be offset from root? I'm also worried about
indentation growing too quickly. otherwise, though, it's definately an
improvement over offset!

> ;; This is the same as (grub-efi-default-targets "boot").  It could
> be
> ;; exported standalone, instead of using bootloader default-targets.
> (define %grub-efi-default-targets
>   '((esp     . "/boot")
>     (install . "/boot/EFI/Guix")
>     (vendir  . "/boot/grub")))
> 
> ;; A simple consequence of this change is that this:
> (with-targets %grub-efi-default-targets
>   (('install => (path :path))
>    ...))
> ;; becomes:
> (let ((install-path (assoc-ref %grub-efi-default-targets 'install)))
>   ...)
> 
> ;; But e.g. device is provided separately (or derived from path):
> (with-targets %grub-efi-default-targets
>   (('esp => (device :device))
>    ...))
> ;; becomes:
> (let* ((path (assoc-ref %grub-efi-default-targets 'esp))
>        ;; A single root-device is provided in addition to targets.
>        (device (root-device->block-device root-device path)))
>   ...)
> 
> ;; This procedure is in (gnu bootloader).  Something like:
> (define (root-device->block-device device path)
>   (match device
>     ((? block-device?) device) ; string with /dev/ prefix
>     ((? uuid?) (find-partition-for-uuid device))
>     ((? string?) (find-partition-for-label device))
>     ;; This might be necessary, but should not be relied on.
>     (_ (mount-source (find-mount path)))))
how are you replacing device-local paths? some bootloaders need that
information to access files before fully loading. also, if the path,
device, label, and uuid fields are combined, the guix system image
won't be able to get all the info it needs to the bootloader
installers. uuid or label needs to be there to identify the device on-
boot, but also path, device, and devpath are required to actually
install bootloader files.

also, one reason with-targets exists is as a safeguard for future
people writing bootloaders. guix system image tends to be overlooked,
so it performs checks to make sure the bootloader targets requested are
available during image generation.


> ;; Example user configuration:
> (define %grub-efi-bootloader
>   (bootloader-configuration
>     (bootloader grub-efi-bootloader)
>     ;; This is for bootloader configuration (not installation), e.g.
>     ;; GRUB search or install-efi disk argument.
>     (root-device "UUID, label, or block device.")
>     ;; This is complementary to root-device.  It will configure some
>     ;; files to be fetched remotely instead of installed.
>     (tftp "Varies for UEFI/GRUB/U-Boot.")))
> 
> (operating-system
>   (bootloader (list %grub-efi-bootloader))
>   ;; This is shared between bootloaders.  Ideally, it does not affect
>   ;; which files are installed or their contents, but only the
> location.
>   (bootloader-targets (grub-efi-default-targets "boot")))
I do really like the conceptual separation between configuration and
installation! though, users would now need to enter the root device
details three times, potentially in inconsistant formats.

- lilah
  

Hello,

On Fri, Oct 04, 2024 at 12:07:16AM -0500, Lilah Tascheter wrote:
> > (define (grub-efi-default-targets esp)
> how would this handle root offsets, eg by guix system init? is
> everything assumed to be offset from root?
For every (key . value) in targets, prefix value with root-offset.  Or
if the target tree is still available, extend it:

  (bootloader-target
    (type 'root)
    (path root-offset)
    (targets (list %target-tree)))

It works under the assumption that bootloader-target paths will not be
referenced in bootloader configuration files.  When that does not hold,
e.g. in the (make-)grub.cfg procedure, then I think either the target
paths (or tree) need to be unprefixed (or unwrapped); or the root should
be offset implicitly, i.e. doing the installation in a chroot.

> I'm also worried about indentation growing too quickly.
Do you have a use case in mind, with more than three levels of nesting?

> otherwise, though, it's definately an improvement over offset!
Thanks.  Using the assumption that Guix only works with UNIX file
systems and not DOS-derivatives or exotic data stores, it only allows
constructing a tree and not a forest or (cyclic) graph, respectively.

> how are you replacing device-local paths? some bootloaders need that
> information to access files before fully loading.
I guess when device-local paths are required, a targets tree should be
provided and queried using the with-targets macro.  They could also be 
cast from target paths, with a warning.

> also, if the path, device, label, and uuid fields are combined, the
> guix system image won't be able to get all the info it needs to the
> bootloader installers. uuid or label needs to be there to identify the
> device on-boot,
So I have tried combining the path field into the device field, but I'm
now in favor of using a target tree/paths field together with a combined
block device, file system label, or UUID field.  Here the assumption is
that any of the aformentioned types can be derived from any other, e.g.
with find-partition-by-uuid and read-partition-label.  If a bootloader
cannot use a provided type, or find other required types, it should
throw an error.  If you have a use case where both a block device and a
(potentially unrelated) UUID are configured, please let me know.

> but also path, device, and devpath are required to actually install
> bootloader files.
I think the device could be installation-agnostic and anything related
to installation could be a different bootloader, or a field like tftp.

> also, one reason with-targets exists is as a safeguard for future
> people writing bootloaders. guix system image tends to be overlooked,
> so it performs checks to make sure the bootloader targets requested
> are available during image generation.
What do you think about having required types per bootloader, and tests
for trees generated from image partitions in (gnu tests image) instead?

That reminds me: I would like to add a supported file systems field to
the bootloader, so that if the file system found for root-device is not
supported, it throws a little error.

> > (operating-system
> >   (bootloader (list %grub-efi-bootloader))
> >   ;; This is shared between bootloaders.  Ideally, it does not affect
> >   ;; which files are installed or their contents, but only the
> > location.
> >   (bootloader-targets (grub-efi-default-targets "boot")))
> I do really like the conceptual separation between configuration and
> installation! though, users would now need to enter the root device
> details three times, potentially in inconsistant formats.
Thanks, I think the two examples below could work pretty well.

Cheers,
Herman

(define %boot-fs
  (file-system
    (device (uuid "E6A5-FEBB" 'fat32))
    (mount-point "/boot") ; Taken as ESP.
    ;; Cannot be used to configure e.g. GRUB netboot, but it would be
    ;; nice to assert (support? bootloader type) in fs->bootloader.
    (type "vfat")))
(operating-system 
  (bootloader ;; Procedure defined in (gnu system file-systems).
              (file-system->grub-efi-bootloader %boot-fs))
  ...)

;; bootloader->file-system would not work as well.  An OS field (macro)
;; to define both simultaneously at a high level could be useful though.
(operating-system
  (file-systems-with-bootloader
    ;; Irrelevant for file-systems.
    (bootloader grub-efi-bootloader)
    ;; Relevant as a file-system and bootloader installation.
    (boot-device "UUID, label, or block device.")
    (mount-point "/boot")
    (type "vfat")
    ;; Not relevant to bootloader.  Default values given.
    (root-file-system (mounted-root-fs)) ; Error if not found.
    ;; Cons the generated boot FS and mounted root FS to this.
    (file-systems %base-file-systems))
  ...)
  

Hi all, question about this entire patchset. I would like to do some testing
and play around with this myself, but I cannot for the life of me figure out
how it's supposed to be patched on top of the main tree, if someone could
clarify.

Right now, I have Guix master branch checked out @7e63a35f, and am under the
assumption that the order of patching should be as follows: 69343->73202
->72457. Is this correct? I can apply 69343 cleanly, however trying to apply
the latest patchsets for 73202 or 72457 fail immediately, complaining that
either I cannot build fake ancestors (attempting a 3-way merge) or that various
files do not match the index. I'm assuming I am missing some more patchsets
somewhere, but am unsure which are dependent on one another.

Hoping for some clarification so I can get my local tree all patched and begin
some testing (nothing too special, just want to set up UKIs on x86_64 but it
could be some insight nonetheless!)

Thanks!

--- 
Best,
Ryan S
  

Hello Ryan,

On Mon, Oct 07, 2024 at 12:59:16PM -0400, Ryan wrote:
> Right now, I have Guix master branch checked out @7e63a35f, and am under the
> assumption that the order of patching should be as follows: 69343->73202
> ->72457. Is this correct?
Almost, v6 of 72457 additionally requires v2 of 70131.

> I can apply 69343 cleanly, however trying to apply
> the latest patchsets for 73202 or 72457 fail immediately, complaining that
Yeah, applying 73202 after 69343 using 'mumi am' immediately returns:

  Applying: gnu: bootloader: Install any bootloader to ESP.
  error: patch failed: gnu/bootloader.scm:113
  error: gnu/bootloader.scm: patch does not apply
  error: patch failed: gnu/build/bootloader.scm:23
  error: gnu/build/bootloader.scm: patch does not apply
  Patch failed at 0001 gnu: bootloader: Install any bootloader to ESP.

Very strange because this is not [PATCH v3 01/14], but [PATCH v3 12/14].
Manually downloading the patches from [1] and applying them seems to
work, until again [PATCH v3 12/14].  Looking closer at the download of
it (34), it seems to actually be [PATCH v3 14/14]...  Besides that, your
message here does not appear on [1], or [2] for October 07.  Do you know
why that is?

But anyway, Lilah also had trouble applying v6 of 72457.  v7 of 72457
will be independent of 70131, and hopefully possible to apply.  For now,
you can get patches similar to those in 70131, 69343, 73202, 72457, and
68524 (UKI bootloader) from my git repository, by running:

git checkout 7e63a35f
git remote add herman https://codeberg.org/herman_rimm/guix.git
git fetch herman
git cherry-pick herman/lint-order..herman/uki-efi-bootloader-stable

Cheers,
Herman

[1]: https://issues.guix.gnu.org/73202
[2]: https://lists.gnu.org/archive/html/guix-patches/2024-10/index.html
  

Hi Herman,

> On Mon, Oct 07, 2024 at 09:23:14PM +0200, Herman wrote:
> Very strange because this is not [PATCH v3 01/14], but [PATCH v3 12/14].
> Manually downloading the patches from [1] and applying them seems to
> work, until again [PATCH v3 12/14].  Looking closer at the download of
> it (34), it seems to actually be [PATCH v3 14/14]...  Besides that, your
> message here does not appear on [1], or [2] for October 07.  Do you know
> why that is?

Very odd, do you think this is a bug in mumi of some kind? I'm new to email
based git workflows, so I've been leaning a mumi for a lot of my setup. Didn't
even think to look if it was trying to apply patches in the correct order, I
just assumed it was and I was doing something wrong.

To your second point, I have the same question. My current theory is that I am
new enough to the mailing lists that I have to go through some sort of approval
process before my emails appear in the actual list and subsequent mumi site,
but I have no proof of this, other than my posts historically taking many hours
to actually appear. Maybe I am doing something wrong?

> But anyway, Lilah also had trouble applying v6 of 72457.  v7 of 72457
> will be independent of 70131, and hopefully possible to apply.  For now,
> you can get patches similar to those in 70131, 69343, 73202, 72457, and
> 68524 (UKI bootloader) from my git repository, by running:
> 
> git checkout 7e63a35f
> git remote add herman https://codeberg.org/herman_rimm/guix.git
> git fetch herman
> git cherry-pick herman/lint-order..herman/uki-efi-bootloader-stable

Thanks for linking your tree (and adding the helpful instructions!) Glad to
report I cherry-picked the commits without any issues. I'll continue working
on my changes I am making to my system (that integrates these changes) and
report on the proper issues if I run into anything!

--- 
Best,
Ryan S
  

ugh forgot to send this to the mailing list, so resending:

> Very odd, do you think this is a bug in mumi of some kind?

yep! I tried to message guix-devel about it but nobody responded. no
clue where to report mumi bugs. a control message I sent caused mumi's
message download links to be offset one back, resulting in a request to
download any email (both in the cli and the web ui) to return the
previous email in order.

it sucks.
  

hi!!

> > I'm also worried about indentation growing too quickly.
> Do you have a use case in mind, with more than three levels of
> nesting?
yeah good point :p

> > also, if the path, device, label, and uuid fields are combined, the
> > guix system image won't be able to get all the info it needs to the
> > bootloader installers. uuid or label needs to be there to identify
> > the device on-boot,
> So I have tried combining the path field into the device field, but
> I'mnow in favor of using a target tree/paths field together with a
> combined block device, file system label, or UUID field.  Here the
> assumption is that any of the aformentioned types can be derived from
> any other, e.g. with find-partition-by-uuid and read-partition-
> label.  If a bootloader cannot use a provided type, or find other
> required types, it should throw an error.  If you have a use case
> where both a block device and a (potentially unrelated) UUID are
> configured, please let me know.
alright, that sounds great! would work for image gen, and can't think
of a reason why distinct uuids and devices would be supplied.

> 
> > but also path, device, and devpath are required to actually install
> > bootloader files.
> I think the device could be installation-agnostic and anything
> related
> to installation could be a different bootloader, or a field like
> tftp.

> > also, one reason with-targets exists is as a safeguard for future
> > people writing bootloaders. guix system image tends to be
> > overlooked,
> > so it performs checks to make sure the bootloader targets requested
> > are available during image generation.
> What do you think about having required types per bootloader, and
> tests
> for trees generated from image partitions in (gnu tests image)
> instead?
oh yeah that's a way better idea! offloads the test work from runtime
to, well, testing.


> That reminds me: I would like to add a supported file systems field
> to the bootloader, so that if the file system found for root-device
> is not supported, it throws a little error.
sounds good, make sure the field supports specifying that all
filesystems are supported though (mostly just because of bootloaders
that install a kernel directly, like uki-efi).

> (define %boot-fs
>   (file-system
>     (device (uuid "E6A5-FEBB" 'fat32))
>     (mount-point "/boot") ; Taken as ESP.
>     ;; Cannot be used to configure e.g. GRUB netboot, but it would be
>     ;; nice to assert (support? bootloader type) in fs->bootloader.
>     (type "vfat")))
> (operating-system 
>   (bootloader ;; Procedure defined in (gnu system file-systems).
>               (file-system->grub-efi-bootloader %boot-fs))
>   ...)
> 
> ;; bootloader->file-system would not work as well.  An OS field
> (macro)
> ;; to define both simultaneously at a high level could be useful
> though.
> (operating-system
>   (file-systems-with-bootloader
>     ;; Irrelevant for file-systems.
>     (bootloader grub-efi-bootloader)
>     ;; Relevant as a file-system and bootloader installation.
>     (boot-device "UUID, label, or block device.")
>     (mount-point "/boot")
>     (type "vfat")
>     ;; Not relevant to bootloader.  Default values given.
>     (root-file-system (mounted-root-fs)) ; Error if not found.
>     ;; Cons the generated boot FS and mounted root FS to this.
>     (file-systems %base-file-systems))
>   ...)
so, the benefit here is that bootloader builds would be deterministic
from the bootloader-configuration, right?  I feel like a new top-level
macro, that requires specific fields for each possible device type is
unwieldly.  it's also potentially important to be able to install
multiple distinct bootloaders with distinct configurations, for eg
u-boot->uefi chainloading or raid arrays. how about something like the
following:

(operating-system
  (bootloader (list (grub-efi-bootloader
                      ;; ... remove the bootloader-configuration record
                      ;; entirely, and have each bootloader take their
                      ;; own config.  apart from targets and
                      ;; menu-entries (which we can split off), there
                      ;; aren't really any shared config opts anyway.
                      ;; assoc-fs assocs a path with a file-system type
                      ;; from the operating-system record (delay
                      ;; or thunk the bootloader field so that images
                      ;; can override file-systems?)
                      (root (assoc-fs file-systems "/")))))
  ;; have your original targets system in place
  (bootloader-targets ...)
  ;; non-grub replacement for menu-entries, potentially with a 
  ;; %base-boot-options thing for the autogenerated ones per
  ;; guix system generation?
  (boot-options ...))

with a field sanitizer to make singular entries into lists, to simplify
single-bootloader use.  devpaths would then be able to be generated by
the bootloader using the configuration target information.

honestly, then maybe just specify a target field (taking a symbol) in
the file-system record, and have assoc-fs take either a target symbol
or mount path. have bootloader-targets be generated from the file-
systems, with the bootloader-targets field just specifying non-
filesystem block devices.

I think parts of that may be similar to what you were originally
intending? I'm sorry, if so.

- lilah
  

Hi Herman and Lilah,

now that guix moved to codeberg, would you consider opening a pull
request there?
  

Hello,

For now, I opened a pull request on Codeberg for issue #75010:

https://codeberg.org/guix/guix/pulls/372

Cheers,
Herman