Auto merge of #8996 - alexcrichton:revert, r=ehuss

Revert #8954 - changing rustdoc's cwd

This PR reverts #8954 in reference to #8993 and #8992 where there's still definitely a bug to be fixed but we should probably avoid regressing in the meantime.

Closes #8992

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<Finger
         profile: profile_hash,
         // Note that .0 is hashed here, not .1 which is the cwd. That doesn't
         // actually affect the output artifact so there's no need to hash it.
-        path: util::hash_u64(path_args(cx.bcx.ws, unit).0),
+        path: util::hash_u64(super::path_args(cx.bcx, unit).0),
         features: format!("{:?}", unit.features),
         deps,
         local: Mutex::new(local),

src/cargo/core/compiler/mod.rs

@@ -55,7 +55,7 @@ use crate::util::errors::{self, CargoResult, CargoResultExt, ProcessError, Verbo
 use crate::util::interning::InternedString;
 use crate::util::machine_message::Message;
 use crate::util::{self, machine_message, ProcessBuilder};
-use crate::util::{add_path_args, internal, join_paths, paths, profile};
+use crate::util::{internal, join_paths, paths, profile};
 
 const RUSTDOC_CRATE_VERSION_FLAG: &str = "--crate-version";
 
@@ -583,7 +583,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
     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 {

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<ProcessError>)> {
-    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 {

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;

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);
-}

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();
 }

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 --crate-name foo --test [..]\
+[RUNNING] `rustdoc --crate-type lib --test [..]\
     --target {target} [..] -C linker=my-linker-tool[..]
 ",
             target = target,

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();
-}

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();

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 \