From b4c4028f769a2dbac4e547a60c1357ddb3797df2 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Mon, 22 Feb 2021 20:06:03 +0100 Subject: [PATCH] Run rustdoc doctests relative to the workspace By doing so, rustdoc will also emit workspace-relative filenames for the doctests. This was first landed in #8954 but later backed out in #8996 because it changed the CWD of rustdoc test invocations. The second try relies on the new `--test-run-directory` rustdoc option which was added in https://github.com/rust-lang/rust/pull/81264 to explicitly control the rustdoc test cwd. fixes #8993 --- src/cargo/core/compiler/fingerprint.rs | 4 +- src/cargo/core/compiler/mod.rs | 41 +------------- src/cargo/core/features.rs | 2 + src/cargo/ops/cargo_test.rs | 24 +++++--- src/cargo/util/mod.rs | 4 +- src/cargo/util/workspace.rs | 39 +++++++++++++ tests/testsuite/build_script.rs | 2 +- tests/testsuite/cross_compile.rs | 2 +- tests/testsuite/doc.rs | 76 ++++++++++++++++++++++++++ tests/testsuite/lto.rs | 2 +- tests/testsuite/rename_deps.rs | 2 +- tests/testsuite/test.rs | 3 + 12 files changed, 148 insertions(+), 53 deletions(-) diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index 76f1a3b02fd..9c776d09fc7 100644 --- a/src/cargo/core/compiler/fingerprint.rs +++ b/src/cargo/core/compiler/fingerprint.rs @@ -334,7 +334,7 @@ use crate::util; use crate::util::errors::{CargoResult, CargoResultExt}; use crate::util::interning::InternedString; use crate::util::paths; -use crate::util::{internal, profile, ProcessBuilder}; +use crate::util::{internal, path_args, profile, ProcessBuilder}; use super::custom_build::BuildDeps; use super::job::{Job, Work}; @@ -1324,7 +1324,7 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult, unit: &Unit) -> CargoResult { let mut rustdoc = cx.compilation.rustdoc_process(unit, None)?; rustdoc.inherit_jobserver(&cx.jobserver); rustdoc.arg("--crate-name").arg(&unit.target.crate_name()); - add_path_args(bcx, unit, &mut rustdoc); + add_path_args(bcx.ws, unit, &mut rustdoc); add_cap_lints(bcx, unit, &mut rustdoc); if let CompileKind::Target(target) = unit.kind { @@ -662,41 +662,6 @@ fn append_crate_version_flag(unit: &Unit, rustdoc: &mut ProcessBuilder) { .arg(unit.pkg.version().to_string()); } -// The path that we pass to rustc is actually fairly important because it will -// show up in error messages (important for readability), debug information -// (important for caching), etc. As a result we need to be pretty careful how we -// actually invoke rustc. -// -// In general users don't expect `cargo build` to cause rebuilds if you change -// directories. That could be if you just change directories in the package or -// if you literally move the whole package wholesale to a new directory. As a -// result we mostly don't factor in `cwd` to this calculation. Instead we try to -// track the workspace as much as possible and we update the current directory -// of rustc/rustdoc where appropriate. -// -// The first returned value here is the argument to pass to rustc, and the -// second is the cwd that rustc should operate in. -fn path_args(bcx: &BuildContext<'_, '_>, unit: &Unit) -> (PathBuf, PathBuf) { - let ws_root = bcx.ws.root(); - let src = match unit.target.src_path() { - TargetSourcePath::Path(path) => path.to_path_buf(), - TargetSourcePath::Metabuild => unit.pkg.manifest().metabuild_path(bcx.ws.target_dir()), - }; - assert!(src.is_absolute()); - if unit.pkg.package_id().source_id().is_path() { - if let Ok(path) = src.strip_prefix(ws_root) { - return (path.to_path_buf(), ws_root.to_path_buf()); - } - } - (src, unit.pkg.root().to_path_buf()) -} - -fn add_path_args(bcx: &BuildContext<'_, '_>, unit: &Unit, cmd: &mut ProcessBuilder) { - let (arg, cwd) = path_args(bcx, unit); - cmd.arg(arg); - cmd.cwd(cwd); -} - fn add_cap_lints(bcx: &BuildContext<'_, '_>, unit: &Unit, cmd: &mut ProcessBuilder) { // If this is an upstream dep we don't want warnings from, turn off all // lints. @@ -786,7 +751,7 @@ fn build_base_args( cmd.arg(format!("--edition={}", edition)); } - add_path_args(bcx, unit, cmd); + add_path_args(bcx.ws, unit, cmd); add_error_format_and_color(cx, cmd, cx.rmeta_required(unit)); if !test { diff --git a/src/cargo/core/features.rs b/src/cargo/core/features.rs index 494e3399300..3256f687f88 100644 --- a/src/cargo/core/features.rs +++ b/src/cargo/core/features.rs @@ -433,6 +433,7 @@ pub struct CliUnstable { pub build_std_features: Option>, pub timings: Option>, pub doctest_xcompile: bool, + pub doctest_in_workspace: bool, pub panic_abort_tests: bool, pub jobserver_per_rustc: bool, pub features: Option>, @@ -596,6 +597,7 @@ impl CliUnstable { "build-std-features" => self.build_std_features = Some(parse_features(v)), "timings" => self.timings = Some(parse_timings(v)), "doctest-xcompile" => self.doctest_xcompile = parse_empty(k, v)?, + "doctest-in-workspace" => self.doctest_in_workspace = parse_empty(k, v)?, "panic-abort-tests" => self.panic_abort_tests = parse_empty(k, v)?, "jobserver-per-rustc" => self.jobserver_per_rustc = parse_empty(k, v)?, "features" => { diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index a9f50168ca4..c0bc2106671 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -5,7 +5,7 @@ use crate::core::shell::Verbosity; use crate::core::Workspace; use crate::ops; use crate::util::errors::CargoResult; -use crate::util::{CargoTestError, Config, ProcessError, Test}; +use crate::util::{add_path_args, CargoTestError, Config, ProcessError, Test}; pub struct TestOptions { pub compile_opts: ops::CompileOptions, @@ -30,7 +30,7 @@ pub fn run_tests( return Ok(Some(CargoTestError::new(test, errors))); } - let (doctest, docerrors) = run_doc_tests(ws.config(), options, test_args, &compilation)?; + let (doctest, docerrors) = run_doc_tests(ws, options, test_args, &compilation)?; let test = if docerrors.is_empty() { test } else { doctest }; errors.extend(docerrors); if errors.is_empty() { @@ -136,13 +136,15 @@ fn run_unit_tests( } fn run_doc_tests( - config: &Config, + ws: &Workspace<'_>, options: &TestOptions, test_args: &[&str], compilation: &Compilation<'_>, ) -> CargoResult<(Test, Vec)> { + let config = ws.config(); let mut errors = Vec::new(); let doctest_xcompile = config.cli_unstable().doctest_xcompile; + let doctest_in_workspace = config.cli_unstable().doctest_in_workspace; for doctest_info in &compilation.to_doc_test { let Doctest { @@ -167,10 +169,18 @@ fn run_doc_tests( config.shell().status("Doc-tests", unit.target.name())?; let mut p = compilation.rustdoc_process(unit, *script_meta)?; - p.arg("--test") - .arg(unit.target.src_path().path().unwrap()) - .arg("--crate-name") - .arg(&unit.target.crate_name()); + p.arg("--crate-name").arg(&unit.target.crate_name()); + p.arg("--test"); + + if doctest_in_workspace { + add_path_args(ws, unit, &mut p); + // FIXME(swatinem): remove the `unstable-options` once rustdoc stabilizes the `test-run-directory` option + p.arg("-Z").arg("unstable-options"); + p.arg("--test-run-directory") + .arg(unit.pkg.root().to_path_buf()); + } else { + p.arg(unit.target.src_path().path().unwrap()); + } if doctest_xcompile { if let CompileKind::Target(target) = unit.kind { diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index f0408d2a920..42b243bf917 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -27,8 +27,8 @@ pub use self::sha256::Sha256; pub use self::to_semver::ToSemver; pub use self::vcs::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo}; pub use self::workspace::{ - print_available_benches, print_available_binaries, print_available_examples, - print_available_packages, print_available_tests, + add_path_args, path_args, print_available_benches, print_available_binaries, + print_available_examples, print_available_packages, print_available_tests, }; mod canonical_url; diff --git a/src/cargo/util/workspace.rs b/src/cargo/util/workspace.rs index d261717dc10..0cac29f677f 100644 --- a/src/cargo/util/workspace.rs +++ b/src/cargo/util/workspace.rs @@ -1,8 +1,12 @@ +use super::ProcessBuilder; +use crate::core::compiler::Unit; +use crate::core::manifest::TargetSourcePath; use crate::core::{Target, Workspace}; use crate::ops::CompileOptions; use crate::util::CargoResult; use anyhow::bail; use std::fmt::Write; +use std::path::PathBuf; fn get_available_targets<'a>( filter_fn: fn(&Target) -> bool, @@ -89,3 +93,38 @@ pub fn print_available_benches(ws: &Workspace<'_>, options: &CompileOptions) -> pub fn print_available_tests(ws: &Workspace<'_>, options: &CompileOptions) -> CargoResult<()> { print_available_targets(Target::is_test, ws, options, "--test", "tests") } + +/// The path that we pass to rustc is actually fairly important because it will +/// show up in error messages (important for readability), debug information +/// (important for caching), etc. As a result we need to be pretty careful how we +/// actually invoke rustc. +/// +/// In general users don't expect `cargo build` to cause rebuilds if you change +/// directories. That could be if you just change directories in the package or +/// if you literally move the whole package wholesale to a new directory. As a +/// result we mostly don't factor in `cwd` to this calculation. Instead we try to +/// track the workspace as much as possible and we update the current directory +/// of rustc/rustdoc where appropriate. +/// +/// The first returned value here is the argument to pass to rustc, and the +/// second is the cwd that rustc should operate in. +pub fn path_args(ws: &Workspace<'_>, unit: &Unit) -> (PathBuf, PathBuf) { + let ws_root = ws.root(); + let src = match unit.target.src_path() { + TargetSourcePath::Path(path) => path.to_path_buf(), + TargetSourcePath::Metabuild => unit.pkg.manifest().metabuild_path(ws.target_dir()), + }; + assert!(src.is_absolute()); + if unit.pkg.package_id().source_id().is_path() { + if let Ok(path) = src.strip_prefix(ws_root) { + return (path.to_path_buf(), ws_root.to_path_buf()); + } + } + (src, unit.pkg.root().to_path_buf()) +} + +pub fn add_path_args(ws: &Workspace<'_>, unit: &Unit, cmd: &mut ProcessBuilder) { + let (arg, cwd) = path_args(ws, unit); + cmd.arg(arg); + cmd.cwd(cwd); +} diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 2965ba98e26..89b38810aeb 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -2753,7 +2753,7 @@ fn doctest_receives_build_link_args() { p.cargo("test -v") .with_stderr_contains( - "[RUNNING] `rustdoc [..]--test [..] --crate-name foo [..]-L native=bar[..]`", + "[RUNNING] `rustdoc [..]--crate-name foo --test [..]-L native=bar[..]`", ) .run(); } diff --git a/tests/testsuite/cross_compile.rs b/tests/testsuite/cross_compile.rs index a45124cf3b7..42c6e2260e4 100644 --- a/tests/testsuite/cross_compile.rs +++ b/tests/testsuite/cross_compile.rs @@ -1109,7 +1109,7 @@ fn doctest_xcompile_linker() { .masquerade_as_nightly_cargo() .with_stderr_contains(&format!( "\ -[RUNNING] `rustdoc --crate-type lib --test [..]\ +[RUNNING] `rustdoc --crate-type lib --crate-name foo --test [..]\ --target {target} [..] -C linker=my-linker-tool[..] ", target = target, diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 19a720a8d5b..4878c649875 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1640,6 +1640,82 @@ fn crate_versions_flag_is_overridden() { asserts(output_documentation()); } +#[cargo_test] +fn doc_test_in_workspace() { + if !is_nightly() { + // -Zdoctest-in-workspace is unstable + return; + } + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = [ + "crate-a", + "crate-b", + ] + "#, + ) + .file( + "crate-a/Cargo.toml", + r#" + [project] + name = "crate-a" + version = "0.1.0" + "#, + ) + .file( + "crate-a/src/lib.rs", + "\ + //! ``` + //! assert_eq!(1, 1); + //! ``` + ", + ) + .file( + "crate-b/Cargo.toml", + r#" + [project] + name = "crate-b" + version = "0.1.0" + "#, + ) + .file( + "crate-b/src/lib.rs", + "\ + //! ``` + //! assert_eq!(1, 1); + //! ``` + ", + ) + .build(); + p.cargo("test -Zdoctest-in-workspace --doc -vv") + .masquerade_as_nightly_cargo() + .with_stderr_contains("[DOCTEST] crate-a") + .with_stdout_contains( + " +running 1 test +test crate-a/src/lib.rs - (line 1) ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] + +", + ) + .with_stderr_contains("[DOCTEST] crate-b") + .with_stdout_contains( + " +running 1 test +test crate-b/src/lib.rs - (line 1) ... ok + +test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..] + +", + ) + .run(); +} + #[cargo_test] fn doc_fingerprint_is_versioning_consistent() { // Random rustc verbose version diff --git a/tests/testsuite/lto.rs b/tests/testsuite/lto.rs index f3bfe8e829f..a6d0534a168 100644 --- a/tests/testsuite/lto.rs +++ b/tests/testsuite/lto.rs @@ -526,7 +526,7 @@ fn cdylib_and_rlib() { [RUNNING] [..]target/release/deps/bar-[..] [RUNNING] [..]target/release/deps/b-[..] [DOCTEST] bar -[RUNNING] `rustdoc --crate-type cdylib --crate-type rlib --test [..]-C embed-bitcode=no[..] +[RUNNING] `rustdoc --crate-type cdylib --crate-type rlib --crate-name bar --test [..]-C embed-bitcode=no[..] ", ) .run(); diff --git a/tests/testsuite/rename_deps.rs b/tests/testsuite/rename_deps.rs index fc3d11d2737..cc0b7173623 100644 --- a/tests/testsuite/rename_deps.rs +++ b/tests/testsuite/rename_deps.rs @@ -266,7 +266,7 @@ fn can_run_doc_tests() { .with_stderr_contains( "\ [DOCTEST] foo -[RUNNING] `rustdoc [..]--test [CWD]/src/lib.rs \ +[RUNNING] `rustdoc [..]--test [..]src/lib.rs \ [..] \ --extern bar=[CWD]/target/debug/deps/libbar-[..].rlib \ --extern baz=[CWD]/target/debug/deps/libbar-[..].rlib \ diff --git a/tests/testsuite/test.rs b/tests/testsuite/test.rs index 620c5d2202d..3e86428c338 100644 --- a/tests/testsuite/test.rs +++ b/tests/testsuite/test.rs @@ -4256,6 +4256,7 @@ fn test_workspaces_cwd() { r#" //! ``` //! assert_eq!("{expected}", std::fs::read_to_string("file.txt").unwrap()); + //! assert_eq!("{expected}", include_str!("../file.txt")); //! assert_eq!( //! std::path::PathBuf::from(std::env!("CARGO_MANIFEST_DIR")), //! std::env::current_dir().unwrap(), @@ -4265,6 +4266,7 @@ fn test_workspaces_cwd() { #[test] fn test_unit_{expected}_cwd() {{ assert_eq!("{expected}", std::fs::read_to_string("file.txt").unwrap()); + assert_eq!("{expected}", include_str!("../file.txt")); assert_eq!( std::path::PathBuf::from(std::env!("CARGO_MANIFEST_DIR")), std::env::current_dir().unwrap(), @@ -4280,6 +4282,7 @@ fn test_workspaces_cwd() { #[test] fn test_integration_{expected}_cwd() {{ assert_eq!("{expected}", std::fs::read_to_string("file.txt").unwrap()); + assert_eq!("{expected}", include_str!("../file.txt")); assert_eq!( std::path::PathBuf::from(std::env!("CARGO_MANIFEST_DIR")), std::env::current_dir().unwrap(),