Auto merge of rust-lang#3242 - jhbabon:cfgs-flags-per-package, r=alex…

…crichton

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 it 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.

Closes rust-lang#3172

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

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,19 @@ 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()) {
+            cx.compilation.cfgs.entry(unit.pkg.package_id().clone())
+                .or_insert(HashSet::new())
+                .extend(feats.iter().map(|feat| 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());
         }

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

src/doc/machine-readable-output.md

@@ -7,7 +7,7 @@ Cargo can output information about project and build in JSON format.
 You can use `cargo metadata` command to get information about project structure
 and dependencies. The output of the command looks like this:
 
-```
+```text
 {
   // Integer version number of the format.
   "version": integer,
@@ -59,7 +59,7 @@ without waiting for the whole build to finish.
 
 The message format looks like this:
 
-```
+```text
 {
   // Type of the message.
   "reason": "compiler-message",

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 = ["feature_a/default"]
+            nightly = ["feature_a/nightly"]
+
+            [dependencies.feature_a]
+            path = "libs/feature_a"
+            default-features = false
+        "#)
+        .file("src/lib.rs", r#"
+            #[cfg(test)]
+            mod tests {
+                #[test]
+                fn it_works() {
+                  assert!(true);
+                }
+            }
+        "#)
+        .file("libs/feature_a/Cargo.toml", r#"
+            [package]
+            name = "feature_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/feature_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("feature_a")
+                .arg("--verbose"),
+                execs().with_status(0)
+                       .with_stderr_contains("\
+[DOCTEST] feature_a
+[RUNNING] `rustdoc --test [..]serde_codegen[..]`"));
+
+    assert_that(p.cargo_process("test")
+                .arg("--verbose"),
+                execs().with_status(0)
+                       .with_stderr_contains("\
+[DOCTEST] foo
+[RUNNING] `rustdoc --test [..]feature_a[..]`"));
+}