Auto merge of #5454 - alexcrichton:fix, r=matklad

Fix optional dependencies and required dev-deps

This fixes an accidental bug introduced in #5300 by ensuring a local map keeps
track of the fact that there can be multiple dependencies for one name

Closes #5453

src/cargo/core/resolver/mod.rs

@@ -340,15 +340,18 @@ fn activate_deps_loop(
                         backtracked = true;
                         Ok((candidate, has_another))
                     }
-                    None => Err(activation_error(
-                        &cx,
-                        registry.registry,
-                        &parent,
-                        &dep,
-                        &conflicting_activations,
-                        &candidates,
-                        config,
-                    )),
+                    None => {
+                        debug!("no candidates found");
+                        Err(activation_error(
+                            &cx,
+                            registry.registry,
+                            &parent,
+                            &dep,
+                            &conflicting_activations,
+                            &candidates,
+                            config,
+                        ))
+                    }
                 }
             })?;
 

src/cargo/core/summary.rs

@@ -140,10 +140,12 @@ fn build_feature_map(
     namespaced: bool,
 ) -> CargoResult<FeatureMap> {
     use self::FeatureValue::*;
-    let dep_map: HashMap<_, _> = dependencies
-        .iter()
-        .map(|d| (d.name().as_str(), d))
-        .collect();
+    let mut dep_map = HashMap::new();
+    for dep in dependencies.iter() {
+        dep_map.entry(dep.name().as_str())
+            .or_insert(Vec::new())
+            .push(dep);
+    }
 
     let mut map = BTreeMap::new();
     for (feature, list) in features.iter() {
@@ -159,7 +161,7 @@ fn build_feature_map(
         let mut dependency_found = if namespaced {
             match dep_map.get(feature.as_str()) {
                 Some(ref dep_data) => {
-                    if !dep_data.is_optional() {
+                    if !dep_data.iter().any(|d| d.is_optional()) {
                         bail!(
                             "Feature `{}` includes the dependency of the same name, but this is \
                              left implicit in the features included by this feature.\n\
@@ -196,7 +198,9 @@ fn build_feature_map(
                     }
                 }
             };
-            let is_optional_dep = dep_data.map_or(false, |d| d.is_optional());
+            let is_optional_dep = dep_data.iter()
+                .flat_map(|d| d.iter())
+                .any(|d| d.is_optional());
             if let FeatureValue::Crate(ref dep_name) = val {
                 // If we have a dependency value, check if this is the dependency named
                 // the same as the feature that we were looking for.

src/cargo/sources/registry/index.rs

@@ -113,13 +113,19 @@ impl<'cfg> RegistryIndex<'cfg> {
             // interpretation of each line here and older cargo will simply
             // ignore the new lines.
             ret.extend(lines.filter_map(|line| {
-                self.parse_registry_package(line).ok().and_then(|v| {
-                    if online || load.is_crate_downloaded(v.0.package_id()) {
-                        Some(v)
-                    } else {
-                        None
+                let (summary, locked) = match self.parse_registry_package(line) {
+                    Ok(p) => p,
+                    Err(e) => {
+                        info!("failed to parse `{}` registry package: {}", name, e);
+                        trace!("line: {}", line);
+                        return None
                     }
-                })
+                };
+                if online || load.is_crate_downloaded(summary.package_id()) {
+                    Some((summary, locked))
+                } else {
+                    None
+                }
             }));
 
             Ok(())

tests/testsuite/features.rs

@@ -1999,3 +1999,35 @@ fn namespaced_same_name() {
         execs().with_status(0),
     );
 }
+
+#[test]
+fn only_dep_is_optional() {
+    Package::new("bar", "0.1.0").publish();
+
+    let p = project("foo")
+        .file(
+            "Cargo.toml",
+            r#"
+                [project]
+                name = "foo"
+                version = "0.0.1"
+                authors = []
+
+                [features]
+                foo = ['bar']
+
+                [dependencies]
+                bar = { version = "0.1", optional = true }
+
+                [dev-dependencies]
+                bar = "0.1"
+            "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    assert_that(
+        p.cargo("build"),
+        execs().with_status(0),
+    );
+}