From 6f49ce636fe864404554b40fab70a2ad39ea2e2c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 18 Dec 2020 08:06:43 -0800 Subject: [PATCH] Revert "Run rustdoc doctests relative to the workspace" This reverts commit 2cc2ae8c963298143b71dceab0014dc09d4f65a8. --- src/cargo/core/compiler/fingerprint.rs | 4 +- src/cargo/core/compiler/mod.rs | 41 +++++++++++++-- src/cargo/ops/cargo_test.rs | 14 +++--- src/cargo/util/mod.rs | 4 +- src/cargo/util/workspace.rs | 39 -------------- tests/testsuite/build_script.rs | 2 +- tests/testsuite/doc.rs | 70 -------------------------- tests/testsuite/lto.rs | 2 +- tests/testsuite/rename_deps.rs | 2 +- 9 files changed, 52 insertions(+), 126 deletions(-) diff --git a/src/cargo/core/compiler/fingerprint.rs b/src/cargo/core/compiler/fingerprint.rs index 7e4a543fca4..a45dee17b19 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, path_args, profile, ProcessBuilder}; +use crate::util::{internal, profile, ProcessBuilder}; use super::custom_build::BuildDeps; use super::job::{Job, Work}; @@ -1313,7 +1313,7 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult, unit: &Unit) -> CargoResult { let mut rustdoc = cx.compilation.rustdoc_process(unit)?; rustdoc.inherit_jobserver(&cx.jobserver); rustdoc.arg("--crate-name").arg(&unit.target.crate_name()); - add_path_args(bcx.ws, unit, &mut rustdoc); + add_path_args(bcx, unit, &mut rustdoc); add_cap_lints(bcx, unit, &mut rustdoc); if let CompileKind::Target(target) = unit.kind { @@ -669,6 +669,41 @@ 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. @@ -763,7 +798,7 @@ fn build_base_args( cmd.arg(format!("--edition={}", edition)); } - add_path_args(bcx.ws, unit, cmd); + add_path_args(bcx, unit, cmd); add_error_format_and_color(cx, cmd, cx.rmeta_required(unit))?; if !test { diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index 953a585e5c6..de1a46585ee 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::{add_path_args, CargoTestError, Config, ProcessError, Test}; +use crate::util::{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, options, test_args, &compilation)?; + let (doctest, docerrors) = run_doc_tests(ws.config(), options, test_args, &compilation)?; let test = if docerrors.is_empty() { test } else { doctest }; errors.extend(docerrors); if errors.is_empty() { @@ -131,12 +131,11 @@ fn run_unit_tests( } fn run_doc_tests( - ws: &Workspace<'_>, + config: &Config, 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; @@ -162,9 +161,10 @@ fn run_doc_tests( config.shell().status("Doc-tests", unit.target.name())?; let mut p = compilation.rustdoc_process(unit)?; - p.arg("--crate-name").arg(&unit.target.crate_name()); - p.arg("--test"); - add_path_args(ws, unit, &mut p); + p.arg("--test") + .arg(unit.target.src_path().path().unwrap()) + .arg("--crate-name") + .arg(&unit.target.crate_name()); 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 666a4022e05..11ac2bae679 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::{ - add_path_args, path_args, print_available_benches, print_available_binaries, - print_available_examples, print_available_packages, print_available_tests, + 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 0cac29f677f..d261717dc10 100644 --- a/src/cargo/util/workspace.rs +++ b/src/cargo/util/workspace.rs @@ -1,12 +1,8 @@ -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, @@ -93,38 +89,3 @@ 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 94a3c409892..255db76e207 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 [..]--crate-name foo --test [..]-L native=bar[..]`", + "[RUNNING] `rustdoc [..]--test [..] --crate-name foo [..]-L native=bar[..]`", ) .run(); } diff --git a/tests/testsuite/doc.rs b/tests/testsuite/doc.rs index 16ec9cb4903..516e0070256 100644 --- a/tests/testsuite/doc.rs +++ b/tests/testsuite/doc.rs @@ -1638,73 +1638,3 @@ fn crate_versions_flag_is_overridden() { p.cargo("rustdoc -- --crate-version 2.0.3").run(); asserts(output_documentation()); } - -#[cargo_test] -fn doc_test_in_workspace() { - 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 --doc -vv") - .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(); -} diff --git a/tests/testsuite/lto.rs b/tests/testsuite/lto.rs index 793c0e69943..0c771329f00 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 --crate-name bar --test [..]-C embed-bitcode=no[..] +[RUNNING] `rustdoc --crate-type cdylib --crate-type rlib --test [..]-C embed-bitcode=no[..] ", ) .run(); diff --git a/tests/testsuite/rename_deps.rs b/tests/testsuite/rename_deps.rs index 7c0cd71c07f..16d15994b2e 100644 --- a/tests/testsuite/rename_deps.rs +++ b/tests/testsuite/rename_deps.rs @@ -265,7 +265,7 @@ fn can_run_doc_tests() { .with_stderr_contains( "\ [DOCTEST] foo -[RUNNING] `rustdoc [..]--test src/lib.rs \ +[RUNNING] `rustdoc [..]--test [CWD]/src/lib.rs \ [..] \ --extern bar=[CWD]/target/debug/deps/libbar-[..].rlib \ --extern baz=[CWD]/target/debug/deps/libbar-[..].rlib \