efi: sync bootentry label with Anaconda #685
Conversation
Read `/etc/system-release` if exists, else get `NAME` from `/etc/os-release` See https://bugzilla.redhat.com/show_bug.cgi?id=2268505#c24
HuijingHei
force-pushed
the
anaconda-label
branch
from
9e5a333
to
47557c3
Compare
| @@ -659,4 +670,10 @@ Boot0003* test"; | |||
| ); | |||
| Ok(()) | |||
| } | |||
| #[test] | |||
| fn test_get_product_name() -> Result<()> { | |||
| let name = get_product_name()?; | |||
There was a problem hiding this comment.
I think we shouldn't read global state in a unit test. This will fail on e.g. Debian or other systems and we want to support those.
We can unit test parsing from a &str easily enough though.
(That said of course, we could use openat or better cap-std to load from a test root...see what we do in https://github.com/containers/bootc/blob/c866bba1e96e912ab2e36b9eec3985e93e7a0c9b/lib/src/generator.rs#L90 - but unfortunately OsRelease doesn't support that yet)
There was a problem hiding this comment.
This will fail on e.g. Debian or other systems and we want to support those.
I was wrong, it shouldn't because the system-release is optional.
However, it does still make sense to avoid reading global state longer term.
There was a problem hiding this comment.
Thanks! Yes, you are correct, will switch to os-release if not exist system-release.
| } | ||
| } | ||
|
|
||
| #[context("Get product name")] | ||
| fn get_product_name() -> Result<String> { |
There was a problem hiding this comment.
Should this change to fn get_product_name(root: &Dir) to pass root dir as parameter?
There was a problem hiding this comment.
Yeah, but not a blocker, just a nice to have IMO.
There was a problem hiding this comment.
Thanks, will update in the later PR.
Read
/etc/system-releaseif exists, else getNAMEfrom/etc/os-releaseSee https://bugzilla.redhat.com/show_bug.cgi?id=2268505#c24