Auto merge of #2876 - bennofs:fix-2851, r=alexcrichton

allow enabling features for deps with --features

Fixes #2851

Author: bors

src/cargo/core/resolver/mod.rs

@@ -659,53 +659,41 @@ fn build_features(s: &Summary, method: &Method)
                    visited: &mut HashSet<String>) -> CargoResult<()> {
         if feat.is_empty() { return Ok(()) }
 
-        if !visited.insert(feat.to_string()) {
-            bail!("Cyclic feature dependency: feature `{}` depends \
-                   on itself", feat)
-        }
-
-        used.insert(feat.to_string());
-
-        // The lookup of here may fail if the feature corresponds to an optional
-        // dependency. If that is the case, we simply enable the optional dependency.
-        //
-        // Otherwise, we check what other features the feature wants and recursively
-        // add them.
-        match s.features().get(feat) {
-            Some(wanted_features) => {
-                for entry in wanted_features {
-                    // If the entry is of the form `foo/bar`, then we just lookup package
-                    // `foo` and enable its feature `bar`. We also add `foo` to the used
-                    // set because `foo` might have been an optional dependency.
-                    //
-                    // Otherwise the entry refers to another feature of our current package,
-                    // so we recurse by calling add_feature again, which may end up enabling
-                    // more features or just enabling a dependency (remember, optional
-                    // dependencies create an implicit feature with the same name).
-                    let mut parts = entry.splitn(2, '/');
-                    let feat_or_package = parts.next().unwrap();
-                    match parts.next() {
-                        Some(feat) => {
-                            let package = feat_or_package;
-                            used.insert(package.to_string());
-                            deps.entry(package.to_string())
-                                .or_insert(Vec::new())
-                                .push(feat.to_string());
-                        }
-                        None => {
-                            let feat = feat_or_package;
-                            try!(add_feature(s, feat, deps, used, visited));
+        // If this feature is of the form `foo/bar`, then we just lookup package
+        // `foo` and enable its feature `bar`. Otherwise this feature is of the
+        // form `foo` and we need to recurse to enable the feature `foo` for our
+        // own package, which may end up enabling more features or just enabling
+        // a dependency.
+        let mut parts = feat.splitn(2, '/');
+        let feat_or_package = parts.next().unwrap();
+        match parts.next() {
+            Some(feat) => {
+                let package = feat_or_package;
+                used.insert(package.to_string());
+                deps.entry(package.to_string())
+                    .or_insert(Vec::new())
+                    .push(feat.to_string());
+            }
+            None => {
+                let feat = feat_or_package;
+                if !visited.insert(feat.to_string()) {
+                    bail!("Cyclic feature dependency: feature `{}` depends \
+                           on itself", feat)
+                }
+                used.insert(feat.to_string());
+                match s.features().get(feat) {
+                    Some(recursive) => {
+                        for f in recursive {
+                            try!(add_feature(s, f, deps, used, visited));
                         }
                     }
+                    None => {
+                        deps.entry(feat.to_string()).or_insert(Vec::new());
+                    }
                 }
-            }
-            None => {
-                deps.entry(feat.to_string()).or_insert(Vec::new());
+                visited.remove(&feat.to_string());
             }
         }
-
-        visited.remove(&feat.to_string());
-
         Ok(())
     }
 }

tests/features.rs

@@ -881,3 +881,73 @@ fn activating_feature_activates_dep() {
     assert_that(p.cargo_process("build").arg("--features").arg("a").arg("-v"),
                 execs().with_status(0));
 }
+
+#[test]
+fn dep_feature_in_cmd_line() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies.derived]
+            path = "derived"
+        "#)
+        .file("src/main.rs", r#"
+            extern crate derived;
+            fn main() { derived::test(); }
+        "#)
+        .file("derived/Cargo.toml", r#"
+            [package]
+            name = "derived"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies.bar]
+            path = "../bar"
+
+            [features]
+            default = []
+            derived-feat = ["bar/some-feat"]
+        "#)
+        .file("derived/src/lib.rs", r#"
+            extern crate bar;
+            pub use bar::test;
+        "#)
+        .file("bar/Cargo.toml", r#"
+            [package]
+            name = "bar"
+            version = "0.0.1"
+            authors = []
+
+            [features]
+            some-feat = []
+        "#)
+        .file("bar/src/lib.rs", r#"
+            #[cfg(feature = "some-feat")]
+            pub fn test() { print!("test"); }
+        "#);
+
+    // The foo project requires that feature "some-feat" in "bar" is enabled.
+    // Building without any features enabled should fail:
+    assert_that(p.cargo_process("build"),
+                execs().with_status(101));
+
+    // We should be able to enable the feature "derived-feat", which enables "some-feat",
+    // on the command line. The feature is enabled, thus building should be successful:
+    assert_that(p.cargo_process("build").arg("--features").arg("derived/derived-feat"),
+                execs().with_status(0));
+
+    // Trying to enable features of transitive dependencies is an error
+    assert_that(p.cargo_process("build").arg("--features").arg("bar/some-feat"),
+                execs().with_status(101).with_stderr("\
+[ERROR] Package `foo v0.0.1 ([..])` does not have these features: `bar`
+"));
+
+    // Hierarchical feature specification should still be disallowed
+    assert_that(p.cargo_process("build").arg("--features").arg("derived/bar/some-feat"),
+                execs().with_status(101).with_stderr("\
+[ERROR] feature names may not contain slashes: `bar/some-feat`
+"));
+}