diff --git a/src/cargo/core/interning.rs b/src/cargo/core/interning.rs index 2f8cf7451fa..d8c18df2d84 100644 --- a/src/cargo/core/interning.rs +++ b/src/cargo/core/interning.rs @@ -1,3 +1,5 @@ +use serde::{Serialize, Serializer}; + use std::fmt; use std::sync::RwLock; use std::collections::HashSet; @@ -95,3 +97,12 @@ impl PartialOrd for InternedString { Some(self.cmp(other)) } } + +impl Serialize for InternedString { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + serializer.serialize_str(self.inner) + } +} diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index f6434f77362..2f7db061a21 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -9,7 +9,7 @@ pub use self::registry::Registry; pub use self::resolver::Resolve; pub use self::shell::{Shell, Verbosity}; pub use self::source::{GitReference, Source, SourceId, SourceMap}; -pub use self::summary::Summary; +pub use self::summary::{FeatureMap, FeatureValue, Summary}; pub use self::workspace::{Members, Workspace, WorkspaceConfig, WorkspaceRootConfig}; pub mod source; diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index f3ce2eaf29c..990144254da 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -1,5 +1,5 @@ use std::cell::{Ref, RefCell}; -use std::collections::{BTreeMap, HashMap}; +use std::collections::HashMap; use std::fmt; use std::hash; use std::path::{Path, PathBuf}; @@ -10,7 +10,7 @@ use toml; use lazycell::LazyCell; use core::{Dependency, Manifest, PackageId, SourceId, Target}; -use core::{SourceMap, Summary}; +use core::{FeatureMap, SourceMap, Summary}; use core::interning::InternedString; use util::{internal, lev_distance, Config}; use util::errors::{CargoResult, CargoResultExt}; @@ -39,7 +39,7 @@ struct SerializedPackage<'a> { source: &'a SourceId, dependencies: &'a [Dependency], targets: &'a [Target], - features: &'a BTreeMap>, + features: &'a FeatureMap, manifest_path: &'a str, } diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index a1f97fe3c25..2de77d39fc3 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -1,7 +1,7 @@ use std::collections::{HashMap, HashSet}; use std::rc::Rc; -use core::{Dependency, PackageId, SourceId, Summary}; +use core::{Dependency, FeatureValue, PackageId, SourceId, Summary}; use core::interning::InternedString; use util::Graph; use util::CargoResult; @@ -170,7 +170,24 @@ impl Context { let deps = s.dependencies(); let deps = deps.iter().filter(|d| d.is_transitive() || dev_deps); - let mut reqs = build_requirements(s, method)?; + // Requested features stored in the Method are stored as string references, but we want to + // transform them into FeatureValues here. In order to pass the borrow checker with + // storage of the FeatureValues that outlives the Requirements object, we do the + // transformation here, and pass the FeatureValues to build_requirements(). + let values = if let Method::Required { + all_features: false, + features: requested, + .. + } = *method + { + requested + .iter() + .map(|f| FeatureValue::new(f, s)) + .collect::>() + } else { + vec![] + }; + let mut reqs = build_requirements(s, method, &values)?; let mut ret = Vec::new(); // Next, collect all actually enabled dependencies and their features. @@ -269,8 +286,12 @@ impl Context { fn build_requirements<'a, 'b: 'a>( s: &'a Summary, method: &'b Method, + requested: &'a [FeatureValue], ) -> CargoResult> { let mut reqs = Requirements::new(s); + for fv in requested.iter() { + reqs.require_value(fv)?; + } match *method { Method::Everything | Method::Required { @@ -283,12 +304,7 @@ fn build_requirements<'a, 'b: 'a>( reqs.require_dependency(dep.name().as_str()); } } - Method::Required { - features: requested_features, - .. - } => for feat in requested_features.iter() { - reqs.add_feature(feat)?; - }, + _ => {} // Explicitly requested features are handled through `requested` } match *method { Method::Everything @@ -359,50 +375,34 @@ impl<'r> Requirements<'r> { } fn require_feature(&mut self, feat: &'r str) -> CargoResult<()> { - if self.seen(feat) { + if feat.is_empty() || self.seen(feat) { return Ok(()); } - for f in self.summary + for fv in self.summary .features() .get(feat) .expect("must be a valid feature") { - if f == feat { - bail!( + match *fv { + FeatureValue::Feature(ref dep_feat) if **dep_feat == *feat => bail!( "Cyclic feature dependency: feature `{}` depends on itself", feat - ); + ), + _ => {} } - self.add_feature(f)?; + self.require_value(&fv)?; } Ok(()) } - fn add_feature(&mut self, feat: &'r str) -> CargoResult<()> { - if feat.is_empty() { - return Ok(()); - } - - // If this feature is of the form `foo/bar`, then we just lookup package - // `foo` and enable its feature `bar`. Otherwise this feature is of the - // form `foo` and we need to recurse to enable the feature `foo` for our - // own package, which may end up enabling more features or just enabling - // a dependency. - let mut parts = feat.splitn(2, '/'); - let feat_or_package = parts.next().unwrap(); - match parts.next() { - Some(feat) => { - self.require_crate_feature(feat_or_package, feat); - } - None => { - if self.summary.features().contains_key(feat_or_package) { - self.require_feature(feat_or_package)?; - } else { - self.require_dependency(feat_or_package); - } + fn require_value(&mut self, fv: &'r FeatureValue) -> CargoResult<()> { + match *fv { + FeatureValue::Feature(ref feat) => self.require_feature(feat), + FeatureValue::Crate(ref dep) => Ok(self.require_dependency(dep)), + FeatureValue::CrateFeature(ref dep, ref dep_feat) => { + Ok(self.require_crate_feature(dep, dep_feat)) } } - Ok(()) } } diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index db2545d44c5..1c8d95b36e4 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -21,7 +21,7 @@ pub struct Summary { struct Inner { package_id: PackageId, dependencies: Vec, - features: BTreeMap>, + features: FeatureMap, checksum: Option, links: Option, } @@ -48,47 +48,12 @@ impl Summary { ) } } - for (feature, list) in features.iter() { - for dep in list.iter() { - let mut parts = dep.splitn(2, '/'); - let dep = parts.next().unwrap(); - let is_reexport = parts.next().is_some(); - if !is_reexport && features.get(dep).is_some() { - continue; - } - match dependencies.iter().find(|d| &*d.name() == dep) { - Some(d) => { - if d.is_optional() || is_reexport { - continue; - } - bail!( - "Feature `{}` depends on `{}` which is not an \ - optional dependency.\nConsider adding \ - `optional = true` to the dependency", - feature, - dep - ) - } - None if is_reexport => bail!( - "Feature `{}` requires a feature of `{}` which is not a \ - dependency", - feature, - dep - ), - None => bail!( - "Feature `{}` includes `{}` which is neither \ - a dependency nor another feature", - feature, - dep - ), - } - } - } + let feature_map = build_feature_map(features, &dependencies)?; Ok(Summary { inner: Rc::new(Inner { package_id: pkg_id, dependencies, - features, + features: feature_map, checksum: None, links: links.map(|l| InternedString::new(&l)), }), @@ -110,7 +75,7 @@ impl Summary { pub fn dependencies(&self) -> &[Dependency] { &self.inner.dependencies } - pub fn features(&self) -> &BTreeMap> { + pub fn features(&self) -> &FeatureMap { &self.inner.features } pub fn checksum(&self) -> Option<&str> { @@ -158,3 +123,108 @@ impl PartialEq for Summary { self.inner.package_id == other.inner.package_id } } + +// Checks features for errors, bailing out a CargoResult:Err if invalid, +// and creates FeatureValues for each feature. +fn build_feature_map( + features: BTreeMap>, + dependencies: &Vec, +) -> CargoResult { + use self::FeatureValue::*; + let mut map = BTreeMap::new(); + for (feature, list) in features.iter() { + let mut values = vec![]; + for dep in list { + let val = FeatureValue::build(dep, |fs| (&features).get(fs).is_some()); + if let &Feature(_) = &val { + values.push(val); + continue; + } + + // Find data for the referenced dependency... + let dep_data = { + let dep_name = match &val { + &Feature(_) => "", + &Crate(ref dep_name) | &CrateFeature(ref dep_name, _) => dep_name, + }; + dependencies.iter().find(|d| *d.name() == *dep_name) + }; + + match (&val, dep_data) { + (&Crate(ref dep), Some(d)) => { + if !d.is_optional() { + bail!( + "Feature `{}` depends on `{}` which is not an \ + optional dependency.\nConsider adding \ + `optional = true` to the dependency", + feature, + dep + ) + } + } + (&CrateFeature(ref dep_name, _), None) => bail!( + "Feature `{}` requires a feature of `{}` which is not a \ + dependency", + feature, + dep_name + ), + (&Crate(ref dep), None) => bail!( + "Feature `{}` includes `{}` which is neither \ + a dependency nor another feature", + feature, + dep + ), + (&CrateFeature(_, _), Some(_)) | (&Feature(_), _) => {} + } + values.push(val); + } + map.insert(feature.clone(), values); + } + Ok(map) +} + +/// FeatureValue represents the types of dependencies a feature can have: +/// +/// * Another feature +/// * An optional dependency +/// * A feature in a depedency +/// +/// The selection between these 3 things happens as part of the construction of the FeatureValue. +#[derive(Clone, Debug, Serialize)] +pub enum FeatureValue { + Feature(InternedString), + Crate(InternedString), + CrateFeature(InternedString, InternedString), +} + +impl FeatureValue { + fn build(feature: &str, is_feature: T) -> FeatureValue + where + T: Fn(&str) -> bool, + { + match feature.find('/') { + Some(pos) => { + let (dep, dep_feat) = feature.split_at(pos); + let dep_feat = &dep_feat[1..]; + FeatureValue::CrateFeature(InternedString::new(dep), InternedString::new(dep_feat)) + } + None if is_feature(&feature) => FeatureValue::Feature(InternedString::new(feature)), + None => FeatureValue::Crate(InternedString::new(feature)), + } + } + + pub fn new(feature: &str, s: &Summary) -> FeatureValue { + Self::build(feature, |fs| s.features().get(fs).is_some()) + } + + pub fn to_string(&self) -> String { + use self::FeatureValue::*; + match *self { + Feature(ref f) => f.to_string(), + Crate(ref c) => c.to_string(), + CrateFeature(ref c, ref f) => [c.as_ref(), f.as_ref()].join("/"), + } + } +} + +pub type FeatureMap = BTreeMap>; diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index ed997ccb266..64f4a8cb57d 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -1,4 +1,5 @@ use std::{cmp, env}; +use std::collections::BTreeMap; use std::fs::{self, File}; use std::iter::repeat; use std::time::Duration; @@ -213,12 +214,23 @@ fn transmit( return Ok(()); } + let string_features = pkg.summary() + .features() + .iter() + .map(|(feat, values)| { + ( + feat.clone(), + values.iter().map(|fv| fv.to_string()).collect(), + ) + }) + .collect::>>(); + let publish = registry.publish( &NewCrate { name: pkg.name().to_string(), vers: pkg.version().to_string(), deps, - features: pkg.summary().features().clone(), + features: string_features, authors: authors.clone(), description: description.clone(), homepage: homepage.clone(), diff --git a/tests/testsuite/metadata.rs b/tests/testsuite/metadata.rs index 4a2154bd930..34720c4b222 100644 --- a/tests/testsuite/metadata.rs +++ b/tests/testsuite/metadata.rs @@ -192,7 +192,9 @@ optional_feat = [] ], "features": { "default": [ - "default_feat" + { + "Feature": "default_feat" + } ], "default_feat": [], "optional_feat": []