Fix deps with `cargo test --all` and doctests

This commit fixes `cargo test --all` with the way we ship libraries to `rustdoc
--test`. I'm... not entirely sure what the previous incarnation was doing but
the current organization is to interpret `compilation.libraries` as a mapping
from a package to the list of crates it needs to link to test.

This updates the support in `cargo_rustc/mod.rs` to create the map appropriately
and tweaks the loop in `cargo_test.rs` as well.

Closes rust-lang/rust#39879

diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs
index afb8962589b1e1e4adcd2c8eee0f3566e4347212..e1b44de785f20e41d394df52d54b6e9ca05ed607 100644 (file)
--- a/src/cargo/ops/cargo_rustc/compilation.rs
+++ b/src/cargo/ops/cargo_rustc/compilation.rs
@@ -8,11 +8,9 @@ use util::{self, CargoResult, Config, ProcessBuilder, process, join_paths};
 
 /// A structure returning the result of a compilation.
 pub struct Compilation<'cfg> {
-    /// All libraries which were built for a package.
-    ///
-    /// This is currently used for passing --extern flags to rustdoc tests later
-    /// on.
-    pub libraries: HashMap<PackageId, Vec<(Target, PathBuf)>>,
+    /// A mapping from a package to the list of libraries that need to be
+    /// linked when working with that package.
+    pub libraries: HashMap<PackageId, HashSet<(Target, PathBuf)>>,
 
     /// An array of all tests created during this compilation.
     pub tests: Vec<(Package, String, PathBuf)>,

diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs
index 3fb4033ed21c64319682afdd0194d7a315431b39..fd8c379a311d638a4edfd812f95f00a86c96fc5d 100644 (file)
--- a/src/cargo/ops/cargo_rustc/mod.rs
+++ b/src/cargo/ops/cargo_rustc/mod.rs
@@ -153,33 +153,31 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
                 cx.compilation.binaries.push(bindst);
             } else if unit.target.is_lib() {
                 let pkgid = unit.pkg.package_id().clone();
-                cx.compilation.libraries.entry(pkgid).or_insert(Vec::new())
-                  .push((unit.target.clone(), dst));
+                cx.compilation.libraries.entry(pkgid).or_insert(HashSet::new())
+                  .insert((unit.target.clone(), dst));
             }
+        }
+
+        for dep in cx.dep_targets(unit)?.iter() {
             if !unit.target.is_lib() { continue }
 
-            // Include immediate lib deps as well
-            for unit in cx.dep_targets(unit)?.iter() {
-                if unit.profile.run_custom_build {
-                    let out_dir = cx.build_script_out_dir(unit).display().to_string();
-                    cx.compilation.extra_env.entry(unit.pkg.package_id().clone())
-                      .or_insert(Vec::new())
-                      .push(("OUT_DIR".to_string(), out_dir));
-                }
+            if dep.profile.run_custom_build {
+                let out_dir = cx.build_script_out_dir(dep).display().to_string();
+                cx.compilation.extra_env.entry(dep.pkg.package_id().clone())
+                  .or_insert(Vec::new())
+                  .push(("OUT_DIR".to_string(), out_dir));
+            }
 
-                let pkgid = unit.pkg.package_id();
-                if !unit.target.is_lib() { continue }
-                if unit.profile.doc { continue }
-                if cx.compilation.libraries.contains_key(&pkgid) {
-                    continue
-                }
+            if !dep.target.is_lib() { continue }
+            if dep.profile.doc { continue }
 
-                let v = cx.target_filenames(unit)?;
-                let v = v.into_iter().map(|(f, _, _)| {
-                    (unit.target.clone(), f)
-                }).collect::<Vec<_>>();
-                cx.compilation.libraries.insert(pkgid.clone(), v);
-            }
+            let v = cx.target_filenames(dep)?;
+            cx.compilation.libraries
+                .entry(unit.pkg.package_id().clone())
+                .or_insert(HashSet::new())
+                .extend(v.into_iter().map(|(f, _, _)| {
+                    (dep.target.clone(), f)
+                }));
         }
 
         if let Some(feats) = cx.resolve.features(&unit.pkg.package_id()) {

diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs
index 6c267efb60b033193e74d704da76a501b63ab086..539265430d8e5e0998d9e2d4da25b2bbb5a7fdc1 100644 (file)
--- a/src/cargo/ops/cargo_test.rs
+++ b/src/cargo/ops/cargo_test.rs
@@ -151,27 +151,26 @@ fn run_doc_tests(options: &TestOptions,
                 }
             }
 
