From 41e3861a495cff5d54ce0cd4f0e5da4841d4011a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 17 Apr 2018 10:48:15 +0300 Subject: [PATCH] Simplify RegistryPackage deserialization --- src/cargo/sources/registry/index.rs | 10 +- src/cargo/sources/registry/mod.rs | 233 ++++++---------------------- 2 files changed, 56 insertions(+), 187 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 74a687b7643..1aef89bed31 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -2,13 +2,12 @@ use std::collections::HashMap; use std::path::Path; use std::str; -use serde::de::DeserializeSeed; use serde_json; use semver::Version; use core::dependency::Dependency; use core::{PackageId, SourceId, Summary}; -use sources::registry::{RegistryPackage, RegistryPackageDefault, INDEX_LOCK}; +use sources::registry::{RegistryPackage, INDEX_LOCK}; use sources::registry::RegistryData; use util::{internal, CargoResult, Config, Filesystem}; @@ -141,7 +140,6 @@ impl<'cfg> RegistryIndex<'cfg> { /// /// The returned boolean is whether or not the summary has been yanked. fn parse_registry_package(&mut self, line: &str) -> CargoResult<(Summary, bool)> { - let mut json_de = serde_json::Deserializer::from_str(line); let RegistryPackage { name, vers, @@ -150,9 +148,11 @@ impl<'cfg> RegistryIndex<'cfg> { features, yanked, links, - } = RegistryPackageDefault(&self.source_id).deserialize(&mut json_de)?; + } = serde_json::from_str(line)?; let pkgid = PackageId::new(&name, &vers, &self.source_id)?; - let summary = Summary::new(pkgid, deps.inner, features, links)?; + let deps = deps.into_iter().map(|dep| dep.into_dep(&self.source_id)) + .collect::>>()?; + let summary = Summary::new(pkgid, deps, features, links)?; let summary = summary.set_checksum(cksum.clone()); if self.hashes.contains_key(&name[..]) { self.hashes.get_mut(&name[..]).unwrap().insert(vers, cksum); diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 95b0d11ac8a..96cce76d460 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -160,13 +160,11 @@ use std::borrow::Cow; use std::collections::BTreeMap; -use std::fmt; use std::fs::File; use std::path::{Path, PathBuf}; use flate2::read::GzDecoder; use semver::Version; -use serde::de; use tar::Archive; use core::{Package, PackageId, Registry, Source, SourceId, Summary}; @@ -212,29 +210,17 @@ pub struct RegistryConfig { pub api: Option, } +#[derive(Deserialize)] pub struct RegistryPackage<'a> { name: Cow<'a, str>, vers: Version, - deps: DependencyList, + deps: Vec>, features: BTreeMap>, cksum: String, yanked: Option, links: Option, } -pub struct RegistryPackageDefault<'a>(pub &'a SourceId); - -impl<'de, 'a> de::DeserializeSeed<'de> for RegistryPackageDefault<'a> { - type Value = RegistryPackage<'de>; - - fn deserialize(self, deserializer: D) -> Result - where - D: de::Deserializer<'de>, - { - deserializer.deserialize_map(self) - } -} - #[derive(Deserialize)] #[serde(field_identifier, rename_all = "lowercase")] enum Field { @@ -247,86 +233,6 @@ enum Field { Links, } -impl<'a, 'de> de::Visitor<'de> for RegistryPackageDefault<'a> { - type Value = RegistryPackage<'de>; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "struct RegistryPackage") - } - - fn visit_map(self, mut map: A) -> Result - where - A: de::MapAccess<'de>, - { - let mut name = None; - let mut vers = None; - let mut deps = None; - let mut features = None; - let mut cksum = None; - let mut yanked = None; - let mut links = None; - while let Some(key) = map.next_key()? { - match key { - Field::Name => { - if name.is_some() { - return Err(de::Error::duplicate_field("name")); - } - name = Some(map.next_value()?); - } - Field::Vers => { - if vers.is_some() { - return Err(de::Error::duplicate_field("vers")); - } - vers = Some(map.next_value()?); - } - Field::Deps => { - if deps.is_some() { - return Err(de::Error::duplicate_field("deps")); - } - deps = Some(map.next_value_seed(DependencyListDefault(self.0))?); - } - Field::Features => { - if features.is_some() { - return Err(de::Error::duplicate_field("features")); - } - features = Some(map.next_value()?); - } - Field::Cksum => { - if cksum.is_some() { - return Err(de::Error::duplicate_field("cksum")); - } - cksum = Some(map.next_value()?); - } - Field::Yanked => { - if yanked.is_some() { - return Err(de::Error::duplicate_field("yanked")); - } - yanked = Some(map.next_value()?); - } - Field::Links => { - if links.is_some() { - return Err(de::Error::duplicate_field("links")); - } - links = Some(map.next_value()?); - } - } - } - Ok(RegistryPackage { - name: name.ok_or_else(|| de::Error::missing_field("name"))?, - vers: vers.ok_or_else(|| de::Error::missing_field("vers"))?, - deps: deps.ok_or_else(|| de::Error::missing_field("deps"))?, - features: features.ok_or_else(|| de::Error::missing_field("features"))?, - cksum: cksum.ok_or_else(|| de::Error::missing_field("cksum"))?, - yanked: yanked.ok_or_else(|| de::Error::missing_field("yanked"))?, - links: links.unwrap_or(None), - }) - } -} - -struct DependencyList { - inner: Vec, -} - #[derive(Deserialize)] struct RegistryDependency<'a> { name: Cow<'a, str>, @@ -339,6 +245,55 @@ struct RegistryDependency<'a> { registry: Option, } +impl<'a> RegistryDependency<'a> { + /// Converts an encoded dependency in the registry to a cargo dependency + pub fn into_dep(self, default: &SourceId) -> CargoResult { + let RegistryDependency { + name, + req, + mut features, + optional, + default_features, + target, + kind, + registry, + } = self; + + let id = if let Some(registry) = registry { + SourceId::for_registry(®istry.to_url()?)? + } else { + default.clone() + }; + + let mut dep = Dependency::parse_no_deprecated(&name, Some(&req), &id)?; + let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") { + "dev" => Kind::Development, + "build" => Kind::Build, + _ => Kind::Normal, + }; + + let platform = match target { + Some(target) => Some(target.parse()?), + None => None, + }; + + // Unfortunately older versions of cargo and/or the registry ended up + // publishing lots of entries where the features array contained the + // empty feature, "", inside. This confuses the resolution process much + // later on and these features aren't actually valid, so filter them all + // out here. + features.retain(|s| !s.is_empty()); + + dep.set_optional(optional) + .set_default_features(default_features) + .set_features(features) + .set_platform(platform) + .set_kind(kind); + + Ok(dep) + } +} + pub trait RegistryData { fn index_path(&self) -> &Filesystem; fn load( @@ -543,89 +498,3 @@ impl<'cfg> Source for RegistrySource<'cfg> { Ok(pkg.package_id().version().to_string()) } } - -struct DependencyListDefault<'a>(&'a SourceId); - -impl<'a, 'de> de::DeserializeSeed<'de> for DependencyListDefault<'a> { - type Value = DependencyList; - fn deserialize(self, deserializer: D) -> Result - where - D: de::Deserializer<'de>, - { - deserializer.deserialize_seq(self) - } -} - -impl<'de, 'a> de::Visitor<'de> for DependencyListDefault<'a> { - type Value = DependencyList; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "a list of dependencies") - } - - fn visit_seq(self, mut seq: A) -> Result - where - A: de::SeqAccess<'de>, - { - let mut ret = Vec::new(); - if let Some(size) = seq.size_hint() { - ret.reserve(size); - } - while let Some(element) = seq.next_element::()? { - let dep = parse_registry_dependency(element, &self.0).map_err(de::Error::custom)?; - ret.push(dep); - } - - Ok(DependencyList { inner: ret }) - } -} - -/// Converts an encoded dependency in the registry to a cargo dependency -fn parse_registry_dependency( - dep: RegistryDependency, - default: &SourceId, -) -> CargoResult { - let RegistryDependency { - name, - req, - mut features, - optional, - default_features, - target, - kind, - registry, - } = dep; - - let id = if let Some(registry) = registry { - SourceId::for_registry(®istry.to_url()?)? - } else { - default.clone() - }; - - let mut dep = Dependency::parse_no_deprecated(&name, Some(&req), &id)?; - let kind = match kind.as_ref().map(|s| &s[..]).unwrap_or("") { - "dev" => Kind::Development, - "build" => Kind::Build, - _ => Kind::Normal, - }; - - let platform = match target { - Some(target) => Some(target.parse()?), - None => None, - }; - - // Unfortunately older versions of cargo and/or the registry ended up - // publishing lots of entries where the features array contained the - // empty feature, "", inside. This confuses the resolution process much - // later on and these features aren't actually valid, so filter them all - // out here. - features.retain(|s| !s.is_empty()); - - dep.set_optional(optional) - .set_default_features(default_features) - .set_features(features) - .set_platform(platform) - .set_kind(kind); - - Ok(dep) -}