Attempt to implement platform features #1197

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

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

@@ -226,15 +226,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

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

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

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

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<Platform>,
 }
 
-#[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<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

@@ -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;

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::<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.
@@ -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,