From 3da3049bef08f0d3e48fbbe611f61b358ef04c31 Mon Sep 17 00:00:00 2001 From: Julian Stecklina Date: Fri, 20 Oct 2023 11:29:00 +0200 Subject: [PATCH 1/2] tool: remove unhelpful wrappers and lightly refactor --- rust/tool/shared/src/generation.rs | 40 ++++++++++++++---------------- rust/tool/systemd/src/install.rs | 4 +-- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/rust/tool/shared/src/generation.rs b/rust/tool/shared/src/generation.rs index 3edf9cc..7cd57d5 100644 --- a/rust/tool/shared/src/generation.rs +++ b/rust/tool/shared/src/generation.rs @@ -26,14 +26,14 @@ pub struct ExtendedBootJson { /// contains most of the information necessary to install the generation onto the EFI System /// Partition. The only information missing is the version number which is encoded in the file name /// of the generation link. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct Generation { /// Profile symlink index - version: u64, + pub version: u64, /// Build time - build_time: Option, + pub build_time: Option, /// Top-level specialisation name - specialisation_name: Option, + pub specialisation_name: Option, /// Top-level extended boot specification pub spec: ExtendedBootJson, } @@ -57,19 +57,14 @@ impl Generation { }) } - pub fn specialise(&self, name: &SpecialisationName, bootspec: &BootSpec) -> Result { - Ok(Self { - version: self.version, - build_time: self.build_time, + pub fn specialise(&self, name: &SpecialisationName, bootspec: &BootSpec) -> Self { + Self { specialisation_name: Some(name.clone()), spec: ExtendedBootJson { bootspec: bootspec.clone(), }, - }) - } - - pub fn is_specialised(&self) -> Option { - self.specialisation_name.clone() + ..self.clone() + } } /// Describe the generation in a single line. @@ -84,14 +79,17 @@ impl Generation { .build_time .map(|x| x.to_string()) .unwrap_or_else(|| String::from("Unknown")); - if self.is_specialised().is_some() { - format!( - "Generation {}-specialised, Built on {}", - self.version, build_time - ) - } else { - format!("Generation {}, Built on {}", self.version, build_time) - } + + format!( + "Generation {}{}, {}", + self.version, + if let Some(specialization) = &self.specialisation_name { + format!("-{specialization}") + } else { + "".to_string() + }, + build_time + ) } } diff --git a/rust/tool/systemd/src/install.rs b/rust/tool/systemd/src/install.rs index 2db01a7..d59d377 100644 --- a/rust/tool/systemd/src/install.rs +++ b/rust/tool/systemd/src/install.rs @@ -163,7 +163,7 @@ impl Installer { self.install_generation(&generation) .context("Failed to install generation.")?; for (name, bootspec) in &generation.spec.bootspec.specialisations { - let specialised_generation = generation.specialise(name, bootspec)?; + let specialised_generation = generation.specialise(name, bootspec); self.install_generation(&specialised_generation) .context("Failed to install specialisation.")?; } @@ -374,7 +374,7 @@ fn stub_name(generation: &Generation, public_key: &Path) -> Result { let stub_input_hash = Base32Unpadded::encode_string(&Sha256::digest( serde_json::to_string(&stub_inputs).unwrap(), )); - if let Some(specialisation_name) = generation.is_specialised() { + if let Some(specialisation_name) = &generation.specialisation_name { Ok(PathBuf::from(format!( "nixos-generation-{}-specialisation-{}-{}.efi", generation, specialisation_name, stub_input_hash From ec05d707f3d1a3e1a1540b8dab8abfeb171f25c8 Mon Sep 17 00:00:00 2001 From: Julian Stecklina Date: Fri, 20 Oct 2023 11:29:28 +0200 Subject: [PATCH 2/2] tool: always include version in PRETTY_NAME ... to give a consistent user experience in systemd-boot. Fixes #220. --- rust/tool/shared/src/generation.rs | 22 ++++++++++++++++------ rust/tool/shared/src/os_release.rs | 13 ++++++++++++- rust/tool/systemd/tests/os_release.rs | 4 ++-- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/rust/tool/shared/src/generation.rs b/rust/tool/shared/src/generation.rs index 7cd57d5..192d900 100644 --- a/rust/tool/shared/src/generation.rs +++ b/rust/tool/shared/src/generation.rs @@ -67,7 +67,16 @@ impl Generation { } } - /// Describe the generation in a single line. + /// A helper for describe functions below. + fn describe_specialisation(&self) -> String { + if let Some(specialization) = &self.specialisation_name { + format!("-{specialization}") + } else { + "".to_string() + } + } + + /// Describe the generation in a single line for humans. /// /// Emulates how NixOS's current systemd-boot-builder.py describes generations so that the user /// interface remains similar. @@ -83,14 +92,15 @@ impl Generation { format!( "Generation {}{}, {}", self.version, - if let Some(specialization) = &self.specialisation_name { - format!("-{specialization}") - } else { - "".to_string() - }, + self.describe_specialisation(), build_time ) } + + /// A unique short identifier. + pub fn version_tag(&self) -> String { + format!("{}{}", self.version, self.describe_specialisation(),) + } } impl fmt::Display for Generation { diff --git a/rust/tool/shared/src/os_release.rs b/rust/tool/shared/src/os_release.rs index cdee3dc..9872bff 100644 --- a/rust/tool/shared/src/os_release.rs +++ b/rust/tool/shared/src/os_release.rs @@ -26,10 +26,21 @@ impl OsRelease { // Because the ID field here does not have the same meaning as in a real os-release file, // it is fine to use a dummy value. map.insert("ID".into(), String::from("lanza")); + + // systemd-boot will only show VERSION_ID when PRETTY_NAME is not unique. This is + // confusing to users. Make sure that our PRETTY_NAME is unique, so we get a consistent + // user experience. + // + // See #220. map.insert( "PRETTY_NAME".into(), - generation.spec.bootspec.bootspec.label.clone(), + format!( + "{} ({})", + generation.spec.bootspec.bootspec.label, + generation.describe() + ), ); + map.insert("VERSION_ID".into(), generation.describe()); Ok(Self(map)) diff --git a/rust/tool/systemd/tests/os_release.rs b/rust/tool/systemd/tests/os_release.rs index eb1f627..50c1986 100644 --- a/rust/tool/systemd/tests/os_release.rs +++ b/rust/tool/systemd/tests/os_release.rs @@ -27,8 +27,8 @@ fn generate_expected_os_release() -> Result<()> { let expected = expect![[r#" ID=lanza - PRETTY_NAME=LanzaOS - VERSION_ID=Generation 1, Built on 1970-01-01 + PRETTY_NAME=LanzaOS (Generation 1, 1970-01-01) + VERSION_ID=Generation 1, 1970-01-01 "#]]; expected.assert_eq(&String::from_utf8(os_release_section)?);