FIX: Call rustdoc test with the correct cfg flags of a package.

There was a situation in which if you you had a lib that depends on a package with features, whenever you ran the tests for the package the `rustdoc test` call was failing because rustdoc was called with the root cfg flags, not the package cfg flags. This fix solves the issue by keeping track of the cfg flags per package, so the rustdoc command will be generated with the correct cfg flags.
rust-lang · bors · Nov 2, 2016 · Oct 31, 2016 · Oct 31, 2016 · Nov 1, 2016
commit e100ee96c209e88a590e4e58243f979f56bc3b88
diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs
@@ -43,8 +43,8 @@ pub struct Compilation<'cfg> {
 
     pub to_doc_test: Vec<Package>,
 
-    /// Features enabled during this compilation.
-    pub cfgs: HashSet<String>,
+    /// Features per package enabled during this compilation.
+    pub cfgs: HashMap<PackageId, HashSet<String>>,
 
     pub target: String,
 
@@ -63,7 +63,7 @@ impl<'cfg> Compilation<'cfg> {
             binaries: Vec::new(),
             extra_env: HashMap::new(),
             to_doc_test: Vec::new(),
-            cfgs: HashSet::new(),
+            cfgs: HashMap::new(),
             config: config,
             target: String::new(),
         }

diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs
@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fs;
@@ -81,7 +81,6 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
         })
     }).collect::<Vec<_>>();
 
-    let root = try!(ws.current());
     let mut cx = try!(Context::new(ws, resolve, packages, config,
                                    build_config, profiles));
 
@@ -142,19 +141,21 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
                 cx.compilation.libraries.insert(pkgid.clone(), v);
             }
         }
-    }
 
-    let root_pkg = root.package_id();
-    if let Some(feats) = cx.resolve.features(root_pkg) {
-        cx.compilation.cfgs.extend(feats.iter().map(|feat| {
-            format!("feature=\"{}\"", feat)
-        }));
+        if let Some(feats) = cx.resolve.features(&unit.pkg.package_id()) {
+            for feat in feats.iter() {
+                cx.compilation.cfgs.entry(unit.pkg.package_id().clone())
+                    .or_insert(HashSet::new())
+                    .insert(format!("feature=\"{}\"", feat));
+            }
+        }
     }
 
     for (&(ref pkg, _), output) in cx.build_state.outputs.lock().unwrap().iter() {
-        if pkg == root_pkg {
-            cx.compilation.cfgs.extend(output.cfgs.iter().cloned());
-        }
+        cx.compilation.cfgs.entry(pkg.clone())
+            .or_insert(HashSet::new())
+            .extend(output.cfgs.iter().cloned());
+
         for dir in output.library_paths.iter() {
             cx.compilation.native_dirs.insert(dir.clone());
         }

diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs
@@ -145,8 +145,10 @@ fn run_doc_tests(options: &TestOptions,
                 p.arg("--test-args").arg(&test_args.join(" "));
             }
 
-            for cfg in compilation.cfgs.iter() {
-                p.arg("--cfg").arg(cfg);
+            if let Some(cfgs) = compilation.cfgs.get(&package.package_id()) {
+                for cfg in cfgs.iter() {
+                    p.arg("--cfg").arg(cfg);
+                }
             }
 
             for (_, libs) in compilation.libraries.iter() {

diff --git a/tests/test.rs b/tests/test.rs
@@ -2262,3 +2262,73 @@ fn panic_abort_multiple() {
                  .arg("-p").arg("a"),
                 execs().with_status(0));
 }
+
+#[test]
+fn pass_correct_cfgs_flags_to_rustdoc() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.1.0"
+            authors = []
+
+            [features]
+            default = ["a/default"]
+            nightly = ["a/nightly"]
+
+            [dependencies.a]
+            path = "libs/a"
+            default-features = false
+        "#)
+        .file("src/lib.rs", r#"
+            #[cfg(test)]
+            mod tests {
+                #[test]
+                fn it_works() {
+                  assert!(true);
+                }
+            }
+        "#)
+        .file("libs/a/Cargo.toml", r#"
+            [package]
+            name = "a"
+            version = "0.1.0"
+            authors = []
+
+            [features]
+            default = ["serde_codegen"]
+            nightly = ["serde_derive"]
+
+            [dependencies]
+            serde_derive = { version = "0.8", optional = true }
+
+            [build-dependencies]
+            serde_codegen = { version = "0.8", optional = true }
+        "#)
+        .file("libs/a/src/lib.rs", r#"
+            #[cfg(feature = "serde_derive")]
+            const MSG: &'static str = "This is safe";
+
+            #[cfg(feature = "serde_codegen")]
+            const MSG: &'static str = "This is risky";
+
+            pub fn get() -> &'static str {
+                MSG
+            }
+        "#);
+
+    assert_that(p.cargo_process("test")
+                .arg("--package").arg("a")
+                .arg("--verbose"),
+                execs().with_status(0)
+                       .with_stderr_contains("\
+[DOCTEST] a
+[RUNNING] `rustdoc --test [..]--cfg feature=\\\"serde_codegen\\\"[..]`"));
+
+    assert_that(p.cargo_process("test")
+                .arg("--verbose"),
+                execs().with_status(0)
+                       .with_stderr_contains("\
+[DOCTEST] foo
+[RUNNING] `rustdoc --test [..]--cfg feature=\\\"a\\\"[..]`"));
+}