Auto merge of #3242 - jhbabon:cfgs-flags-per-package, r=alexcrichton

bors · web-flow · commit eca9e159b6b0 · 2016-11-01T16:06:35.000-07:00
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 #3172
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,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());
         }
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/src/doc/machine-readable-output.md b/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",
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 = ["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[..]`"));
+}

Original file line number	Diff line number	Diff line change
`@@ -145,8 +145,10 @@ fn run_doc_tests(options: &TestOptions,`
`145`	`145`	`p.arg("--test-args").arg(&test_args.join(" "));`
`146`	`146`	`}`
`147`	`147`
`148`		`- for cfg in compilation.cfgs.iter() {`
`149`		`- p.arg("--cfg").arg(cfg);`
	`148`	`+ if let Some(cfgs) = compilation.cfgs.get(&package.package_id()) {`
	`149`	`+ for cfg in cfgs.iter() {`
	`150`	`+ p.arg("--cfg").arg(cfg);`
	`151`	`+ }`
`150`	`152`	`}`
`151`	`153`
`152`	`154`	`for (_, libs) in compilation.libraries.iter() {`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ Cargo can output information about project and build in JSON format.`
`7`	`7`	You can use `cargo metadata` command to get information about project structure
`8`	`8`	`and dependencies. The output of the command looks like this:`
`9`	`9`
`10`		-```
	`10`	+```text
`11`	`11`	`{`
`12`	`12`	`// Integer version number of the format.`
`13`	`13`	`"version": integer,`
`@@ -59,7 +59,7 @@ without waiting for the whole build to finish.`
`59`	`59`
`60`	`60`	`The message format looks like this:`
`61`	`61`
`62`		-```
	`62`	+```text
`63`	`63`	`{`
`64`	`64`	`// Type of the message.`
`65`	`65`	`"reason": "compiler-message",`