From bc4478b0b9cec30e0b8d643ac942ba3f1473a166 Mon Sep 17 00:00:00 2001 From: Aiden McClelland Date: Tue, 17 Feb 2026 14:12:14 -0700 Subject: [PATCH] refactor: manifest wraps PackageMetadata, move dependency_metadata to PackageVersionInfo Manifest now embeds PackageMetadata via #[serde(flatten)] instead of duplicating ~14 fields. icon and dependency_metadata moved from PackageMetadata to PackageVersionInfo since they are registry-enrichment data loaded from the S9PK archive. merge_with now returns errors on metadata/icon/dependency_metadata mismatches instead of silently ignoring them. --- core/locales/i18n.yaml | 22 ++++ core/src/backup/backup_bulk.rs | 4 +- core/src/registry/package/get.rs | 11 +- core/src/registry/package/index.rs | 122 ++++++++++++----------- core/src/s9pk/v2/compat.rs | 40 ++++---- core/src/s9pk/v2/manifest.rs | 35 +------ core/src/s9pk/v2/pack.rs | 4 +- core/src/service/persistent_container.rs | 2 +- 8 files changed, 122 insertions(+), 118 deletions(-) diff --git a/core/locales/i18n.yaml b/core/locales/i18n.yaml index 7f6cd150e..335d1f56c 100644 --- a/core/locales/i18n.yaml +++ b/core/locales/i18n.yaml @@ -1790,6 +1790,28 @@ registry.package.remove-mirror.unauthorized: fr_FR: "Non autorisé" pl_PL: "Brak autoryzacji" +# registry/package/index.rs +registry.package.index.metadata-mismatch: + en_US: "package metadata mismatch: remove the existing version first, then re-add" + de_DE: "Paketmetadaten stimmen nicht überein: vorhandene Version zuerst entfernen, dann erneut hinzufügen" + es_ES: "discrepancia de metadatos del paquete: elimine la versión existente primero, luego vuelva a agregarla" + fr_FR: "discordance des métadonnées du paquet : supprimez d'abord la version existante, puis ajoutez-la à nouveau" + pl_PL: "niezgodność metadanych pakietu: najpierw usuń istniejącą wersję, a następnie dodaj ponownie" + +registry.package.index.icon-mismatch: + en_US: "package icon mismatch: remove the existing version first, then re-add" + de_DE: "Paketsymbol stimmt nicht überein: vorhandene Version zuerst entfernen, dann erneut hinzufügen" + es_ES: "discrepancia del icono del paquete: elimine la versión existente primero, luego vuelva a agregarla" + fr_FR: "discordance de l'icône du paquet : supprimez d'abord la version existante, puis ajoutez-la à nouveau" + pl_PL: "niezgodność ikony pakietu: najpierw usuń istniejącą wersję, a następnie dodaj ponownie" + +registry.package.index.dependency-metadata-mismatch: + en_US: "dependency metadata mismatch: remove the existing version first, then re-add" + de_DE: "Abhängigkeitsmetadaten stimmen nicht überein: vorhandene Version zuerst entfernen, dann erneut hinzufügen" + es_ES: "discrepancia de metadatos de dependencia: elimine la versión existente primero, luego vuelva a agregarla" + fr_FR: "discordance des métadonnées de dépendance : supprimez d'abord la version existante, puis ajoutez-la à nouveau" + pl_PL: "niezgodność metadanych zależności: najpierw usuń istniejącą wersję, a następnie dodaj ponownie" + # registry/package/get.rs registry.package.get.version-not-found: en_US: "Could not find a version of %{id} that satisfies %{version}" diff --git a/core/src/backup/backup_bulk.rs b/core/src/backup/backup_bulk.rs index 0f48252fa..0acc385ee 100644 --- a/core/src/backup/backup_bulk.rs +++ b/core/src/backup/backup_bulk.rs @@ -271,9 +271,9 @@ async fn perform_backup( package_backups.insert( id.clone(), PackageBackupInfo { - os_version: manifest.as_os_version().de()?, + os_version: manifest.as_metadata().as_os_version().de()?, version: manifest.as_version().de()?, - title: manifest.as_title().de()?, + title: manifest.as_metadata().as_title().de()?, timestamp: Utc::now(), }, ); diff --git a/core/src/registry/package/get.rs b/core/src/registry/package/get.rs index 2d834bf87..8e270d285 100644 --- a/core/src/registry/package/get.rs +++ b/core/src/registry/package/get.rs @@ -586,7 +586,6 @@ fn check_matching_info_short() { let info = PackageVersionInfo { metadata: PackageMetadata { title: "Test Package".into(), - icon: DataUrl::from_vec("image/png", vec![]), description: Description { short: lang_map("A short description"), long: lang_map("A longer description of the test package"), @@ -594,18 +593,18 @@ fn check_matching_info_short() { release_notes: lang_map("Initial release"), git_hash: None, license: "MIT".into(), - wrapper_repo: "https://github.com/example/wrapper".parse().unwrap(), + package_repo: "https://github.com/example/wrapper".parse().unwrap(), upstream_repo: "https://github.com/example/upstream".parse().unwrap(), - support_site: "https://example.com/support".parse().unwrap(), - marketing_site: "https://example.com".parse().unwrap(), + marketing_url: Some("https://example.com".parse().unwrap()), donation_url: None, - docs_url: None, + docs_urls: Vec::new(), alerts: Alerts::default(), - dependency_metadata: BTreeMap::new(), os_version: exver::Version::new([0, 3, 6], []), sdk_version: None, hardware_acceleration: false, }, + icon: DataUrl::from_vec("image/png", vec![]), + dependency_metadata: BTreeMap::new(), source_version: None, s9pks: Vec::new(), }; diff --git a/core/src/registry/package/index.rs b/core/src/registry/package/index.rs index 64b83d5e4..40e05c326 100644 --- a/core/src/registry/package/index.rs +++ b/core/src/registry/package/index.rs @@ -17,7 +17,9 @@ use crate::registry::device_info::DeviceInfo; use crate::rpc_continuations::Guid; use crate::s9pk::S9pk; use crate::s9pk::git_hash::GitHash; -use crate::s9pk::manifest::{Alerts, Description, HardwareRequirements, LocaleString}; +use crate::s9pk::manifest::{ + Alerts, Description, HardwareRequirements, LocaleString, current_version, +}; use crate::s9pk::merkle_archive::source::FileSource; use crate::sign::commitment::merkle_archive::MerkleArchiveCommitment; use crate::sign::{AnySignature, AnyVerifyingKey}; @@ -69,32 +71,36 @@ impl DependencyMetadata { } } -#[derive(Debug, Deserialize, Serialize, HasModel, TS, PartialEq)] +fn placeholder_url() -> Url { + "https://example.com".parse().unwrap() +} + +#[derive(Clone, Debug, Deserialize, Serialize, HasModel, TS, PartialEq)] #[serde(rename_all = "camelCase")] #[model = "Model"] pub struct PackageMetadata { #[ts(type = "string")] pub title: InternedString, - pub icon: DataUrl<'static>, pub description: Description, pub release_notes: LocaleString, pub git_hash: Option, #[ts(type = "string")] pub license: InternedString, #[ts(type = "string")] - pub wrapper_repo: Url, + #[serde(default = "placeholder_url")] // TODO: remove + pub package_repo: Url, #[ts(type = "string")] pub upstream_repo: Url, #[ts(type = "string")] - pub support_site: Url, - #[ts(type = "string")] - pub marketing_site: Url, + pub marketing_url: Option, #[ts(type = "string | null")] pub donation_url: Option, - #[ts(type = "string | null")] - pub docs_url: Option, + #[serde(default)] + #[ts(type = "string[]")] + pub docs_urls: Vec, + #[serde(default)] pub alerts: Alerts, - pub dependency_metadata: BTreeMap, + #[serde(default = "current_version")] #[ts(type = "string")] pub os_version: Version, #[ts(type = "string | null")] @@ -102,43 +108,6 @@ pub struct PackageMetadata { #[serde(default)] pub hardware_acceleration: bool, } -impl PackageMetadata { - pub async fn load(s9pk: &S9pk) -> Result { - let manifest = s9pk.as_manifest(); - let mut dependency_metadata = BTreeMap::new(); - for (id, info) in &manifest.dependencies.0 { - let metadata = s9pk.dependency_metadata(id).await?; - dependency_metadata.insert( - id.clone(), - DependencyMetadata { - title: metadata.map(|m| m.title), - icon: s9pk.dependency_icon_data_url(id).await?, - description: info.description.clone(), - optional: info.optional, - }, - ); - } - Ok(Self { - title: manifest.title.clone(), - icon: s9pk.icon_data_url().await?, - description: manifest.description.clone(), - release_notes: manifest.release_notes.clone(), - git_hash: manifest.git_hash.clone(), - license: manifest.license.clone(), - wrapper_repo: manifest.wrapper_repo.clone(), - upstream_repo: manifest.upstream_repo.clone(), - support_site: manifest.support_site.clone(), - marketing_site: manifest.marketing_site.clone(), - donation_url: manifest.donation_url.clone(), - docs_url: manifest.docs_url.clone(), - alerts: manifest.alerts.clone(), - dependency_metadata, - os_version: manifest.os_version.clone(), - sdk_version: manifest.sdk_version.clone(), - hardware_acceleration: manifest.hardware_acceleration.clone(), - }) - } -} #[derive(Debug, Deserialize, Serialize, HasModel, TS)] #[serde(rename_all = "camelCase")] @@ -147,6 +116,8 @@ impl PackageMetadata { pub struct PackageVersionInfo { #[serde(flatten)] pub metadata: PackageMetadata, + pub icon: DataUrl<'static>, + pub dependency_metadata: BTreeMap, #[ts(type = "string | null")] pub source_version: Option, pub s9pks: Vec<(HardwareRequirements, RegistryAsset)>, @@ -156,11 +127,28 @@ impl PackageVersionInfo { s9pk: &S9pk, urls: Vec, ) -> Result { + let manifest = s9pk.as_manifest(); + let icon = s9pk.icon_data_url().await?; + let mut dependency_metadata = BTreeMap::new(); + for (id, info) in &manifest.dependencies.0 { + let dep_meta = s9pk.dependency_metadata(id).await?; + dependency_metadata.insert( + id.clone(), + DependencyMetadata { + title: dep_meta.map(|m| m.title), + icon: s9pk.dependency_icon_data_url(id).await?, + description: info.description.clone(), + optional: info.optional, + }, + ); + } Ok(Self { - metadata: PackageMetadata::load(s9pk).await?, + metadata: manifest.metadata.clone(), + icon, + dependency_metadata, source_version: None, // TODO s9pks: vec![( - s9pk.as_manifest().hardware_requirements.clone(), + manifest.hardware_requirements.clone(), RegistryAsset { published_at: Utc::now(), urls, @@ -176,6 +164,27 @@ impl PackageVersionInfo { }) } pub fn merge_with(&mut self, other: Self, replace_urls: bool) -> Result<(), Error> { + if self.metadata != other.metadata { + return Err(Error::new( + color_eyre::eyre::eyre!("{}", t!("registry.package.index.metadata-mismatch")), + ErrorKind::InvalidRequest, + )); + } + if self.icon != other.icon { + return Err(Error::new( + color_eyre::eyre::eyre!("{}", t!("registry.package.index.icon-mismatch")), + ErrorKind::InvalidRequest, + )); + } + if self.dependency_metadata != other.dependency_metadata { + return Err(Error::new( + color_eyre::eyre::eyre!( + "{}", + t!("registry.package.index.dependency-metadata-mismatch") + ), + ErrorKind::InvalidRequest, + )); + } for (hw_req, asset) in other.s9pks { if let Some((_, matching)) = self .s9pks @@ -221,10 +230,9 @@ impl PackageVersionInfo { ]); table.add_row(row![br -> "GIT HASH", self.metadata.git_hash.as_deref().unwrap_or("N/A")]); table.add_row(row![br -> "LICENSE", &self.metadata.license]); - table.add_row(row![br -> "PACKAGE REPO", &self.metadata.wrapper_repo.to_string()]); + table.add_row(row![br -> "PACKAGE REPO", &self.metadata.package_repo.to_string()]); table.add_row(row![br -> "SERVICE REPO", &self.metadata.upstream_repo.to_string()]); - table.add_row(row![br -> "WEBSITE", &self.metadata.marketing_site.to_string()]); - table.add_row(row![br -> "SUPPORT", &self.metadata.support_site.to_string()]); + table.add_row(row![br -> "WEBSITE", self.metadata.marketing_url.as_ref().map_or("N/A".to_owned(), |u| u.to_string())]); table } @@ -287,19 +295,17 @@ impl Model { } if let Some(locale) = device_info.os.language.as_deref() { - let metadata = self.as_metadata_mut(); - metadata + self.as_metadata_mut() .as_alerts_mut() .mutate(|a| Ok(a.localize_for(locale)))?; - metadata - .as_dependency_metadata_mut() + self.as_dependency_metadata_mut() .as_entries_mut()? .into_iter() .try_for_each(|(_, d)| d.mutate(|d| Ok(d.localize_for(locale))))?; - metadata + self.as_metadata_mut() .as_description_mut() .mutate(|d| Ok(d.localize_for(locale)))?; - metadata + self.as_metadata_mut() .as_release_notes_mut() .mutate(|r| Ok(r.localize_for(locale)))?; } diff --git a/core/src/s9pk/v2/compat.rs b/core/src/s9pk/v2/compat.rs index 837632fff..592e213ca 100644 --- a/core/src/s9pk/v2/compat.rs +++ b/core/src/s9pk/v2/compat.rs @@ -9,6 +9,7 @@ use tokio::process::Command; use crate::dependencies::{DepInfo, Dependencies}; use crate::prelude::*; +use crate::registry::package::index::PackageMetadata; use crate::s9pk::manifest::{DeviceFilter, LocaleString, Manifest}; use crate::s9pk::merkle_archive::directory_contents::DirectoryContents; use crate::s9pk::merkle_archive::source::TmpSource; @@ -195,20 +196,31 @@ impl TryFrom for Manifest { } Ok(Self { id: value.id, - title: format!("{} (Legacy)", value.title).into(), version: version.into(), satisfies: BTreeSet::new(), - release_notes: LocaleString::Translated(value.release_notes), can_migrate_from: VersionRange::any(), can_migrate_to: VersionRange::none(), - license: value.license.into(), - wrapper_repo: value.wrapper_repo, - upstream_repo: value.upstream_repo, - support_site: value.support_site.unwrap_or_else(|| default_url.clone()), - marketing_site: value.marketing_site.unwrap_or_else(|| default_url.clone()), - donation_url: value.donation_url, - docs_url: None, - description: value.description, + metadata: PackageMetadata { + title: format!("{} (Legacy)", value.title).into(), + release_notes: LocaleString::Translated(value.release_notes), + license: value.license.into(), + package_repo: value.wrapper_repo, + upstream_repo: value.upstream_repo, + marketing_url: Some( + value.marketing_site.unwrap_or_else(|| default_url.clone()), + ), + donation_url: value.donation_url, + docs_urls: Vec::new(), + description: value.description, + alerts: value.alerts, + git_hash: value.git_hash, + os_version: value.eos_version, + sdk_version: None, + hardware_acceleration: match value.main { + PackageProcedure::Docker(d) => d.gpu_acceleration, + PackageProcedure::Script(_) => false, + }, + }, images: BTreeMap::new(), volumes: value .volumes @@ -217,7 +229,6 @@ impl TryFrom for Manifest { .map(|(id, _)| id.clone()) .chain([VolumeId::from_str("embassy").unwrap()]) .collect(), - alerts: value.alerts, dependencies: Dependencies( value .dependencies @@ -252,13 +263,6 @@ impl TryFrom for Manifest { }) .collect(), }, - git_hash: value.git_hash, - os_version: value.eos_version, - sdk_version: None, - hardware_acceleration: match value.main { - PackageProcedure::Docker(d) => d.gpu_acceleration, - PackageProcedure::Script(_) => false, - }, }) } } diff --git a/core/src/s9pk/v2/manifest.rs b/core/src/s9pk/v2/manifest.rs index 164c91cb2..39361e341 100644 --- a/core/src/s9pk/v2/manifest.rs +++ b/core/src/s9pk/v2/manifest.rs @@ -7,12 +7,11 @@ use exver::{Version, VersionRange}; use imbl_value::{InOMap, InternedString}; use serde::{Deserialize, Serialize}; use ts_rs::TS; -use url::Url; pub use crate::PackageId; use crate::dependencies::Dependencies; use crate::prelude::*; -use crate::s9pk::git_hash::GitHash; +use crate::registry::package::index::PackageMetadata; use crate::s9pk::merkle_archive::directory_contents::DirectoryContents; use crate::s9pk::merkle_archive::expected::{Expected, Filter}; use crate::s9pk::v2::pack::ImageConfig; @@ -22,7 +21,7 @@ use crate::util::{FromStrParser, VersionString, mime}; use crate::version::{Current, VersionT}; use crate::{ImageId, VolumeId}; -fn current_version() -> Version { +pub(crate) fn current_version() -> Version { Current::default().semver() } @@ -32,46 +31,20 @@ fn current_version() -> Version { #[ts(export)] pub struct Manifest { pub id: PackageId, - #[ts(type = "string")] - pub title: InternedString, pub version: VersionString, pub satisfies: BTreeSet, - pub release_notes: LocaleString, #[ts(type = "string")] pub can_migrate_to: VersionRange, #[ts(type = "string")] pub can_migrate_from: VersionRange, - #[ts(type = "string")] - pub license: InternedString, // type of license - #[ts(type = "string")] - pub wrapper_repo: Url, - #[ts(type = "string")] - pub upstream_repo: Url, - #[ts(type = "string")] - pub support_site: Url, - #[ts(type = "string")] - pub marketing_site: Url, - #[ts(type = "string | null")] - pub donation_url: Option, - #[ts(type = "string | null")] - pub docs_url: Option, - pub description: Description, + #[serde(flatten)] + pub metadata: PackageMetadata, pub images: BTreeMap, pub volumes: BTreeSet, #[serde(default)] - pub alerts: Alerts, - #[serde(default)] pub dependencies: Dependencies, #[serde(default)] pub hardware_requirements: HardwareRequirements, - #[serde(default)] - pub hardware_acceleration: bool, - pub git_hash: Option, - #[serde(default = "current_version")] - #[ts(type = "string")] - pub os_version: Version, - #[ts(type = "string | null")] - pub sdk_version: Option, } impl Manifest { pub fn validate_for<'a, T: Clone>( diff --git a/core/src/s9pk/v2/pack.rs b/core/src/s9pk/v2/pack.rs index 667241eca..cffd32d3d 100644 --- a/core/src/s9pk/v2/pack.rs +++ b/core/src/s9pk/v2/pack.rs @@ -685,7 +685,7 @@ pub async fn pack(ctx: CliContext, params: PackParams) -> Result<(), Error> { .await?; let manifest = s9pk.as_manifest_mut(); - manifest.git_hash = Some(GitHash::from_path(params.path()).await?); + manifest.metadata.git_hash = Some(GitHash::from_path(params.path()).await?); if !params.arch.is_empty() { let arches: BTreeSet = match manifest.hardware_requirements.arch.take() { Some(a) => params @@ -792,7 +792,7 @@ pub async fn pack(ctx: CliContext, params: PackParams) -> Result<(), Error> { } }; Some(( - LocaleString::Translated(s9pk.as_manifest().title.to_string()), + LocaleString::Translated(s9pk.as_manifest().metadata.title.to_string()), s9pk.icon_data_url().await?, )) } diff --git a/core/src/service/persistent_container.rs b/core/src/service/persistent_container.rs index 7c73f7bf0..6bc52b4db 100644 --- a/core/src/service/persistent_container.rs +++ b/core/src/service/persistent_container.rs @@ -97,7 +97,7 @@ impl PersistentContainer { .join(&s9pk.as_manifest().id), ), LxcConfig { - hardware_acceleration: s9pk.manifest.hardware_acceleration, + hardware_acceleration: s9pk.manifest.metadata.hardware_acceleration, }, ) .await?;