Auto merge of #1812 - alexcrichton:moar-filtering, r=huonw

Development dependencies are traversed during the resolution process, causing
them to be returned as the list of dependencies for a package in terms of
resolution. The compilation phase would then filter these out depending on the
dependency's transitivity, but this was incorrectly accounted for when the
dependency showed up multiple times in a few lists.

This commit fixes this behavior by updating the filtering logic to ensure that
only activated optional dependencies (those whose feature names are listed) are
compiled.

Closes #1805

src/cargo/core/resolver/mod.rs

@@ -182,7 +182,7 @@ fn activate(mut cx: Box<Context>,
 
     // Next, transform all dependencies into a list of possible candidates which
     // can satisfy that dependency.
-    let mut deps = try!(deps.into_iter().map(|(_dep_name, (dep, features))| {
+    let mut deps = try!(deps.into_iter().map(|(dep, features)| {
         let mut candidates = try!(registry.query(dep));
         // When we attempt versions for a package, we'll want to start at the
         // maximum version and work our way down.
@@ -430,8 +430,7 @@ fn compatible(a: &semver::Version, b: &semver::Version) -> bool {
 
 fn resolve_features<'a>(cx: &mut Context, parent: &'a Summary,
                         method: Method)
-                        -> CargoResult<HashMap<&'a str,
-                                               (&'a Dependency, Vec<String>)>> {
+                        -> CargoResult<Vec<(&'a Dependency, Vec<String>)>> {
     let dev_deps = match method {
         Method::Everything => true,
         Method::Required { dev_deps, .. } => dev_deps,
@@ -452,7 +451,7 @@ fn resolve_features<'a>(cx: &mut Context, parent: &'a Summary,
     });
 
     let (mut feature_deps, used_features) = try!(build_features(parent, method));
-    let mut ret = HashMap::new();
+    let mut ret = Vec::new();
 
     // Next, sanitize all requested features by whitelisting all the requested
     // features that correspond to optional dependencies
@@ -461,7 +460,7 @@ fn resolve_features<'a>(cx: &mut Context, parent: &'a Summary,
         if dep.is_optional() && !feature_deps.contains_key(dep.name()) {
             continue
         }
-        let mut base = feature_deps.remove(dep.name()).unwrap_or(vec![]);
+        let mut base = feature_deps.remove(dep.name()).unwrap_or(Vec::new());
         for feature in dep.features().iter() {
             base.push(feature.clone());
             if feature.contains("/") {
@@ -471,7 +470,7 @@ fn resolve_features<'a>(cx: &mut Context, parent: &'a Summary,
                                          feature)));
             }
         }
-        ret.insert(dep.name(), (dep, base));
+        ret.push((dep, base));
     }
 
     // All features can only point to optional dependencies, in which case they

src/cargo/ops/cargo_rustc/context.rs

@@ -351,7 +351,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
             pkg.dependencies().iter().filter(|d| {
                 d.name() == dep.name()
             }).any(|d| {
-
                 // If this target is a build command, then we only want build
                 // dependencies, otherwise we want everything *other than* build
                 // dependencies.
@@ -364,7 +363,14 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
                                     target.is_example() ||
                                     profile.test;
 
-                is_correct_dep && is_actual_dep
+                // If the dependency is optional, then we're only activating it
+                // if the corresponding feature was activated
+                let activated = !d.is_optional() ||
+                                self.resolve.features(pkg.package_id()).map(|f| {
+                                    f.contains(d.name())
+                                }).unwrap_or(false);
+
+                is_correct_dep && is_actual_dep && activated
             })
         }).filter_map(|pkg| {
             pkg.targets().iter().find(|t| t.is_lib()).map(|t| {

tests/test_cargo_features.rs

@@ -741,3 +741,31 @@ test!(unions_work_with_no_default_features {
     assert_that(p.cargo("build"), execs().with_status(0).with_stdout(""));
     assert_that(p.cargo("build"), execs().with_status(0).with_stdout(""));
 });
+
+test!(optional_and_dev_dep {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name    = "test"
+            version = "0.1.0"
+            authors = []
+
+            [dependencies]
+            foo = { path = "foo", optional = true }
+            [dev-dependencies]
+            foo = { path = "foo" }
+        "#)
+        .file("src/lib.rs", "")
+        .file("foo/Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+        "#)
+        .file("foo/src/lib.rs", "");
+
+    assert_that(p.cargo_process("build"),
+                execs().with_status(0).with_stdout(format!("\
+{compiling} test v0.1.0 ([..])
+", compiling = COMPILING)));
+});