diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index dce94689eb3..b410d56df0e 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -12,7 +12,7 @@ use cargo::core::resolver::{self, ResolveOpts}; use cargo::core::source::{GitReference, SourceId}; use cargo::core::Resolve; use cargo::core::{Dependency, PackageId, Registry, Summary}; -use cargo::util::{CargoResult, Config, Graph, IntoUrl}; +use cargo::util::{CargoResult, Config, Graph, IntoUrl, Platform}; use proptest::collection::{btree_map, vec}; use proptest::prelude::*; @@ -170,7 +170,7 @@ pub fn resolve_with_config_raw( let summary = Summary::new( pkg_id("root"), deps, - &BTreeMap::>::new(), + &BTreeMap::, Vec)>::new(), None::, false, ) @@ -571,7 +571,7 @@ pub fn pkg_dep(name: T, dep: Vec) -> Summary { Summary::new( name.to_pkgid(), dep, - &BTreeMap::>::new(), + &BTreeMap::, Vec)>::new(), link, false, ) @@ -599,7 +599,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, ) @@ -613,7 +613,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(), ) diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index 18956632152..3e67718fbfe 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -10,7 +10,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, Config, Rustc}; +use crate::util::{profile, Cfg, Config, Platform, Rustc}; mod target_info; pub use self::target_info::{FileFlavor, TargetInfo}; @@ -95,12 +95,10 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { .is_public_dep(unit.pkg.package_id(), dep.pkg.package_id()) } - /// 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, }; @@ -111,7 +109,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) + } + + /// Gets 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 7ab5f5b74b1..24661b9d189 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -205,7 +205,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 @@ -213,7 +214,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 6e80f31a195..6b68e202cec 100644 --- a/src/cargo/core/compiler/context/unit_dependencies.rs +++ b/src/cargo/core/compiler/context/unit_dependencies.rs @@ -162,6 +162,19 @@ fn compute_deps<'a, 'cfg, 'tmp>( return false; } + // If the dependency is optional, then we're only activating it + // if the corresponding feature was activated + 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 // actually used! true @@ -228,7 +241,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 7978d1d480e..f991e961b3e 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -182,7 +182,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 efe332a93eb..795e2d148dc 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -18,7 +18,7 @@ use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::Arc; -use failure::Error; +use failure::{bail, Error}; use lazycell::LazyCell; use log::debug; use same_file::is_same_file; @@ -630,8 +630,16 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult rustdoc.arg("-o").arg(doc_dir); + // Need to keep a correct order on the features, so get the sorted name first, + // then resolve the specified platform. for feat in bcx.resolve.features_sorted(unit.pkg.package_id()) { - rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); + if let Some(platform) = bcx.resolve.features(unit.pkg.package_id()).get(feat) { + if bcx.platform_activated(platform.as_ref(), unit.kind) { + rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); + } + } else { + bail!("Failed to get the target for the feature `{}`", feat); + } } add_error_format_and_color(cx, &mut rustdoc, false)?; @@ -897,7 +905,13 @@ 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 let Some(platform) = bcx.resolve.features(unit.pkg.package_id()).get(feat) { + if bcx.platform_activated(platform.as_ref(), unit.kind) { + cmd.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); + } + } else { + bail!("Failed to get the target for the feature `{}`", feat); + } } match cx.files().metadata(unit) { diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 1da54590ef4..27859338666 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::{Config, Platform}; /// 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, @@ -457,46 +449,3 @@ impl Dependency { } } } - -impl Platform { - pub fn matches(&self, name: &str, cfg: &[Cfg]) -> bool { - match *self { - Platform::Name(ref p) => p == name, - Platform::Cfg(ref p) => p.matches(cfg), - } - } -} - -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 9e285b6c266..45a2b1c9e38 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -13,7 +13,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, FeatureValue, RefFeatureMap, 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 01a9864cec9..9cf2fcbbf80 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::config::PackageCacheLock; use crate::util::errors::{CargoResult, CargoResultExt, HttpNot200}; @@ -64,7 +64,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], @@ -94,6 +94,12 @@ 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); + // Avoid leaking platform info into the metadata. + 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. @@ -114,7 +120,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 27b9a0585eb..f4185c6ad64 100644 --- a/src/cargo/core/resolver/context.rs +++ b/src/cargo/core/resolver/context.rs @@ -13,7 +13,7 @@ use crate::util::CargoResult; use crate::util::Graph; use super::dep_cache::RegistryQueryer; -use super::types::{ConflictMap, FeaturesSet, ResolveOpts}; +use super::types::{ConflictMap, FeaturesMap, ResolveOpts}; pub use super::encode::Metadata; pub use super::encode::{EncodableDependency, EncodablePackageId, EncodableResolve}; @@ -27,7 +27,7 @@ pub use super::resolve::Resolve; pub struct Context { pub activations: Activations, /// list the features that are activated for each package - pub resolve_features: im_rc::HashMap, + pub resolve_features: im_rc::HashMap, /// get the package that will be linking to a native library by its links attribute pub links: im_rc::HashMap, /// for each package the list of names it can see, @@ -165,9 +165,13 @@ impl Context { let has_default_feature = summary.features().contains_key("default"); Ok(match self.resolve_features.get(&id) { Some(prev) => { - opts.features.is_subset(prev) + opts.features + .keys() + .filter(|k| !prev.contains_key(k.as_str())) + .next() + .is_none() && (!opts.uses_default_features - || prev.contains("default") + || prev.contains_key("default") || !has_default_feature) } None => { diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index e20a78a66ae..05ad7adbbb5 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -10,7 +10,7 @@ //! This module impl that cache in all the gory details use std::cmp::Ordering; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::rc::Rc; use log::debug; @@ -18,8 +18,9 @@ use log::debug; use crate::core::interning::InternedString; use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, Summary}; use crate::util::errors::CargoResult; +use crate::util::Platform; -use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesSet}; +use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesMap}; use crate::core::resolver::{ActivateResult, ResolveOpts}; pub struct RegistryQueryer<'a> { @@ -35,7 +36,7 @@ pub struct RegistryQueryer<'a> { /// a cache of `Dependency`s that are required for a `Summary` summary_cache: HashMap< (Option, Summary, ResolveOpts), - Rc<(HashSet, Rc>)>, + Rc<(HashMap>, Rc>)>, >, /// all the cases we ended up using a supplied replacement used_replacements: HashMap, @@ -200,7 +201,7 @@ impl<'a> RegistryQueryer<'a> { parent: Option, candidate: &Summary, opts: &ResolveOpts, - ) -> ActivateResult, Rc>)>> { + ) -> ActivateResult>, Rc>)>> { // if we have calculated a result before, then we can just return it, // as it is a "pure" query of its arguments. if let Some(out) = self @@ -248,7 +249,10 @@ pub fn resolve_features<'b>( parent: Option, s: &'b Summary, opts: &'b ResolveOpts, -) -> ActivateResult<(HashSet, Vec<(Dependency, FeaturesSet)>)> { +) -> ActivateResult<( + HashMap>, + Vec<(Dependency, FeaturesMap)>, +)> { // First, filter by dev-dependencies. let deps = s.dependencies(); let deps = deps.iter().filter(|d| d.is_transitive() || opts.dev_deps); @@ -256,7 +260,7 @@ pub fn resolve_features<'b>( let reqs = build_requirements(s, opts)?; let mut ret = Vec::new(); let mut used_features = HashSet::new(); - let default_dep = (false, BTreeSet::new()); + let default_dep = (false, BTreeMap::new()); // Next, collect all actually enabled dependencies and their features. for dep in deps { @@ -292,12 +296,16 @@ pub fn resolve_features<'b>( }); } let mut base = base.1.clone(); - base.extend(dep.features().iter()); + base.extend( + dep.features() + .iter() + .map(|f| (f.clone(), dep.platform().map(|p| p.clone()))), + ); for feature in base.iter() { - if feature.contains('/') { + if feature.0.contains('/') { return Err(failure::format_err!( "feature names may not contain slashes: `{}`", - feature + feature.0 ) .into()); } @@ -339,23 +347,34 @@ fn build_requirements<'a, 'b: 'a>( opts: &'b ResolveOpts, ) -> CargoResult> { let mut reqs = Requirements::new(s); + let empty_ftrs = (None, vec![]); if opts.all_features { - for key in s.features().keys() { - reqs.require_feature(*key)?; + for (key, val) in s.features() { + reqs.require_feature(*key, val.0.as_ref())?; } for dep in s.dependencies().iter().filter(|d| d.is_optional()) { - reqs.require_dependency(dep.name_in_toml()); + reqs.require_dependency(dep.name_in_toml(), dep.platform()); } } else { - for &f in opts.features.iter() { - reqs.require_value(&FeatureValue::new(f, s))?; + for (f, _) in opts.features.iter() { + reqs.require_value( + &FeatureValue::new(*f, s), + s.features().get(f).unwrap_or(&empty_ftrs).0.as_ref(), + )?; } } if opts.uses_default_features { if s.features().contains_key("default") { - reqs.require_feature(InternedString::new("default"))?; + reqs.require_feature( + InternedString::new("default"), + s.features() + .get("default") + .unwrap_or(&empty_ftrs) + .0 + .as_ref(), + )?; } } @@ -369,12 +388,12 @@ struct Requirements<'a> { // specified set of features enabled. The boolean indicates whether this // package was specifically requested (rather than just requesting features // *within* this package). - deps: HashMap)>, + deps: HashMap>)>, // 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 Requirements<'_> { @@ -382,16 +401,21 @@ impl Requirements<'_> { Requirements { summary, deps: HashMap::new(), - used: HashSet::new(), - visited: HashSet::new(), + used: HashMap::new(), + visited: HashMap::new(), } } - fn into_used(self) -> HashSet { + fn into_used(self) -> HashMap> { self.used } - fn require_crate_feature(&mut self, package: InternedString, feat: InternedString) { + fn require_crate_feature( + &mut self, + package: InternedString, + feat: InternedString, + platform: Option<&Platform>, + ) { // If `package` is indeed an optional dependency then we activate the // feature named `package`, but otherwise if `package` is a required // dependency then there's no feature associated with it. @@ -402,42 +426,46 @@ impl Requirements<'_> { .find(|p| p.name_in_toml() == package) { if dep.is_optional() { - self.used.insert(package); + self.used.insert(package, platform.cloned()); } } self.deps .entry(package) - .or_insert((false, BTreeSet::new())) + .or_insert((false, BTreeMap::new())) .1 - .insert(feat); + .insert(feat, platform.cloned()); } - fn seen(&mut self, feat: InternedString) -> bool { - if self.visited.insert(feat) { - self.used.insert(feat); + fn seen(&mut self, feat: InternedString, platform: Option<&Platform>) -> bool { + if self.visited.insert(feat, platform.cloned()).is_none() { + self.used.insert(feat, platform.cloned()); false } else { true } } - fn require_dependency(&mut self, pkg: InternedString) { - if self.seen(pkg) { + fn require_dependency(&mut self, pkg: InternedString, platform: Option<&Platform>) { + if self.seen(pkg, platform) { return; } - self.deps.entry(pkg).or_insert((false, BTreeSet::new())).0 = true; + self.deps.entry(pkg).or_insert((false, BTreeMap::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 + let feature = self .summary .features() .get(feat.as_str()) - .expect("must be a valid feature") - { + .expect("must be a valid feature"); + for fv in feature.1.as_slice() { match *fv { FeatureValue::Feature(ref dep_feat) if **dep_feat == *feat => failure::bail!( "cyclic feature dependency: feature `{}` depends on itself", @@ -445,17 +473,25 @@ impl Requirements<'_> { ), _ => {} } - self.require_value(fv)?; + self.require_value(fv, feature.0.as_ref())?; } Ok(()) } - fn require_value(&mut self, fv: &FeatureValue) -> CargoResult<()> { + fn require_value(&mut self, fv: &FeatureValue, platform: Option<&Platform>) -> CargoResult<()> { match fv { - FeatureValue::Feature(feat) => self.require_feature(*feat)?, - FeatureValue::Crate(dep) => self.require_dependency(*dep), + FeatureValue::Feature(feat) => self.require_feature( + *feat, + self.summary + .features() + .get(feat) + .expect("must be a valid feature") + .0 + .as_ref(), + )?, + FeatureValue::Crate(dep) => self.require_dependency(*dep, platform), 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 4aaa7eeafed..14a4c4d87dc 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -63,7 +63,7 @@ use crate::util::profile; use self::context::Context; use self::dep_cache::RegistryQueryer; use self::types::{ConflictMap, ConflictReason, DepsFrame}; -use self::types::{FeaturesSet, RcVecIter, RemainingDeps, ResolverProgress}; +use self::types::{FeaturesMap, RcVecIter, RemainingDeps, ResolverProgress}; pub use self::encode::Metadata; pub use self::encode::{EncodableDependency, EncodablePackageId, EncodableResolve}; @@ -146,7 +146,12 @@ pub fn resolve( cx.resolve_replacements(®istry), cx.resolve_features .iter() - .map(|(k, v)| (*k, v.iter().map(|x| x.to_string()).collect())) + .map(|(k, v)| { + ( + *k, + v.iter().map(|(f, p)| (f.to_string(), p.clone())).collect(), + ) + }) .collect(), cksums, BTreeMap::new(), @@ -691,7 +696,7 @@ fn activate( .entry(candidate.package_id()) .or_insert_with(Rc::default), ) - .extend(used_features); + .extend(used_features.clone()); } let frame = DepsFrame { @@ -709,7 +714,7 @@ struct BacktrackFrame { remaining_candidates: RemainingCandidates, parent: Summary, dep: Dependency, - features: FeaturesSet, + features: FeaturesMap, conflicting_activations: ConflictMap, } diff --git a/src/cargo/core/resolver/resolve.rs b/src/cargo/core/resolver/resolve.rs index 9ced48f4d67..7f210a1e812 100644 --- a/src/cargo/core/resolver/resolve.rs +++ b/src/cargo/core/resolver/resolve.rs @@ -8,7 +8,7 @@ use url::Url; use crate::core::dependency::Kind; 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; @@ -27,12 +27,12 @@ pub struct Resolve { replacements: HashMap, /// Inverted version of `replacements`. reverse_replacements: HashMap, - /// An empty `HashSet` to avoid creating a new `HashSet` for every package + /// An empty `HashMap` to avoid creating a new `HashMap` for every package /// that does not have any features, and to avoid using `Option` to /// simplify the API. - empty_features: HashSet, + empty_features: HashMap>, /// Features enabled for a given package. - features: HashMap>, + features: HashMap>>, /// Checksum for each package. A SHA256 hash of the `.crate` file used to /// validate the correct crate file is used. This is `None` for sources /// that do not use `.crate` files, like path or git dependencies. @@ -71,7 +71,7 @@ impl Resolve { pub fn new( graph: Graph>, replacements: HashMap, - features: HashMap>, + features: HashMap>>, checksums: HashMap>, metadata: Metadata, unused_patches: Vec, @@ -101,7 +101,7 @@ impl Resolve { checksums, metadata, unused_patches, - empty_features: HashSet::new(), + empty_features: HashMap::new(), reverse_replacements, public_dependencies, version, @@ -269,7 +269,7 @@ 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) } @@ -281,7 +281,7 @@ unable to verify that `{0}` is the same as when the lockfile was generated } pub fn features_sorted(&self, pkg: PackageId) -> Vec<&str> { - let mut v = Vec::from_iter(self.features(pkg).iter().map(|s| s.as_ref())); + let mut v = Vec::from_iter(self.features(pkg).iter().map(|(s, _)| s.as_ref())); v.sort_unstable(); v } diff --git a/src/cargo/core/resolver/types.rs b/src/cargo/core/resolver/types.rs index 881869ef16f..02f078460d9 100644 --- a/src/cargo/core/resolver/types.rs +++ b/src/cargo/core/resolver/types.rs @@ -1,5 +1,5 @@ use std::cmp::Ordering; -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use std::ops::Range; use std::rc::Rc; use std::time::{Duration, Instant}; @@ -7,7 +7,7 @@ use std::time::{Duration, Instant}; use crate::core::interning::InternedString; use crate::core::{Dependency, PackageId, Summary}; use crate::util::errors::CargoResult; -use crate::util::Config; +use crate::util::{Config, Platform}; use im_rc; @@ -89,7 +89,7 @@ impl ResolverProgress { } } -/// The preferred way to store the set of activated features for a package. +/// The preferred way to store the map of activated feature-platform pairs for a package. /// This is sorted so that it impls Hash, and owns its contents, /// needed so it can be part of the key for caching in the `DepsCache`. /// It is also cloned often as part of `Context`, hence the `RC`. @@ -97,7 +97,7 @@ impl ResolverProgress { /// but this can change with improvements to std, im, or llvm. /// Using a consistent type for this allows us to use the highly /// optimized comparison operators like `is_subset` at the interfaces. -pub type FeaturesSet = Rc>; +pub type FeaturesMap = Rc>>; /// Options for how the resolve should work. #[derive(Clone, Debug, Eq, PartialEq, Hash)] @@ -107,7 +107,7 @@ pub struct ResolveOpts { /// This may be set to `false` by things like `cargo install` or `-Z avoid-dev-deps`. pub dev_deps: bool, /// Set of features to enable (`--features=…`). - pub features: FeaturesSet, + pub features: FeaturesMap, /// Indicates *all* features should be enabled (`--all-features`). pub all_features: bool, /// Include the `default` feature (`--no-default-features` sets this false). @@ -119,7 +119,7 @@ impl ResolveOpts { pub fn everything() -> ResolveOpts { ResolveOpts { dev_deps: true, - features: Rc::new(BTreeSet::new()), + features: Rc::new(BTreeMap::new()), all_features: true, uses_default_features: true, } @@ -139,14 +139,14 @@ impl ResolveOpts { } } - fn split_features(features: &[String]) -> BTreeSet { + fn split_features(features: &[String]) -> BTreeMap> { features .iter() .flat_map(|s| s.split_whitespace()) .flat_map(|s| s.split(',')) .filter(|s| !s.is_empty()) - .map(InternedString::new) - .collect::>() + .map(|s| (InternedString::new(s), None)) + .collect::>>() } } @@ -253,7 +253,7 @@ impl RemainingDeps { /// Information about the dependencies for a crate, a tuple of: /// /// (dependency info, candidates, features activated) -pub type DepInfo = (Dependency, Rc>, FeaturesSet); +pub type DepInfo = (Dependency, Rc>, FeaturesMap); /// All possible reasons that a package might fail to activate. /// diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index fb52b9179d7..0dbfd2456b3 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -11,7 +11,7 @@ use crate::core::interning::InternedString; use crate::core::{Dependency, PackageId, SourceId}; use semver::Version; -use crate::util::CargoResult; +use crate::util::{CargoResult, Platform}; /// Subset of a `Manifest`. Contains only the most important information about /// a package. @@ -36,7 +36,7 @@ impl Summary { pub fn new( pkg_id: PackageId, dependencies: Vec, - features: &BTreeMap>>, + features: &BTreeMap, Vec>)>, links: Option>, namespaced_features: bool, ) -> CargoResult @@ -149,7 +149,7 @@ impl Hash 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 @@ -201,7 +201,7 @@ 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()), @@ -239,7 +239,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 @@ -337,7 +337,10 @@ where ) } - map.insert(InternedString::new(feature.borrow()), values); + map.insert( + InternedString::new(feature.borrow()), + (list.0.clone(), values), + ); } Ok(map) } @@ -416,4 +419,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 c68d4fc1b1d..653b1a6e8e0 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -36,7 +36,7 @@ use crate::core::{Package, Target}; use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace}; use crate::ops; use crate::util::config::Config; -use crate::util::{closest_msg, profile, CargoResult}; +use crate::util::{closest_msg, profile, CargoResult, Platform}; /// Contains information about how a package should be compiled. #[derive(Debug)] @@ -791,7 +791,7 @@ fn generate_targets<'a>( let features = features_map .entry(pkg) .or_insert_with(|| resolve_all_features(resolve, pkg.package_id())); - rf.iter().filter(|f| !features.contains(*f)).collect() + rf.iter().filter(|f| !features.contains_key(*f)).collect() } None => Vec::new(), }; @@ -821,14 +821,14 @@ fn generate_targets<'a>( fn resolve_all_features( resolve_with_overrides: &Resolve, package_id: PackageId, -) -> HashSet { +) -> HashMap> { let mut features = resolve_with_overrides.features(package_id).clone(); // 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, feature.1.clone()); } } diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index 5ecdaf6f53c..20d7aa01ea8 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -237,7 +237,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 67116074cb7..57d24a8f0aa 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -722,7 +722,11 @@ impl IndexSummary { .into_iter() .map(|dep| dep.into_dep(source_id)) .collect::>>()?; - let mut summary = Summary::new(pkgid, deps, &features, links, false)?; + let ftrs = features + .iter() + .map(|(k, v)| (k.clone(), (None, v.clone()))) + .collect(); + let mut summary = Summary::new(pkgid, deps, &ftrs, links, false)?; summary.set_checksum(cksum.clone()); Ok(IndexSummary { summary, diff --git a/src/cargo/util/cfg.rs b/src/cargo/util/cfg.rs index 4c4ad232df4..4826c3b4bce 100644 --- a/src/cargo/util/cfg.rs +++ b/src/cargo/util/cfg.rs @@ -2,6 +2,9 @@ use std::fmt; use std::iter; use std::str::{self, FromStr}; +use serde::ser; + +use crate::util::errors::CargoResultExt; use crate::util::CargoResult; #[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)] @@ -28,6 +31,12 @@ enum Token<'a> { String(&'a str), } +#[derive(Eq, PartialEq, Hash, Ord, PartialOrd, Clone, Debug)] +pub enum Platform { + Name(String), + Cfg(CfgExpr), +} + struct Tokenizer<'a> { s: iter::Peekable>, orig: &'a str, @@ -276,3 +285,46 @@ impl<'a> Token<'a> { } } } + +impl Platform { + pub fn matches(&self, name: &str, cfg: &[Cfg]) -> bool { + match *self { + Platform::Name(ref p) => p == name, + Platform::Cfg(ref p) => p.matches(cfg), + } + } +} + +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/util/mod.rs b/src/cargo/util/mod.rs index 50fbd8c2b1b..424b525f4cc 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; pub use self::diagnostic_server::RustfixDiagnosticServer; diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index e2232827885..3633ff25b13 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}; @@ -21,7 +21,7 @@ use crate::core::{Edition, EitherManifest, Feature, Features, VirtualManifest}; use crate::core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig}; use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY}; use crate::util::errors::{CargoResult, CargoResultExt, ManifestError}; -use crate::util::{self, paths, validate_package_name, Config, IntoUrl}; +use crate::util::{self, paths, validate_package_name, Config, IntoUrl, 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), )?; @@ -1543,6 +1555,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..db028e73c9f --- /dev/null +++ b/tests/testsuite/cfg_features.rs @@ -0,0 +1,363 @@ +use crate::support::project; + +#[cargo_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(); +} + +#[cargo_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(); +} + +#[cargo_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(); +} + +#[cargo_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(); +} + +#[cargo_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(); +} + +#[cargo_test] +fn transitive() { + #[cfg(target_os = "macos")] + let config = "target_os = \"macos\""; + #[cfg(target_os = "windows")] + let config = "target_os = \"windows\""; + #[cfg(all(not(target_os = "macos"), not(target_os = "windows")))] + let config = "unix"; + + let p = project() + .no_manifest() + // root depends on a and c="1.1.0" + .file( + "root/Cargo.toml", + r#" + [package] + name = "root" + version = "0.0.1" + authors = [] + + [dependencies] + a = { version = "*", path = "../a" } + c = { version = "1.1.0", path = "../c1" } + "#, + ) + .file( + "root/src/main.rs", + r#" + fn main() { + println!("Hello, world!"); + } + "#, + ) + // a depends on b and on OSX depends on b's flag maybe + .file( + "a/Cargo.toml", + &format!( + r#" + [package] + name = "a" + version = "0.1.0" + + [lib] + name = "a" + + [target.'cfg(not({}))'.dependencies] + b = {{ version = "*", path = "../b" }} + + [target.'cfg({})'.dependencies] + b = {{ version = "*", path = "../b", features = ["maybe"] }} + "#, + config, config, + ), + ) + .file( + "a/src/lib.rs", + r#" + "#, + ) + // b depends on c="=1.0.0" if maybe is active. + .file( + "b/Cargo.toml", + r#" + [package] + name = "b" + version = "0.1.0" + + [dependencies] + c = { version = "1.0.0", path = "../c0", optional = true } + + [features] + maybe = ["c"] + "#, + ) + .file( + "b/src/lib.rs", + r#" + #[cfg(feature = "maybe")] + pub fn maybe() { + c::cee(); + } + "#, + ) + // c 1.0.0 + .file( + "c0/Cargo.toml", + r#" + [package] + name = "c" + version = "1.0.0" + + [lib] + name = "c" + + [dependencies] + "#, + ) + .file( + "c0/src/lib.rs", + r#" + pub fn cee() {} + "#, + ) + // c 1.1.0 + .file( + "c1/Cargo.toml", + r#" + [package] + name = "c" + version = "1.1.0" + + [lib] + name = "c" + + [dependencies] + "#, + ) + .file( + "c1/src/lib.rs", + r#" + "#, + ) + .build(); + + p.cargo("build") + .cwd("root") + .with_stderr( + "\ +[COMPILING] c v1.0.0 ([..]) +[COMPILING] c v1.1.0 ([..]) +[COMPILING] b v0.1.0 ([..]) +[COMPILING] a v0.1.0 ([..]) +[COMPILING] root v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + +// https://github.com/rust-lang/cargo/issues/5313 +#[cargo_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 618c92ceb23..a8a2d7cc309 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -27,6 +27,7 @@ mod cargo_alias_config; mod cargo_command; mod cargo_features; mod cfg; +mod cfg_features; mod check; mod clean; mod clippy;