composefs/boot: Fix sd-boot order on update

[Johan-Liebert1]

Grub sorts its BLS config in descending order, while sd-boot sorts the configs in ascending order. While upgrading we were always setting the new sort key to be 1 which would work for Grub but not for sd-boot.

Fixes: #1777

[Johan-Liebert1]

I'm also working on adding tests for these cases

[gemini-code-assist]

Code Review

This pull request fixes the boot order for sd-boot during an update by setting the correct sort-key for BLS entries. The change correctly handles the different sorting behaviors of Grub (descending) and systemd-boot (ascending). The logic is sound. I've added one suggestion to refactor duplicated logic to improve code maintainability.

[cgwalters]

Hmm I think it'd be cleaner if computed all the BLS entries without writing them, and then walk over all of them and reverse the order if we detect grub or so?

[Johan-Liebert1]

That is kind of what we're doing, but in a bit of a weird way. This might be good for a cleanup. I'll pick it up

[cgwalters]

How about

sorting_keys(&self) -> impl Iterator<Item=&'static str> {
  let r = ["0", "1"]
  match self {
    Systemd => r
    Grub => r.rev()
  }

[Johan-Liebert1]

We use the secondary keys when upgrading/switching, so this doesn't quite work for that case

[cgwalters]

OK first when talking about grub, there's a giant mess there because it has a wildly varying patch set per distro/OS. In particular https://src.fedoraproject.org/rpms/grub2 now has 390 (!) patches.

Anyways I think the real problem here is:

~/src/src.fedoraproject.org/rpms/grub2/grub2-2.12-build/grub-2.12> grep -r sort-key .
./docs/grub.texi:sort-key     fedora
~/src/src.fedoraproject.org/rpms/grub2/grub2-2.12-build/grub-2.12>

i.e. at least Fedora's grub2 isn't implementing sort-key at all AFAICS. I suspect we're getting sorting by filenames instead.

We should make this at least build or runtime configurable probably in the future, but for now until we gather more data on grub on other OSes it's likely simplest to just not emit sort-key at all with grub and ensure that we always do filename ordering.

[cgwalters]

See above

[TGNThump]

From what I can see, this patch doesn't change the current behaviour for Grub, but fixes the currently broken behaviour for systemd-boot. Is it worth getting this in first and then dealing with Grub as a separate PR if more info is needed before figuring out the best solution for Grub?

[Johan-Liebert1]

I suspect we're getting sorting by filenames instead

That's what's happening, at least in fedora based images. The sort-key is present in the BLS Config even for grub is because we only have one write function for the BLSConfig struct which writes every field in the struct regardless of which bootloader is used

[cgwalters]

From what I can see, this patch doesn't change the current behaviour for Grub, but fixes the currently broken behaviour for systemd-boot. Is it worth getting this in first and then dealing with Grub as a separate PR if more info is needed before figuring out the best solution for Grub?

Yes I think that's true, however I think it's well worth some further cleanups here. We'll get this fixed soon.