Skip to content

Commit

Permalink
Revert "Run rustdoc doctests relative to the workspace"
Browse files Browse the repository at this point in the history
This reverts commit 2cc2ae8.
  • Loading branch information
alexcrichton committed Dec 18, 2020
1 parent aca5261 commit 6f49ce6
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 126 deletions.
4 changes: 2 additions & 2 deletions src/cargo/core/compiler/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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),
Expand Down
41 changes: 38 additions & 3 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 7 additions & 7 deletions src/cargo/ops/cargo_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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() {
Expand Down Expand Up @@ -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;

Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
39 changes: 0 additions & 39 deletions src/cargo/util/workspace.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);
}
2 changes: 1 addition & 1 deletion tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
70 changes: 0 additions & 70 deletions tests/testsuite/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
2 changes: 1 addition & 1 deletion tests/testsuite/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/rename_deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down

0 comments on commit 6f49ce6

Please sign in to comment.