diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index e216bfa800d..447080171d1 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -9,7 +9,7 @@ use crate::core::profiles::Profiles; use crate::core::{Dependency, Workspace}; use crate::core::{PackageId, PackageSet, Resolve}; use crate::util::errors::CargoResult; -use crate::util::{profile, Cfg, CfgExpr, Config, Rustc}; +use crate::util::{profile, Cfg, CfgExpr, Config, Rustc, Platform}; use super::{BuildConfig, BuildOutput, Kind, Unit}; @@ -90,13 +90,11 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { self.resolve .extern_crate_name(unit.pkg.package_id(), dep.pkg.package_id(), dep.target) } - - /// Whether a dependency should be compiled for the host or target platform, + + /// Whether a given platform matches the host or target platform, /// specified by `Kind`. - pub fn dep_platform_activated(&self, dep: &Dependency, kind: Kind) -> bool { - // If this dependency is only available for certain platforms, - // make sure we're only enabling it for that platform. - let platform = match dep.platform() { + pub fn platform_activated(&self, platform: Option<&Platform>, kind: Kind) -> bool { + let platform = match platform { Some(p) => p, None => return true, }; @@ -107,7 +105,15 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { platform.matches(name, info.cfg()) } - /// Gets the user-specified linker for a particular host or target. + /// Whether a dependency should be compiled for the host or target platform, + /// specified by `Kind`. + pub fn dep_platform_activated(&self, dep: &Dependency, kind: Kind) -> bool { + // If this dependency is only available for certain platforms, + // make sure we're only enabling it for that platform. + self.platform_activated(dep.platform(), kind) + } + + /// Get the user-specified linker for a particular host or target pub fn linker(&self, kind: Kind) -> Option<&Path> { self.target_config(kind).linker.as_ref().map(|s| s.as_ref()) } diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 67e22d5ec83..3d5108645f8 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -226,7 +226,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> { }); } - let feats = self.bcx.resolve.features(unit.pkg.package_id()); + let bcx = self.bcx; + let feats = bcx.resolve.features(unit.pkg.package_id()); if !feats.is_empty() { self.compilation .cfgs @@ -234,7 +235,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> { .or_insert_with(|| { feats .iter() - .map(|feat| format!("feature=\"{}\"", feat)) + .filter(|feat| bcx.platform_activated(feat.1.as_ref(), unit.kind)) + .map(|feat| format!("feature=\"{}\"", feat.0)) .collect() }); } diff --git a/src/cargo/core/compiler/context/unit_dependencies.rs b/src/cargo/core/compiler/context/unit_dependencies.rs index 7ba1aaa38d4..9067dd02ee8 100644 --- a/src/cargo/core/compiler/context/unit_dependencies.rs +++ b/src/cargo/core/compiler/context/unit_dependencies.rs @@ -158,8 +158,15 @@ fn compute_deps<'a, 'cfg, 'tmp>( // If the dependency is optional, then we're only activating it // if the corresponding feature was activated - if dep.is_optional() && !bcx.resolve.features(id).contains(&*dep.name_in_toml()) { - return false; + if dep.is_optional() { + // Same for features this dependency is referenced + if let Some(platform) = bcx.resolve.features(id).get(&*dep.name_in_toml()) { + if !bcx.platform_activated(platform.as_ref(), unit.kind) { + return false; + } + } else { + return false; + } } // If we've gotten past all that, then this dependency is @@ -228,7 +235,7 @@ fn compute_deps<'a, 'cfg, 'tmp>( t.is_bin() && // Skip binaries with required features that have not been selected. t.required_features().unwrap_or(&no_required_features).iter().all(|f| { - bcx.resolve.features(id).contains(f) + bcx.resolve.features(id).contains_key(f) && bcx.platform_activated(bcx.resolve.features(id).get(f).unwrap().as_ref(), unit.kind) }) }) .map(|t| { diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 7516812842c..691b07e787d 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -192,7 +192,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes // Be sure to pass along all enabled features for this package, this is the // last piece of statically known information that we have. for feat in bcx.resolve.features(unit.pkg.package_id()).iter() { - cmd.env(&format!("CARGO_FEATURE_{}", super::envify(feat)), "1"); + cmd.env(&format!("CARGO_FEATURE_{}", super::envify(feat.0)), "1"); } let mut cfg_map = HashMap::new(); diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index c58a78679d1..4687b5b0395 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -416,7 +416,7 @@ fn link_targets<'a, 'cfg>( .resolve .features_sorted(package_id) .into_iter() - .map(|s| s.to_owned()) + .map(|s| s.0.to_owned()) .collect(); let json_messages = bcx.build_config.json_messages(); let executable = cx.get_executable(unit)?; @@ -635,7 +635,10 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult rustdoc.arg("-o").arg(doc_dir); for feat in bcx.resolve.features_sorted(unit.pkg.package_id()) { - rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); + if !bcx.platform_activated(feat.1, unit.kind) { + continue; + } + rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat.0)); } add_error_format(bcx, &mut rustdoc); @@ -866,7 +869,10 @@ fn build_base_args<'a, 'cfg>( // rustc-caching strategies like sccache are able to cache more, so sort the // feature list here. for feat in bcx.resolve.features_sorted(unit.pkg.package_id()) { - cmd.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); + if !bcx.platform_activated(feat.1, unit.kind) { + continue; + } + cmd.arg("--cfg").arg(&format!("feature=\"{}\"", feat.0)); } match cx.files().metadata(unit) { diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index b779d77c693..11ec9878cb5 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -1,6 +1,4 @@ -use std::fmt; use std::rc::Rc; -use std::str::FromStr; use log::trace; use semver::ReqParseError; @@ -12,7 +10,7 @@ use url::Url; use crate::core::interning::InternedString; use crate::core::{PackageId, SourceId, Summary}; use crate::util::errors::{CargoResult, CargoResultExt}; -use crate::util::{Cfg, CfgExpr, Config}; +use crate::util::{Platform, Config}; /// Information about a dependency requested by a Cargo manifest. /// Cheap to copy. @@ -49,12 +47,6 @@ struct Inner { platform: Option, } -#[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)] -pub enum Platform { - Name(String), - Cfg(CfgExpr), -} - #[derive(Serialize)] struct SerializedDependency<'a> { name: &'a str, @@ -450,49 +442,3 @@ impl Dependency { } } } - -impl Platform { - pub fn matches(&self, name: &str, cfg: Option<&[Cfg]>) -> bool { - match *self { - Platform::Name(ref p) => p == name, - Platform::Cfg(ref p) => match cfg { - Some(cfg) => p.matches(cfg), - None => false, - }, - } - } -} - -impl ser::Serialize for Platform { - fn serialize(&self, s: S) -> Result - where - S: ser::Serializer, - { - self.to_string().serialize(s) - } -} - -impl FromStr for Platform { - type Err = failure::Error; - - fn from_str(s: &str) -> CargoResult { - if s.starts_with("cfg(") && s.ends_with(')') { - let s = &s[4..s.len() - 1]; - let p = s.parse().map(Platform::Cfg).chain_err(|| { - failure::format_err!("failed to parse `{}` as a cfg expression", s) - })?; - Ok(p) - } else { - Ok(Platform::Name(s.to_string())) - } - } -} - -impl fmt::Display for Platform { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self { - Platform::Name(ref n) => n.fmt(f), - Platform::Cfg(ref e) => write!(f, "cfg({})", e), - } - } -} diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 27265541130..7d6cc367cc4 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -12,7 +12,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::{FeatureMap, FeatureValue, Summary}; +pub use self::summary::{FeatureMap, RefFeatureMap, FeatureValue, Summary}; pub use self::workspace::{Members, Workspace, WorkspaceConfig, WorkspaceRootConfig}; pub mod compiler; diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 1cc12fe137b..6f6c0a3dece 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -20,7 +20,7 @@ use serde::Serialize; use crate::core::interning::InternedString; use crate::core::source::MaybePackage; use crate::core::{Dependency, Manifest, PackageId, SourceId, Target}; -use crate::core::{FeatureMap, SourceMap, Summary}; +use crate::core::{RefFeatureMap, SourceMap, Summary}; use crate::ops; use crate::util::errors::{CargoResult, CargoResultExt, HttpNot200}; use crate::util::network::Retry; @@ -63,7 +63,7 @@ struct SerializedPackage<'a> { source: SourceId, dependencies: &'a [Dependency], targets: Vec<&'a Target>, - features: &'a FeatureMap, + features: RefFeatureMap<'a>, manifest_path: &'a Path, metadata: Option<&'a toml::Value>, authors: &'a [String], @@ -93,6 +93,11 @@ impl ser::Serialize for Package { let keywords = manmeta.keywords.as_ref(); let readme = manmeta.readme.as_ref().map(String::as_ref); let repository = manmeta.repository.as_ref().map(String::as_ref); + let features = summary + .features() + .iter() + .map(|(k, (_, v))| (*k, v.as_slice())) + .collect::>(); // Filter out metabuild targets. They are an internal implementation // detail that is probably not relevant externally. There's also not a // real path to show in `src_path`, and this avoids changing the format. @@ -113,7 +118,7 @@ impl ser::Serialize for Package { source: summary.source_id(), dependencies: summary.dependencies(), targets, - features: summary.features(), + features, manifest_path: &self.manifest_path, metadata: self.manifest.custom_metadata(), authors, diff --git a/src/cargo/core/resolver/context.rs b/src/cargo/core/resolver/context.rs index 8a704d864cc..b35e8677cad 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -10,7 +10,8 @@ use log::debug; use crate::core::interning::InternedString; use crate::core::{Dependency, FeatureValue, PackageId, SourceId, Summary}; use crate::util::CargoResult; -use crate::util::Graph; +use crate::util::{Platform, Graph}; +use im_rc; use super::errors::ActivateResult; use super::types::{ @@ -28,9 +29,7 @@ pub use super::resolve::Resolve; #[derive(Clone)] pub struct Context { pub activations: Activations, - /// list the features that are activated for each package - pub resolve_features: im_rc::HashMap>>, - /// get the package that will be linking to a native library by its links attribute + pub resolve_features: im_rc::HashMap>>>, pub links: im_rc::HashMap, /// for each package the list of names it can see, /// then for each name the exact version that name represents and weather the name is public. @@ -157,8 +156,8 @@ impl Context { let has_default_feature = summary.features().contains_key("default"); Ok(match self.resolve_features.get(&id) { Some(prev) => { - features.iter().all(|f| prev.contains(f)) - && (!use_default || prev.contains("default") || !has_default_feature) + features.iter().all(|f| prev.contains_key(f)) + && (!use_default || prev.contains_key("default") || !has_default_feature) } None => features.is_empty() && (!use_default || !has_default_feature), }) @@ -182,7 +181,7 @@ impl Context { .into_iter() .map(|(dep, features)| { let candidates = registry.query(&dep)?; - Ok((dep, candidates, Rc::new(features))) + Ok((dep, candidates, Rc::new(features.iter().map(|(k,_)| *k).collect()))) }) .collect::>>()?; @@ -234,7 +233,7 @@ impl Context { parent: Option<&Summary>, s: &'b Summary, method: &'b Method<'_>, - ) -> ActivateResult)>> { + ) -> ActivateResult)>)>> { let dev_deps = match *method { Method::Everything => true, Method::Required { dev_deps, .. } => dev_deps, @@ -287,7 +286,8 @@ impl Context { .into()); } } - ret.push((dep.clone(), base)); + // TODO danger => dep platform is copied to the feature platform! + ret.push((dep.clone(), base.into_iter().map(|f| (f, dep.platform().cloned())).collect())); } // Any entries in `reqs.dep` which weren't used are bugs in that the @@ -320,11 +320,11 @@ impl Context { let set = Rc::make_mut( self.resolve_features .entry(pkgid) - .or_insert_with(|| Rc::new(HashSet::new())), + .or_insert_with(|| Rc::new(HashMap::new())), ); - for feature in reqs.used { - set.insert(feature); + for (k, v) in reqs.used { + set.insert(k, v); } } @@ -372,8 +372,8 @@ fn build_requirements<'a, 'b: 'a>( | Method::Required { all_features: true, .. } => { - for key in s.features().keys() { - reqs.require_feature(*key)?; + for (key, (platform, _)) in s.features().iter() { + reqs.require_feature(*key, platform.as_ref())?; } for dep in s.dependencies().iter().filter(|d| d.is_optional()) { reqs.require_dependency(dep.name_in_toml()); @@ -384,8 +384,15 @@ fn build_requirements<'a, 'b: 'a>( features: requested, .. } => { - for &f in requested.iter() { - reqs.require_value(&FeatureValue::new(f, s))?; + for item in requested.iter() { + reqs.require_value( + &FeatureValue::new(*item, s), + if let Some((platform, _)) = s.features().get(item) { + platform.as_ref() + } else { + None + } + )?; } } } @@ -395,8 +402,8 @@ fn build_requirements<'a, 'b: 'a>( uses_default_features: true, .. } => { - if s.features().contains_key("default") { - reqs.require_feature(InternedString::new("default"))?; + if let Some((platform,_)) = s.features().get("default") { + reqs.require_feature(InternedString::new("default"), platform.as_ref())?; } } Method::Required { @@ -418,8 +425,8 @@ struct Requirements<'a> { // The used features set is the set of features which this local package had // enabled, which is later used when compiling to instruct the code what // features were enabled. - used: HashSet, - visited: HashSet, + used: HashMap>, + visited: HashMap>, } impl<'r> Requirements<'r> { @@ -427,13 +434,13 @@ impl<'r> Requirements<'r> { Requirements { summary, deps: HashMap::new(), - used: HashSet::new(), - visited: HashSet::new(), + used: HashMap::new(), + visited: HashMap::new(), } } - fn require_crate_feature(&mut self, package: InternedString, feat: InternedString) { - self.used.insert(package); + fn require_crate_feature(&mut self, package: InternedString, feat: InternedString, platform: Option<&Platform>) { + self.used.insert(package, platform.cloned()); self.deps .entry(package) .or_insert((false, Vec::new())) @@ -441,31 +448,31 @@ impl<'r> Requirements<'r> { .push(feat); } - fn seen(&mut self, feat: InternedString) -> bool { - if self.visited.insert(feat) { - self.used.insert(feat); - false - } else { + fn seen(&mut self, feat: InternedString, platform: Option<&Platform>) -> bool { + if self.visited.insert(feat, platform.cloned()).is_some() { true + } else { + self.used.insert(feat, platform.cloned()); + false } } fn require_dependency(&mut self, pkg: InternedString) { - if self.seen(pkg) { + if self.seen(pkg, None) { return; } self.deps.entry(pkg).or_insert((false, Vec::new())).0 = true; } - fn require_feature(&mut self, feat: InternedString) -> CargoResult<()> { - if feat.is_empty() || self.seen(feat) { + fn require_feature(&mut self, feat: InternedString, platform: Option<&Platform>) -> CargoResult<()> { + if feat.is_empty() || self.seen(feat, platform) { return Ok(()); } for fv in self .summary .features() .get(feat.as_str()) - .expect("must be a valid feature") + .expect("must be a valid feature").1.as_slice() { match *fv { FeatureValue::Feature(ref dep_feat) if **dep_feat == *feat => failure::bail!( @@ -474,17 +481,26 @@ impl<'r> Requirements<'r> { ), _ => {} } - self.require_value(fv)?; + let platform = if let FeatureValue::Feature(feature) = fv { + if let Some((platform, _)) = self.summary.features().get(feature) { + platform.as_ref() + } else { + platform + } + } else { + platform.clone() + }; + self.require_value(&fv, platform)?; } Ok(()) } - fn require_value<'f>(&mut self, fv: &'f FeatureValue) -> CargoResult<()> { + fn require_value<'f>(&mut self, fv: &'f FeatureValue, platform: Option<&Platform>) -> CargoResult<()> { match fv { - FeatureValue::Feature(feat) => self.require_feature(*feat)?, + FeatureValue::Feature(feat) => self.require_feature(*feat, platform)?, FeatureValue::Crate(dep) => self.require_dependency(*dep), FeatureValue::CrateFeature(dep, dep_feat) => { - self.require_crate_feature(*dep, *dep_feat) + self.require_crate_feature(*dep, *dep_feat, platform) } }; Ok(()) diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 5c6e240328b..243c5dbf43a 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -146,7 +146,7 @@ pub fn resolve( cx.resolve_replacements(), cx.resolve_features .iter() - .map(|(k, v)| (*k, v.iter().map(|x| x.to_string()).collect())) + .map(|(k, v)| (*k, v.iter().map(|(k, v)| (k.to_string(), v.clone())).collect())) .collect(), cksums, BTreeMap::new(), diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 04261d1f398..6b3630d8267 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -1,13 +1,12 @@ use std::borrow::Borrow; -use std::collections::{HashMap, HashSet}; +use std::collections::{HashMap, BTreeMap}; use std::fmt; -use std::iter::FromIterator; use url::Url; use crate::core::{Dependency, PackageId, PackageIdSpec, Summary, Target}; use crate::util::errors::CargoResult; -use crate::util::Graph; +use crate::util::{Graph, Platform}; use super::encode::Metadata; @@ -24,8 +23,8 @@ pub struct Resolve { graph: Graph>, replacements: HashMap, reverse_replacements: HashMap, - empty_features: HashSet, - features: HashMap>, + empty_features: HashMap>, + features: HashMap>>, checksums: HashMap>, metadata: Metadata, unused_patches: Vec, @@ -35,7 +34,7 @@ impl Resolve { pub fn new( graph: Graph>, replacements: HashMap, - features: HashMap>, + features: HashMap>>, checksums: HashMap>, metadata: Metadata, unused_patches: Vec, @@ -48,7 +47,7 @@ impl Resolve { checksums, metadata, unused_patches, - empty_features: HashSet::new(), + empty_features: HashMap::new(), reverse_replacements, } } @@ -193,14 +192,12 @@ unable to verify that `{0}` is the same as when the lockfile was generated &self.replacements } - pub fn features(&self, pkg: PackageId) -> &HashSet { + pub fn features(&self, pkg: PackageId) -> &HashMap> { self.features.get(&pkg).unwrap_or(&self.empty_features) } - pub fn features_sorted(&self, pkg: PackageId) -> Vec<&str> { - let mut v = Vec::from_iter(self.features(pkg).iter().map(|s| s.as_ref())); - v.sort_unstable(); - v + pub fn features_sorted(&self, pkg: PackageId) -> BTreeMap<&str, Option<&Platform>> { + self.features(pkg).iter().map(|(k, v)| (k.as_str(), v.as_ref())).collect() } pub fn query(&self, spec: &str) -> CargoResult { diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 69f290435b1..315e9e05f7f 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -10,7 +10,7 @@ use crate::core::interning::InternedString; use crate::core::{Dependency, PackageId, SourceId}; use semver::Version; -use crate::util::CargoResult; +use crate::util::{Platform, CargoResult}; /// Subset of a `Manifest`. Contains only the most important information about /// a package. @@ -35,7 +35,7 @@ impl Summary { pub fn new( pkg_id: PackageId, dependencies: Vec, - features: &BTreeMap>>, + features: &BTreeMap, Vec>)>, links: Option>, namespaced_features: bool, ) -> CargoResult @@ -141,7 +141,7 @@ impl PartialEq for Summary { // Checks features for errors, bailing out a CargoResult:Err if invalid, // and creates FeatureValues for each feature. fn build_feature_map( - features: &BTreeMap>>, + features: &BTreeMap, Vec>)>, dependencies: &[Dependency], namespaced: bool, ) -> CargoResult @@ -193,10 +193,10 @@ where }; let mut values = vec![]; - for dep in list { + for dep in list.1.as_slice() { let val = FeatureValue::build( InternedString::new(dep.as_ref()), - |fs| features.contains_key(fs.as_str()), + |fs| features.contains_key(fs.as_str()), namespaced, ); @@ -231,7 +231,7 @@ where // we don't have to do so here. (&Feature(feat), _, true) => { if namespaced && !features.contains_key(&*feat) { - map.insert(feat, vec![FeatureValue::Crate(feat)]); + map.insert(feat, (list.0.clone(), vec![FeatureValue::Crate(feat)])); } } // If features are namespaced and the value is not defined as a feature @@ -329,7 +329,7 @@ where ) } - map.insert(InternedString::new(feature.borrow()), values); + map.insert(InternedString::new(feature.borrow()), (list.0.clone(), values)); } Ok(map) } @@ -408,4 +408,5 @@ impl Serialize for FeatureValue { } } -pub type FeatureMap = BTreeMap>; +pub type FeatureMap = BTreeMap, Vec)>; +pub type RefFeatureMap<'a> = BTreeMap; diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 0a5905be052..5b989006f6f 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -820,13 +820,13 @@ fn resolve_all_features( resolve_with_overrides: &Resolve, package_id: PackageId, ) -> HashSet { - let mut features = resolve_with_overrides.features(package_id).clone(); + let mut features = resolve_with_overrides.features(package_id).keys().cloned().collect::>(); // Include features enabled for use by dependencies so targets can also use them with the // required-features field when deciding whether to be built or skipped. for (dep, _) in resolve_with_overrides.deps(package_id) { for feature in resolve_with_overrides.features(dep) { - features.insert(dep.name().to_string() + "/" + feature); + features.insert(dep.name().to_string() + "/" + feature.0); } } diff --git a/src/cargo/ops/cargo_output_metadata.rs b/src/cargo/ops/cargo_output_metadata.rs index 957664cfda5..87647c3a8d6 100644 --- a/src/cargo/ops/cargo_output_metadata.rs +++ b/src/cargo/ops/cargo_output_metadata.rs @@ -131,7 +131,7 @@ where .map(|name| Dep { name, pkg }) }) .collect(), - features: resolve.features_sorted(id), + features: resolve.features_sorted(id).keys().cloned().collect(), } })) } diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index f52d85b9182..9014a8f4419 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -238,7 +238,7 @@ fn transmit( .map(|(feat, values)| { ( feat.to_string(), - values.iter().map(|fv| fv.to_string(summary)).collect(), + values.1.iter().map(|fv| fv.to_string(&summary)).collect(), ) }) .collect::>>(); diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 9b083463c08..98b511a9791 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -253,7 +253,8 @@ impl<'cfg> RegistryIndex<'cfg> { .into_iter() .map(|dep| dep.into_dep(self.source_id)) .collect::>>()?; - let summary = Summary::new(pkgid, deps, &features, links, false)?; + let ftrs = features.iter().map(|(k, v)| (k.clone(), (None, v.clone()))).collect(); + let summary = Summary::new(pkgid, deps, &ftrs, links, false)?; let summary = summary.set_checksum(cksum.clone()); self.hashes .entry(name.as_str()) diff --git a/src/cargo/util/cfg.rs b/src/cargo/util/cfg.rs index 4c4ad232df4..8dcee7a62a6 100644 --- a/src/cargo/util/cfg.rs +++ b/src/cargo/util/cfg.rs @@ -1,8 +1,62 @@ use std::fmt; use std::iter; use std::str::{self, FromStr}; +use serde::ser; use crate::util::CargoResult; +use crate::util::errors::CargoResultExt; + +#[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)] +pub enum Platform { + Name(String), + Cfg(CfgExpr), +} + +impl Platform { + pub fn matches(&self, name: &str, cfg: Option<&[Cfg]>) -> bool { + match *self { + Platform::Name(ref p) => p == name, + Platform::Cfg(ref p) => match cfg { + Some(cfg) => p.matches(cfg), + None => false, + }, + } + } +} + +impl ser::Serialize for Platform { + fn serialize(&self, s: S) -> Result + where + S: ser::Serializer, + { + self.to_string().serialize(s) + } +} + +impl FromStr for Platform { + type Err = failure::Error; + + fn from_str(s: &str) -> CargoResult { + if s.starts_with("cfg(") && s.ends_with(')') { + let s = &s[4..s.len() - 1]; + let p = s.parse().map(Platform::Cfg).chain_err(|| { + failure::format_err!("failed to parse `{}` as a cfg expression", s) + })?; + Ok(p) + } else { + Ok(Platform::Name(s.to_string())) + } + } +} + +impl fmt::Display for Platform { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + Platform::Name(ref n) => n.fmt(f), + Platform::Cfg(ref e) => write!(f, "cfg({})", e), + } + } +} #[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)] pub enum Cfg { diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index e46df6ce943..1e3e742f968 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -1,6 +1,6 @@ use std::time::Duration; -pub use self::cfg::{Cfg, CfgExpr}; +pub use self::cfg::{Cfg, CfgExpr, Platform}; pub use self::config::{homedir, Config, ConfigValue}; pub use self::dependency_queue::{DependencyQueue, Dirty, Fresh, Freshness}; pub use self::diagnostic_server::RustfixDiagnosticServer; diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 676e882e15c..525451ddeb0 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -13,7 +13,7 @@ use serde::ser; use serde::{Deserialize, Serialize}; use url::Url; -use crate::core::dependency::{Kind, Platform}; +use crate::core::dependency::Kind; use crate::core::manifest::{LibKind, ManifestMetadata, TargetSourcePath, Warnings}; use crate::core::profiles::Profiles; use crate::core::{Dependency, Manifest, PackageId, Summary, Target}; @@ -22,7 +22,7 @@ use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, Worksp use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::errors::{CargoResult, CargoResultExt, ManifestError}; use crate::util::paths; -use crate::util::{self, validate_package_name, Config, ToUrl}; +use crate::util::{self, validate_package_name, Config, ToUrl, Platform}; mod targets; use self::targets::targets; @@ -754,6 +754,7 @@ impl TomlManifest { Ok(( k.clone(), TomlPlatform { + features: v.features.clone(), dependencies: map_deps(config, v.dependencies.as_ref())?, dev_dependencies: map_deps( config, @@ -892,6 +893,7 @@ impl TomlManifest { } let mut deps = Vec::new(); + let mut ftrs = BTreeMap::new(); let replace; let patch; @@ -924,6 +926,21 @@ impl TomlManifest { Ok(()) } + fn process_features( + ftrs: &mut BTreeMap, Vec)>, + new_ftrs: Option<&BTreeMap>>, + platform: Option<&Platform> + ) -> CargoResult<()> { + let features = match new_ftrs { + Some(features) => features, + None => return Ok(()), + }; + for (n, v) in features.iter() { + ftrs.insert(n.clone(), (platform.cloned(), v.clone())); + } + + Ok(()) + } // Collect the dependencies. process_dependencies(&mut cx, me.dependencies.as_ref(), None)?; @@ -937,6 +954,7 @@ impl TomlManifest { .as_ref() .or_else(|| me.build_dependencies2.as_ref()); process_dependencies(&mut cx, build_deps, Some(Kind::Build))?; + process_features(&mut ftrs, me.features.as_ref(), None)?; for (name, platform) in me.target.iter().flat_map(|t| t) { cx.platform = Some(name.parse()?); @@ -951,6 +969,7 @@ impl TomlManifest { .as_ref() .or_else(|| platform.dev_dependencies2.as_ref()); process_dependencies(&mut cx, dev_deps, Some(Kind::Development))?; + process_features(&mut ftrs, platform.features.as_ref(), cx.platform.as_ref())?; } replace = me.replace(&mut cx)?; @@ -982,14 +1001,7 @@ impl TomlManifest { let summary = Summary::new( pkgid, deps, - &me.features - .as_ref() - .map(|x| { - x.iter() - .map(|(k, v)| (k.as_str(), v.iter().collect())) - .collect() - }) - .unwrap_or_else(BTreeMap::new), + &ftrs, project.links.as_ref().map(|x| x.as_str()), project.namespaced_features.unwrap_or(false), )?; @@ -1516,6 +1528,7 @@ impl ser::Serialize for PathValue { /// Corresponds to a `target` entry, but `TomlTarget` is already used. #[derive(Serialize, Deserialize, Debug)] struct TomlPlatform { + features: Option>>, dependencies: Option>, #[serde(rename = "build-dependencies")] build_dependencies: Option>, diff --git a/tests/testsuite/cfg_features.rs b/tests/testsuite/cfg_features.rs new file mode 100644 index 00000000000..e7670a8ae6b --- /dev/null +++ b/tests/testsuite/cfg_features.rs @@ -0,0 +1,194 @@ +use crate::support::project; + +#[test] +fn syntax() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [target.'cfg(unix)'.features] + b = [] + [target.'cfg(windows)'.features] + b = [] + "#, + ).file("src/lib.rs", r#" + pub fn bb() {} + "#).build(); + p.cargo("build") + .with_stderr( + "\ +[COMPILING] a v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ).run(); +} + +#[test] +fn include_by_param() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [target.'cfg(unix)'.features] + b = [] + [target.'cfg(windows)'.features] + c = [] + "#, + ).file("src/lib.rs", r#" + #[cfg(feature = "b")] + pub const BB: usize = 0; + #[cfg(feature = "c")] + pub const BB: usize = 1; + + pub fn bb() -> Result<(), ()> { if BB > 0 { Ok(()) } else { Err(()) } } + "#).build(); + p.cargo(format!("build --features {}", if cfg!(unix) { "b" } else { "c" }).as_str()) + .with_stderr( + "\ +[COMPILING] a v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ).run(); +} + +#[test] +fn dont_include_by_platform() { + let other_family = if cfg!(unix) { "windows" } else { "unix" }; + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [target.'cfg({})'.features] + b = [] + "#, + other_family + ), + ).file("src/lib.rs", r#" + #[cfg(feature = "b")] + pub const BB: usize = 0; + + pub fn bb() { let _ = BB; } + "#).build(); + p.cargo("build --features b -vv") + .with_status(101) + .with_stderr_contains( + "\ +error[E0425]: cannot find value `BB` in this scope", + ).run(); +} + +#[test] +fn dont_include_by_param() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [target.'cfg(unix)'.features] + b = [] + [target.'cfg(windows)'.features] + c = [] + "#, + ).file("src/lib.rs", r#" + #[cfg(feature = "b")] + pub const BB: usize = 0; + #[cfg(feature = "c")] + pub const BB: usize = 1; + + pub fn bb() -> Result<(), ()> { if BB > 0 { Ok(()) } else { Err(()) } } + "#).build(); + p.cargo("build -v") + .with_status(101) + .with_stderr_contains( + "\ +error[E0425]: cannot find value `BB` in this scope", + ).run(); +} + +#[test] +fn dont_include_default() { + let other_family = if cfg!(unix) { "windows" } else { "unix" }; + let p = project() + .file( + "Cargo.toml", + &format!(r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [target.'cfg({})'.features] + b = [] + + [features] + default = ["b"] + "#, + other_family), + ).file("src/lib.rs", r#" + #[cfg(feature = "b")] + pub const BB: usize = 0; + + pub fn bb() { let _ = BB; } + "#).build(); + p.cargo("build -v") + .with_status(101) + .with_stderr_contains( + "\ +error[E0425]: cannot find value `BB` in this scope", + ).run(); +} + +// https://github.com/rust-lang/cargo/issues/5313 +#[test] +#[cfg(all( + target_arch = "x86_64", + target_os = "linux", + target_env = "gnu" +))] +fn cfg_looks_at_rustflags_for_target() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "a" + version = "0.0.1" + authors = [] + + [target.'cfg(with_b)'.features] + b = [] + "#, + ).file( + "src/main.rs", + r#" + #[cfg(with_b)] + pub const BB: usize = 0; + + fn main() { let _ = BB; } + "#, + ).build(); + + p.cargo("build --target x86_64-unknown-linux-gnu") + .env("RUSTFLAGS", "--cfg with_b") + .run(); +} diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 69ca0186988..bc630e48aee 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -22,6 +22,7 @@ mod cargo_alias_config; mod cargo_command; mod cargo_features; mod cfg; +mod cfg_features; mod check; mod clean; mod collisions; diff --git a/tests/testsuite/support/resolver.rs b/tests/testsuite/support/resolver.rs index bce8c317e81..192c56115a4 100644 --- a/tests/testsuite/support/resolver.rs +++ b/tests/testsuite/support/resolver.rs @@ -11,7 +11,7 @@ use cargo::core::resolver::{self, Method}; use cargo::core::source::{GitReference, SourceId}; use cargo::core::Resolve; use cargo::core::{Dependency, PackageId, Registry, Summary}; -use cargo::util::{CargoResult, Config, ToUrl}; +use cargo::util::{CargoResult, Config, ToUrl, Platform}; use proptest::collection::{btree_map, vec}; use proptest::prelude::*; @@ -101,7 +101,7 @@ pub fn resolve_with_config_raw( let summary = Summary::new( pkg, deps, - &BTreeMap::>::new(), + &BTreeMap::, Vec)>::new(), None::, false, ) @@ -196,7 +196,7 @@ pub fn pkg_dep(name: T, dep: Vec) -> Summary { Summary::new( name.to_pkgid(), dep, - &BTreeMap::>::new(), + &BTreeMap::, Vec)>::new(), link, false, ) @@ -224,7 +224,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary { Summary::new( pkg_id_loc(name, loc), Vec::new(), - &BTreeMap::>::new(), + &BTreeMap::, Vec)>::new(), link, false, ) @@ -238,7 +238,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary { Summary::new( sum.package_id(), deps, - &BTreeMap::>::new(), + &BTreeMap::, Vec)>::new(), sum.links().map(|a| a.as_str()), sum.namespaced_features(), )