layout/blspec.py: Properly implement specification type 1 and type 2

[AndrewAmmerlaan]

This makes eclean-kernel work correctly with uki's in the Boot Loader Specification type 2 layout.

As described in the kernel-install manual, in the type 1 layout linux and initrd are under $BOOT/loader/entries/ENTRY-TOKEN-KERNEL-VERSION (not under /boot/efi etc).

In the type 2 layout unified kernel image are in the Linux directory on the EFI System Partition (which might be mounted under /efi, /boot/efi or /boot/efi/efi, etc).

We explicitly do not touch efi files in the ESP/Linux directory that do not match our ENTRY_TOKEN, these might belong to other distributions.

If we can't find anything in the type 1 or type 2 location, then we raise an error.

Closes: #28

[AndrewAmmerlaan]

And updated the tests accordingly

[mgorny]

Are both layouts supposed to be used together? Does systemd-boot handle kernels from both?

[AndrewAmmerlaan]

Does systemd-boot handle kernels from both?

Yes sd-boot checks $BOOT/loader/entries for boot entries and checks $ESP for efi exectuables (Linux UKI's but also other OS boot loaders etc).

It's perfectly fine to do, e.g.:

KERNEL_INSTALL_LAYOUT=bls kernel-install some_kernel
KERNEL_INSTALL_LAYOUT=uki kernel-install some_other_kernel

Or even to install the same kernel with both layouts.

Are both layouts supposed to be used together?

Well whether or not your supposed to do this is a different question. I would say no because it is confusing. But in e.g. multi-boot cases where one distro installs uki's and another installs bls it might be unavoidable. In any case sd-boot does not care if we have Linux kernels in both bls and uki layouts at the same time.

We should definitely support cleaning both the UKI and the regular linux+initrd of a given kernel version because in the situation where a user switches from one layout to another they can end up with the same kernel version installed in both layouts.

[mgorny]

Again, it would make it easier to figure out if you split it into smaller changes. First refactor stuff, then add support for the other layout.

[AndrewAmmerlaan]

Again, it would make it easier to figure out if you split it into smaller changes. First refactor stuff, then add support for the other layout.

I separated this into three commits. I hope it helps, but the first commit with the indentation changes still is difficult to read.

[prometheanfire]

Tested this against 2.99.4 and it cleaned up my crazy systemd-boot rebuild, signed, etc. Worked well

[mgorny]

Thinking about it, even if it somehow worked, I don't think we should be combining kernels from both layouts under the same version. After all, the bootloader doesn't "combine" them either.

Probably the correct thing would be to include "sublayout" identifier in the dictionary key. However, I don't think collisions are all that likely, so I think it would be good enough to just error out if one ever occurs.

[AndrewAmmerlaan]

Thinking about it, even if it somehow worked, I don't think we should be combining kernels from both layouts under the same version. After all, the bootloader doesn't "combine" them either.

Probably the correct thing would be to include "sublayout" identifier in the dictionary key. However, I don't think collisions are all that likely, so I think it would be good enough to just error out if one ever occurs.

Well I can't say I agree. Having a given kernel version installed in both layouts is unusual but if it does occur then I don't see why a user would want to keep one or the other. If eclean-kernel asks me if I want to clean up version x.y.z then I expect it to clean up all files that belong to kernel version x.y.z.
Say I am running version X and decide to switch to uki's, now I have X installed in both layouts. After an upgrade to X+1 I would want to clean up both versions of X. An error is just annoying because now I have to manually cleanup X.

Plus explicitly not supporting simultaneous use of both layouts just makes the code more complex. As I explained above we already need the try-except anyway for dealing with any initrd files installed with a given kernel version. If I now have to make a uki file for a given version a special case then I can't really use one function for both layouts like I'm doing now.

This just feels to me like going out of our way to explicitly not support something that is unusual but nonetheless valid. I really don't see the point.

[mgorny]

They are not the same kernel. They are two different kernels that may (or may not, if their "internal versions" mismatch) share modules. They may belong to two different distributions even.

[AndrewAmmerlaan]

They are not the same kernel. They are two different kernels that may (or may not, if their "internal versions" mismatch) share modules. They may belong to two different distributions even.

No, we only touch uki's that match our ENTRY_TOKEN (see line 137). Just like we only touch the directory matching our ENTRY_TOKEN in the type 1 layout. I don't see how they could be different versions since this is part of the file name in type 2 layout and part of the directory name in the type 1 layout. Also the fact that kernels in both layouts share modules is all the more reason to remove the kernel for both layouts if both are present. Removing the linux+initrd in the type 1 layout and not removing the uki with the same version will just leave a broken uki since the module directory was removed.

[mgorny]

The module directory won't be removed unless all kernels referencing it are removed.

[AndrewAmmerlaan]

Alright, but I'm still not convinced we should make this error out if there is a type 1 and type 2 kernel with the same entry_token and the same version.

$BOOT/$ENTRY_TOKEN/x.y.z-gentoo/linux and $BOOT/EFI/Linux/$ENTRY_TOKEN-x.y.z-gentoo.efi might be differently configured but they are the same kernel version belonging to the same distro.

[mgorny]

Sure, that's why I said it's preferable to have it emit two separate entries, i.e. include the layout type in dict key.

[AndrewAmmerlaan]

Sure, that's why I said it's preferable to have it emit two separate entries, i.e. include the layout type in dict key.

Alright, I'll look into it. It is not immediately obvious to me how to make that work with the rest of the higher up code so it might take a while.

[AndrewAmmerlaan]

Alright, I'll look into it. It is not immediately obvious to me how to make that work with the rest of the higher up code so it might take a while.

Actually, never mind, this was way easier then I expected. With this extra commit eclean-kernel now asks if I want to remove the type 1 entry, and then asks again if I want to remove the type 2 entry.

[AndrewAmmerlaan]

And now also fixed the styling and type hinting complaints (80 characters per line is very short though IMO).

[mgorny]

Thanks a lot!

[AndrewAmmerlaan]

🥳 Thanks for merging, and thanks for all the feedback!