diff --git a/kernels-data/src/metadata.rs b/kernels-data/src/metadata.rs index 297635991..0eb5a0449 100644 --- a/kernels-data/src/metadata.rs +++ b/kernels-data/src/metadata.rs @@ -1,11 +1,11 @@ use std::str::FromStr; use eyre::Result; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use crate::config::{Backend, Build, GitUrl, KernelDependency, KernelName}; use crate::digest::Digest; -use crate::git::GitStatus; +use crate::git::{GitStatus, Oid}; #[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] @@ -16,9 +16,27 @@ pub struct BackendInfo { pub archs: Option>, } +fn deserialize_present<'de, D, T>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, + T: Deserialize<'de>, +{ + T::deserialize(deserializer).map(Some) +} + +// Use a repr type because a flattened `Option` silently drops invalid fields. +#[derive(Deserialize)] +struct KernelBuilderVersionRepr { + version: String, + #[serde(default, alias = "sha", deserialize_with = "deserialize_present")] + commit: Option, + #[serde(default, deserialize_with = "deserialize_present")] + dirty: Option, +} + /// Provenance of the `kernel-builder` that produced a build. #[derive(Clone, Debug, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] +#[serde(rename_all = "kebab-case", try_from = "KernelBuilderVersionRepr")] pub struct KernelBuilderVersion { pub version: String, /// Git state of the `kernel-builder` source, when known. @@ -29,6 +47,23 @@ pub struct KernelBuilderVersion { pub git: Option, } +impl TryFrom for KernelBuilderVersion { + type Error = &'static str; + + fn try_from(repr: KernelBuilderVersionRepr) -> Result { + let git = match (repr.commit, repr.dirty) { + (None, None) => None, + (Some(commit), Some(dirty)) => Some(GitStatus { commit, dirty }), + _ => return Err("git commit and dirty must be specified together"), + }; + + Ok(Self { + version: repr.version, + git, + }) + } +} + /// Provenance of a kernel build: the git state of the `kernel-builder` and of /// the kernel source it was built from. #[derive(Clone, Debug, Deserialize, Serialize)] @@ -433,19 +468,27 @@ mod tests { } #[test] - fn unreadable_kernel_builder_git_is_dropped() { - // `KernelBuilderVersion::git` is flattened, and serde deserializes a - // flattened `Option` by discarding the error rather than propagating - // it. So an unreadable `kernel-builder` git status drops that status - // instead of rejecting the metadata. + fn unreadable_kernel_builder_git_is_an_error() { + // Invalid legacy `sha` field should be rejected rather than dropped. let mut value: serde_json::Value = serde_json::from_str(LEGACY_HUB_METADATA).unwrap(); value["provenance"]["kernel-builder"]["sha"] = serde_json::json!("not-an-oid"); - let metadata: Metadata = serde_json::from_value(value).unwrap(); - let provenance = metadata.provenance.expect("provenance should be read"); - assert!(provenance.kernel_builder.git.is_none()); - assert_eq!(provenance.kernel_builder.version, "0.17.0-dev0"); - assert!(provenance.kernel.is_some()); + assert!(serde_json::from_value::(value).is_err()); + } + + #[test] + fn invalid_kernel_builder_commit_is_rejected_instead_of_dropped() { + let json = r#"{ + "version": "0.1.0", + "commit": "not-an-oid", + "dirty": true + }"#; + + // This previously became `KernelBuilderVersion { version: "0.1.0", git: None }`, hiding `dirty: true`. + let error = serde_json::from_str::(json) + .expect_err("invalid git provenance should be rejected"); + + assert!(error.to_string().contains("Invalid git object id")); } #[test]