From c445aa354ae9ec411df63734d5e5fb7e27f451ed Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 16 Feb 2017 08:04:09 -0800 Subject: [PATCH] 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 --- src/cargo/ops/cargo_rustc/compilation.rs | 8 ++--- src/cargo/ops/cargo_rustc/mod.rs | 42 +++++++++++------------- src/cargo/ops/cargo_test.rs | 39 +++++++++++----------- tests/test.rs | 42 ++++++++++++++++++++++++ 4 files changed, 84 insertions(+), 47 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/compilation.rs b/src/cargo/ops/cargo_rustc/compilation.rs index afb896258..e1b44de78 100644 --- 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>, + /// A mapping from a package to the list of libraries that need to be + /// linked when working with that package. + pub libraries: HashMap>, /// 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 3fb4033ed..fd8c379a3 100644 --- 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::>(); - 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 6c267efb6..539265430 100644 --- 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 8e0b9128a..59be240b0 100644 --- 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)); +} -- 2.39.5