Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFE: allow systemd-boot to process unified kernels with os-release data that lacks any version info #20820

Closed
mbiebl opened this issue Sep 23, 2021 · 11 comments · Fixed by #21364
Labels
RFE 🎁 Request for Enhancement, i.e. a feature request sd-boot/sd-stub/bootctl

Comments

@mbiebl
Copy link
Contributor

mbiebl commented Sep 23, 2021

Originally filed as downstream bug report https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=993292

For some reason (which the Debian base-files maintainer does not mention), the base-files package in Debian dropped the VERSION, VERSION_ID and CODENAME field.

This apparently broke systemd-boot, which relies on that information:
Quoting from the downstream bug report

The upgrade of base-files from 11.1 to 12 broke the use of EFI Unified 
Kernel Images[1] with sd-boot. Sd-boot depends on at least one of 
VERSION, VERSION_ID, or CODENAME being set[2] by dracut in the embedded
copy of (/etc|/usr/lib)/os-release to detect the file as a valid image.

Would it be possible to set at least one of those?

Regards,


Victor Westerhuis

[1]: 
https://systemd.io/BOOT_LOADER_SPECIFICATION/#type-2-efi-unified-kernel-images
[2]: 
https://sources.debian.org/src/systemd/247.9-1/src/boot/efi/boot.c/#L1985

The base-files maintainer claims this to be a bug in systemd-boot, saying

This is
*not* a bug in base-files. The standard says

VERSION
  This field is optional.
VERSION_ID
  This field is optional.
VERSION_CODENAME
  This field is optional and may not be implemented on all systems.

Source: https://www.freedesktop.org/software/systemd/man/os-release.html

Therefore any program unconditionally assuming that any of those exist
is buggy and should be fixed.

So, either we make those fields mandatory or we adjust sd-boot to deal with their absence more gracefully.

@bluca
Copy link
Member

bluca commented Sep 23, 2021

A bit strange not to set any of those - but easy to fix, want to send a PR? https://github.com/systemd/systemd/blob/main/src/boot/efi/boot.c#L2076

@poettering
Copy link
Member

We use the version to order boot menu entries. if we have nothing then we can't sort things. What do you expect us to do in this case?

That these things are optional in the spec is one thing. sd-boot makes stronger requirements than that though.

@poettering
Copy link
Member

The boot loader spec even says VERSION_ID is required in unified images.

@mbiebl
Copy link
Contributor Author

mbiebl commented Sep 23, 2021

The boot loader spec even says VERSION_ID is required in unified images.

so we have conflicting specs?

@mbiebl
Copy link
Contributor Author

mbiebl commented Sep 23, 2021

A bit strange not to set any of those - but easy to fix, want to send a PR? https://github.com/systemd/systemd/blob/main/src/boot/efi/boot.c#L2076

Personally, I don't really care. If anyone feels like convincing the base-files maintainer to add those fields, please go ahead.

@bluca
Copy link
Member

bluca commented Sep 23, 2021

The boot loader spec even says VERSION_ID is required in unified images.

so we have conflicting specs?

They are not conflicting, one is a subset of the other. Ie, the number of images using systemd is greater than the number of images using systemd + systemd-boot, hence it makes sense for the general-purpose spec to be more relaxed, and for the systemd-boot-related one to add additional requirements.

@mbiebl
Copy link
Contributor Author

mbiebl commented Sep 23, 2021

Well, they are. One spec says the fields are mandatory, the other says they aren't. So they contradict each other. It's quite simple really.

As said, I personally don't care. If you think the current behaviour is correct, feel free to convince the base-files maintainer. I don't want to get into this fight.

@mbiebl
Copy link
Contributor Author

mbiebl commented Sep 23, 2021

afaics, the other alternative is to disable gnu-efi support in Debian, given that it is currently broken anyway.

@poettering
Copy link
Member

the specs aren't really conflicting. One just requires a bit more. I mean, that's how specs work: some build on other specs but make stricter requirements. there's nothing offensive or wrong with doing so.

I am not against allowing images without any form of version to work, I am just not sure what the right strategy for this is, as we then cannot order boot entries anymore.

@poettering poettering added the RFE 🎁 Request for Enhancement, i.e. a feature request label Sep 23, 2021
@poettering poettering changed the title systemd-boot does not handle missing VERSION, VERSION_ID and CODENAME RFE: allow systemd-boot to process unified kernels with os-release data that lacks any version info Sep 23, 2021
@poettering
Copy link
Member

so looking at the sources I think a fix for this is probably pretty easy: the code doesn't actually use the version for sorting like i claimed earlier, it's purely for presentation purposes (we order by filename where the entry is defined in). And we are fine with entries that have no versions in the rest of the code, so it should suffice to just drop the check that requires the field to exist. happy to take a patch for that.

@viccie30
Copy link
Contributor

Since this is my downstream bug report in Debian, I'll see if I can write a patch and submit it as a pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFE 🎁 Request for Enhancement, i.e. a feature request sd-boot/sd-stub/bootctl
Development

Successfully merging a pull request may close this issue.

4 participants