-            for (_, libs) in compilation.libraries.iter() {
-                for &(ref target, ref lib) in libs.iter() {
-                    // Note that we can *only* doctest rlib outputs here.  A
-                    // staticlib output cannot be linked by the compiler (it just
-                    // doesn't do that). A dylib output, however, can be linked by
-                    // the compiler, but will always fail. Currently all dylibs are
-                    // built as "static dylibs" where the standard library is
-                    // statically linked into the dylib. The doc tests fail,
-                    // however, for now as they try to link the standard library
-                    // dynamically as well, causing problems. As a result we only
-                    // pass `--extern` for rlib deps and skip out on all other
-                    // artifacts.
-                    if lib.extension() != Some(OsStr::new("rlib")) &&
-                       !target.for_host() {
-                        continue
-                    }
-                    let mut arg = OsString::from(target.crate_name());
-                    arg.push("=");
-                    arg.push(lib);
-                    p.arg("--extern").arg(&arg);
+            let libs = &compilation.libraries[package.package_id()];
+            for &(ref target, ref lib) in libs.iter() {
+                // Note that we can *only* doctest rlib outputs here.  A
+                // staticlib output cannot be linked by the compiler (it just
+                // doesn't do that). A dylib output, however, can be linked by
+                // the compiler, but will always fail. Currently all dylibs are
+                // built as "static dylibs" where the standard library is
+                // statically linked into the dylib. The doc tests fail,
+                // however, for now as they try to link the standard library
+                // dynamically as well, causing problems. As a result we only
+                // pass `--extern` for rlib deps and skip out on all other
+                // artifacts.
+                if lib.extension() != Some(OsStr::new("rlib")) &&
+                   !target.for_host() {
+                    continue
                 }
+                let mut arg = OsString::from(target.crate_name());
+                arg.push("=");
+                arg.push(lib);
+                p.arg("--extern").arg(&arg);
             }
 
             config.shell().verbose(|shell| {

diff --git a/tests/test.rs b/tests/test.rs
index 8e0b9128a22a6570c8254c340d981f5daa24938c..59be240b06c9dbf9942f7719136bd1c9381b62b3 100644 (file)
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -2557,3 +2557,45 @@ fn doctest_only_with_dev_dep() {
     assert_that(p.cargo_process("test").arg("--doc").arg("-v"),
                 execs().with_status(0));
 }
+
+#[test]
+fn doctest_and_registry() {
+    let p = project("workspace")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "a"
+            version = "0.1.0"
+
+            [dependencies]
+            b = { path = "b" }
+            c = { path = "c" }
+
+            [workspace]
+        "#)
+        .file("src/lib.rs", "")
+        .file("b/Cargo.toml", r#"
+            [project]
+            name = "b"
+            version = "0.1.0"
+        "#)
+        .file("b/src/lib.rs", "
+            /// ```
+            /// b::foo();
+            /// ```
+            pub fn foo() {}
+        ")
+        .file("c/Cargo.toml", r#"
+            [project]
+            name = "c"
+            version = "0.1.0"
+
+            [dependencies]
+            b = "0.1"
+        "#)
+        .file("c/src/lib.rs", "");
+
+    Package::new("b", "0.1.0").publish();
+
+    assert_that(p.cargo_process("test").arg("--all").arg("-v"),
+                execs().with_status(0));
+}