From 4af5e47241eebd15b67e7dd431748bd1e0fafcf2 Mon Sep 17 00:00:00 2001 From: Joe Neeman Date: Thu, 25 Jul 2024 15:31:11 +0700 Subject: [PATCH] Use deserialize to simplify manifest construction --- core/stdlib/std.ncl | 52 ++++++++++-- package/src/lib.rs | 3 +- package/src/manifest.rs | 179 ++++++++++------------------------------ 3 files changed, 93 insertions(+), 141 deletions(-) diff --git a/core/stdlib/std.ncl b/core/stdlib/std.ncl index 4a176a81b0..5f611faccb 100644 --- a/core/stdlib/std.ncl +++ b/core/stdlib/std.ncl @@ -2582,18 +2582,42 @@ = fun x n => %pow% x n, }, - package = { + package = + let + # https://semver.org is kind enough to supply this "official" semver regex. + semver_re = m%"^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$"% + in + let + # An expression representing a range of semver versions, like `<1.2`. + # Unlike semver itself, semver range expressions don't seem to have an official standard. + semver_main_range_re = m%"(~|=|\^|<|>|<=|>=)?(0|[1-9]\d*)(\.(0|[1-9]\d*))?(\.(0|[1-9]\d*))?"% + in + let + # For requirements with non-empty prereleases, we support exactly two flavors: 0.1.2-pre3 or =0.1.2-pre3. + # We could also support inequalities, but we should avoid ^ and ~ because prereleases have no guaranteed + # compatibility semantics. + semver_prerelease_range_re = m%"=?(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)-((0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(\.(0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*)"% + in + let semver_range_re = "(%{semver_main_range_re})|(%{semver_prerelease_range_re})" in + let semver_req_re = m%"^(%{semver_range_re}(,\s*%{semver_range_re})*)$"% in + { + is_semver_req + : String -> Bool + | doc m%" + Returns true if a string is a valid semantic version requirement. + "% + = std.string.is_match semver_req_re + , is_semver : String -> Bool | doc m%" Returns true if a string is a valid semantic version identifier. "% - # This lovely expression is the officially recommended semver regex from https://semver.org - = std.string.is_match m%"^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$"% + = std.string.is_match semver_re , Semver | doc m%" - A contract that checks whether a string represents a valid semantic version ("semver") identifier. + A contract for semantic version ("semver") identifiers. # Examples @@ -2606,6 +2630,20 @@ ``` "% = std.contract.from_predicate is_semver, + SemverReq + | doc m%" + A contract for semantic version ("semver") requirements. + + # Examples + + ```nickel + "^1.2" | std.package.SemverReq + => "^1.2" + + ">=1.2, <1.4" | std.package.SemverReq + => ">=1.2, <1.4" + "% + = std.contract.from_predicate is_semver_req, Manifest = { name | String @@ -2614,12 +2652,14 @@ "%, version + | String | Semver | doc m%" The version of this package. "%, - nickel-version + nickel_version + | String | Semver | doc m%" The minimal nickel version required for this package. @@ -2630,7 +2670,7 @@ [| 'Path String, 'Git { url | String, }, - 'Index { name | String, version | String | Semver }, + 'Index { name | String, version | String | SemverReq }, |] } | doc m%" diff --git a/package/src/lib.rs b/package/src/lib.rs index 56fd6a6208..d1f53c4a92 100644 --- a/package/src/lib.rs +++ b/package/src/lib.rs @@ -16,10 +16,11 @@ use util::cache_dir; /// A source includes the place to fetch a package from (e.g. git or a registry), /// along with possibly some narrowing-down of the allowed versions (e.g. a range /// of versions, or a git commit id). -#[derive(Clone, Debug, PartialEq, Eq, Hash)] +#[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize)] pub enum Dependency { // TODO: allow targeting branches or revisions, and allow supplying a relative path Git { + #[serde(with = "serde_url")] url: gix::Url, //tree: Option, }, diff --git a/package/src/manifest.rs b/package/src/manifest.rs index a47f3b587b..ecaa448928 100644 --- a/package/src/manifest.rs +++ b/package/src/manifest.rs @@ -3,7 +3,6 @@ use std::{ path::{Path, PathBuf}, }; -use gix::Url; use nickel_lang_core::{ cache::normalize_rel_path, eval::cache::CacheImpl, @@ -24,7 +23,15 @@ use crate::{ Dependency, Precise, }; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Deserialize)] +struct ManifestFileFormat { + pub name: Ident, + pub version: semver::Version, + pub nickel_version: semver::Version, + pub dependencies: HashMap, +} + +#[derive(Clone, Debug, PartialEq)] pub struct ManifestFile { // The directory containing the manifest file. Path deps are resolved relative to this. // If `None`, path deps aren't allowed. @@ -148,142 +155,23 @@ impl ManifestFile { fn from_term(rt: &RichTerm) -> Result { // This is only ever called with terms that have passed the `std.package.Manifest` // contract, so we can assume that they have the right fields. - fn err(s: &str) -> Error { - Error::InternalManifestError { msg: s.to_owned() } - } - - let Term::Record(data) = rt.as_ref() else { - return Err(err("manifest not a record")); - }; - - // FIXME: yuck - let name = data - .fields - .get(&Ident::new("name")) - .ok_or_else(|| err("no name"))? - .value - .as_ref() - .ok_or_else(|| err("name has no value"))?; - let Term::Str(name) = name.as_ref() else { - return Err(err("name not a string")); - }; - - let version = data - .fields - .get(&Ident::new("version")) - .ok_or_else(|| err("no version"))? - .value - .as_ref() - .ok_or_else(|| err("version has no value"))?; - let Term::Str(version) = version.as_ref() else { - return Err(err("version not a string")); - }; - - let nickel_version = data - .fields - .get(&Ident::new("nickel-version")) - .ok_or_else(|| err("no nickel-version"))? - .value - .as_ref() - .ok_or_else(|| err("nickel-version has no value"))?; - let Term::Str(nickel_version) = nickel_version.as_ref() else { - return Err(err("nickel-version not a string")); - }; - - let deps = data - .fields - .get(&Ident::new("dependencies")) - .ok_or_else(|| err("no dependencies"))? - .value - .as_ref() - .ok_or_else(|| err("dependencies has no value"))?; - let Term::Record(deps) = deps.as_ref() else { - return Err(err("dependencies not a record")); - }; - - let mut ret = Self { - dependencies: HashMap::new(), + let ManifestFileFormat { + name, + version, + nickel_version, + dependencies, + } = ManifestFileFormat::deserialize(rt.clone()) + .map_err(|e| Error::InternalManifestError { msg: e.to_string() })?; + Ok(Self { parent_dir: None, - version: version.parse().map_err(|_| err("invalid version"))?, - nickel_version: nickel_version - .parse() - .map_err(|_| err("invalid nickel version"))?, - name: Ident::new(name), - }; - - for (name, dep) in &deps.fields { - let Term::EnumVariant { tag, arg, .. } = dep - .value - .as_ref() - .ok_or_else(|| err("dependency has no value"))? - .as_ref() - else { - return Err(err("dependency not an enum")); - }; - - match tag.ident().label() { - "Git" => { - let Term::Record(data) = arg.as_ref() else { - return Err(err("payload wasn't a record")); - }; - - let url = data - .fields - .get(&Ident::new("url")) - .ok_or_else(|| err("no url"))? - .value - .as_ref() - .ok_or_else(|| err("url has no value"))?; - let Term::Str(url) = url.as_ref() else { - return Err(err("url wasn't a string")); - }; - - ret.dependencies.insert( - name.ident(), - Dependency::Git { - url: Url::try_from(url.to_string()).map_err(|e| Error::InvalidUrl { - url: url.to_string(), - msg: e.to_string(), - })?, - }, - ); - } - "Path" => { - let Term::Str(path) = arg.as_ref() else { - return Err(err("payload wasn't a string")); - }; - - ret.dependencies.insert( - name.ident(), - Dependency::Path { - path: PathBuf::from(path.to_string()), - }, - ); - } - "Index" => { - let payload: IndexPayload = serde_json::from_value( - serde_json::to_value(arg.as_ref()).map_err(|_| err("bad payload"))?, - ) - .map_err(|_| err("bad payload"))?; - - let id: crate::index::Id = payload.name.parse().unwrap(); - let version: semver::VersionReq = payload.version.parse().unwrap(); - ret.dependencies - .insert(name.ident(), Dependency::Index { id, version }); - } - _ => return Err(err("bad tag")), - } - } - Ok(ret) + name, + version, + nickel_version, + dependencies, + }) } } -#[derive(Deserialize)] -struct IndexPayload { - name: String, - version: String, -} - #[derive(Clone, Debug)] pub struct RealizedDependency { /// Either `Git` or `Path`. @@ -408,3 +296,26 @@ impl Realization { Ok(id) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn manifest() { + let manifest = ManifestFile::from_contents( + r#"{name = "foo", version = "1.0.0", nickel_version = "1.8.0"}"#.as_bytes(), + ) + .unwrap(); + assert_eq!( + manifest, + ManifestFile { + parent_dir: None, + name: "foo".into(), + version: semver::Version::new(1, 0, 0), + nickel_version: semver::Version::new(1, 8, 0), + dependencies: HashMap::default() + } + ) + } +}