Target features #1197

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::<String, Vec<String>>::new(),
+        &BTreeMap::<String, (Option<Platform>, Vec<String>)>::new(),
         None::<String>,
         false,
     )
@@ -571,7 +571,7 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
     Summary::new(
         name.to_pkgid(),
         dep,
-        &BTreeMap::<String, Vec<String>>::new(),
+        &BTreeMap::<String, (Option<Platform>, Vec<String>)>::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::<String, Vec<String>>::new(),
+        &BTreeMap::<String, (Option<Platform>, Vec<String>)>::new(),
         link,
         false,
     )
@@ -613,7 +613,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
     Summary::new(
         sum.package_id(),
         deps,
-        &BTreeMap::<String, Vec<String>>::new(),
+        &BTreeMap::<String, (Option<Platform>, Vec<String>)>::new(),
         sum.links().map(|a| a.as_str()),
         sum.namespaced_features(),
     )

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())
     }

src/cargo/core/compiler/context/mod.rs

@@ -205,15 +205,17 @@ 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
                     .entry(unit.pkg.package_id())
                     .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()
                     });
             }

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| {

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();

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) {

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<Platform>,
 }
 
-#[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<S>(&self, s: S) -> Result<S::Ok, S::Error>
-    where
-        S: ser::Serializer,
-    {
-        self.to_string().serialize(s)
-    }
-}
-
-impl FromStr for Platform {
-    type Err = failure::Error;
-
-    fn from_str(s: &str) -> CargoResult<Platform> {
-        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),
-        }
-    }
-}

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;

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::<RefFeatureMap<'_>>();
         // 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,

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<PackageId, FeaturesSet>,
+    pub resolve_features: im_rc::HashMap<PackageId, FeaturesMap>,
     /// get the package that will be linking to a native library by its links attribute
     pub links: im_rc::HashMap<InternedString, PackageId>,
     /// 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 => {