Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile rustdoc less often. #73883

Merged
merged 2 commits into from
Jul 2, 2020
Merged

Compile rustdoc less often. #73883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9154863
Compile rustdoc less often.
ehuss Jun 28, 2020
0b9bc79
Tests for number of times rustdoc is built with x.py test and doc.
ehuss Jul 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions src/bootstrap/builder/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ fn dist_baseline() {
&[dist::Std { compiler: Compiler { host: a, stage: 1 }, target: a },]
);
assert_eq!(first(builder.cache.all::<dist::Src>()), &[dist::Src]);
// Make sure rustdoc is only built once.
assert_eq!(
first(builder.cache.all::<tool::Rustdoc>()),
&[tool::Rustdoc { compiler: Compiler { host: a, stage: 2 } },]
);
}

#[test]
Expand Down Expand Up @@ -414,3 +419,77 @@ fn test_exclude() {
// Ensure other tests are not affected.
assert!(builder.cache.contains::<test::RustdocUi>());
}

#[test]
fn doc_default() {
let mut config = configure(&[], &[]);
config.compiler_docs = true;
config.cmd = Subcommand::Doc { paths: Vec::new(), open: false };
let build = Build::new(config);
let mut builder = Builder::new(&build);
builder.run_step_descriptions(&Builder::get_step_descriptions(Kind::Doc), &[]);
let a = INTERNER.intern_str("A");

// error_index_generator uses stage 1 to share rustdoc artifacts with the
// rustdoc tool.
assert_eq!(
first(builder.cache.all::<doc::ErrorIndex>()),
&[doc::ErrorIndex { compiler: Compiler { host: a, stage: 1 }, target: a },]
);
assert_eq!(
first(builder.cache.all::<tool::ErrorIndex>()),
&[tool::ErrorIndex { compiler: Compiler { host: a, stage: 1 } }]
);
// This is actually stage 1, but Rustdoc::run swaps out the compiler with
// stage minus 1 if --stage is not 0. Very confusing!
assert_eq!(
first(builder.cache.all::<tool::Rustdoc>()),
&[tool::Rustdoc { compiler: Compiler { host: a, stage: 2 } },]
);
}

#[test]
fn test_docs() {
// Behavior of `x.py test` doing various documentation tests.
let mut config = configure(&[], &[]);
config.cmd = Subcommand::Test {
paths: vec![],
test_args: vec![],
rustc_args: vec![],
fail_fast: true,
doc_tests: DocTests::Yes,
bless: false,
compare_mode: None,
rustfix_coverage: false,
pass: None,
};
let build = Build::new(config);
let mut builder = Builder::new(&build);
builder.run_step_descriptions(&Builder::get_step_descriptions(Kind::Test), &[]);
let a = INTERNER.intern_str("A");

// error_index_generator uses stage 1 to share rustdoc artifacts with the
// rustdoc tool.
assert_eq!(
first(builder.cache.all::<doc::ErrorIndex>()),
&[doc::ErrorIndex { compiler: Compiler { host: a, stage: 1 }, target: a },]
);
assert_eq!(
first(builder.cache.all::<tool::ErrorIndex>()),
&[tool::ErrorIndex { compiler: Compiler { host: a, stage: 1 } }]
);
// Unfortunately rustdoc is built twice. Once from stage1 for compiletest
// (and other things), and once from stage0 for std crates. Ideally it
// would only be built once. If someone wants to fix this, it might be
// worth investigating if it would be possible to test std from stage1.
// Note that the stages here are +1 than what they actually are because
// Rustdoc::run swaps out the compiler with stage minus 1 if --stage is
// not 0.
assert_eq!(
first(builder.cache.all::<tool::Rustdoc>()),
&[
tool::Rustdoc { compiler: Compiler { host: a, stage: 1 } },
tool::Rustdoc { compiler: Compiler { host: a, stage: 2 } },
]
);
}
31 changes: 15 additions & 16 deletions src/bootstrap/doc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,7 @@ impl Step for Rustc {
let out = builder.compiler_doc_out(target);
t!(fs::create_dir_all(&out));

// Get the correct compiler for this stage.
let compiler = builder.compiler_for(stage, builder.config.build, target);
let compiler = builder.compiler(stage, builder.config.build);

if !builder.config.compiler_docs {
builder.info("\tskipping - compiler/librustdoc docs disabled");
Expand Down Expand Up @@ -599,8 +598,7 @@ impl Step for Rustdoc {
let out = builder.compiler_doc_out(target);
t!(fs::create_dir_all(&out));

// Get the correct compiler for this stage.
let compiler = builder.compiler_for(stage, builder.config.build, target);
let compiler = builder.compiler(stage, builder.config.build);

if !builder.config.compiler_docs {
builder.info("\tskipping - compiler/librustdoc docs disabled");
Expand Down Expand Up @@ -639,9 +637,10 @@ impl Step for Rustdoc {
}
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
#[derive(Ord, PartialOrd, Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub struct ErrorIndex {
target: Interned<String>,
pub compiler: Compiler,
pub target: Interned<String>,
}

impl Step for ErrorIndex {
Expand All @@ -655,26 +654,26 @@ impl Step for ErrorIndex {
}

fn make_run(run: RunConfig<'_>) {
run.builder.ensure(ErrorIndex { target: run.target });
let target = run.target;
// error_index_generator depends on librustdoc. Use the compiler that
// is normally used to build rustdoc for other documentation so that
// it shares the same artifacts.
let compiler =
run.builder.compiler_for(run.builder.top_stage, run.builder.config.build, target);
run.builder.ensure(ErrorIndex { compiler, target });
}

/// Generates the HTML rendered error-index by running the
/// `error_index_generator` tool.
fn run(self, builder: &Builder<'_>) {
let target = self.target;

builder.info(&format!("Documenting error index ({})", target));
let out = builder.doc_out(target);
builder.info(&format!("Documenting error index ({})", self.target));
let out = builder.doc_out(self.target);
t!(fs::create_dir_all(&out));
let compiler = builder.compiler(2, builder.config.build);
let mut index = tool::ErrorIndex::command(builder, compiler);
let mut index = tool::ErrorIndex::command(builder, self.compiler);
index.arg("html");
index.arg(out.join("error-index.html"));
index.arg(crate::channel::CFG_RELEASE_NUM);

// FIXME: shouldn't have to pass this env var
index.env("CFG_BUILD", &builder.config.build);

builder.run(&mut index);
}
}
Expand Down
60 changes: 47 additions & 13 deletions src/bootstrap/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1454,8 +1454,11 @@ impl Step for ErrorIndex {
}

fn make_run(run: RunConfig<'_>) {
run.builder
.ensure(ErrorIndex { compiler: run.builder.compiler(run.builder.top_stage, run.host) });
// error_index_generator depends on librustdoc. Use the compiler that
// is normally used to build rustdoc for other tests (like compiletest
// tests in src/test/rustdoc) so that it shares the same artifacts.
let compiler = run.builder.compiler_for(run.builder.top_stage, run.host, run.host);
run.builder.ensure(ErrorIndex { compiler });
}

/// Runs the error index generator tool to execute the tests located in the error
Expand All @@ -1467,22 +1470,23 @@ impl Step for ErrorIndex {
fn run(self, builder: &Builder<'_>) {
let compiler = self.compiler;

builder.ensure(compile::Std { compiler, target: compiler.host });

let dir = testdir(builder, compiler.host);
t!(fs::create_dir_all(&dir));
let output = dir.join("error-index.md");

let mut tool = tool::ErrorIndex::command(
builder,
builder.compiler(compiler.stage, builder.config.build),
);
tool.arg("markdown").arg(&output).env("CFG_BUILD", &builder.config.build);
let mut tool = tool::ErrorIndex::command(builder, compiler);
tool.arg("markdown").arg(&output);

builder.info(&format!("Testing error-index stage{}", compiler.stage));
// Use the rustdoc that was built by self.compiler. This copy of
// rustdoc is shared with other tests (like compiletest tests in
// src/test/rustdoc). This helps avoid building rustdoc multiple
// times.
let rustdoc_compiler = builder.compiler(builder.top_stage, builder.config.build);
builder.info(&format!("Testing error-index stage{}", rustdoc_compiler.stage));
let _time = util::timeit(&builder);
builder.run_quiet(&mut tool);
markdown_test(builder, compiler, &output);
builder.ensure(compile::Std { compiler: rustdoc_compiler, target: rustdoc_compiler.host });
markdown_test(builder, rustdoc_compiler, &output);
}
}

Expand Down Expand Up @@ -1797,9 +1801,13 @@ impl Step for CrateRustdoc {

fn run(self, builder: &Builder<'_>) {
let test_kind = self.test_kind;
let target = self.host;

let compiler = builder.compiler(builder.top_stage, self.host);
let target = compiler.host;
// Use the previous stage compiler to reuse the artifacts that are
// created when running compiletest for src/test/rustdoc. If this used
// `compiler`, then it would cause rustdoc to be built *again*, which
// isn't really necessary.
let compiler = builder.compiler_for(builder.top_stage, target, target);
builder.ensure(compile::Rustc { compiler, target });

let mut cargo = tool::prepare_tool_cargo(
Expand All @@ -1825,6 +1833,32 @@ impl Step for CrateRustdoc {
cargo.arg("'-Ctarget-feature=-crt-static'");
}

// This is needed for running doctests on librustdoc. This is a bit of
// an unfortunate interaction with how bootstrap works and how cargo
// sets up the dylib path, and the fact that the doctest (in
// html/markdown.rs) links to rustc-private libs. For stage1, the
// compiler host dylibs (in stage1/lib) are not the same as the target
// dylibs (in stage1/lib/rustlib/...). This is different from a normal
// rust distribution where they are the same.
//
// On the cargo side, normal tests use `target_process` which handles
// setting up the dylib for a *target* (stage1/lib/rustlib/... in this
// case). However, for doctests it uses `rustdoc_process` which only
// sets up the dylib path for the *host* (stage1/lib), which is the
// wrong directory.
//
// It should be considered to just stop running doctests on
// librustdoc. There is only one test, and it doesn't look too
// important. There might be other ways to avoid this, but it seems
// pretty convoluted.
//
// See also https://github.com/rust-lang/rust/issues/13983 where the
// host vs target dylibs for rustdoc are consistently tricky to deal
// with.
let mut dylib_path = dylib_path();
dylib_path.insert(0, PathBuf::from(&*builder.sysroot_libdir(compiler, target)));
cargo.env(dylib_path_var(), env::join_paths(&dylib_path).unwrap());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be on board with deleting that test - perhaps in a separate PR - and adding a tidy lint or something banning doc tests in librustdoc.


if !builder.config.verbose_tests {
cargo.arg("--quiet");
}
Expand Down
10 changes: 5 additions & 5 deletions src/bootstrap/tool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ bootstrap_tool!(
ExpandYamlAnchors, "src/tools/expand-yaml-anchors", "expand-yaml-anchors";
);

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
pub struct ErrorIndex {
pub compiler: Compiler,
}
Expand All @@ -392,9 +392,9 @@ impl Step for ErrorIndex {
fn make_run(run: RunConfig<'_>) {
// Compile the error-index in the same stage as rustdoc to avoid
// recompiling rustdoc twice if we can.
let stage = if run.builder.top_stage >= 2 { run.builder.top_stage } else { 0 };
run.builder
.ensure(ErrorIndex { compiler: run.builder.compiler(stage, run.builder.config.build) });
let host = run.builder.config.build;
let compiler = run.builder.compiler_for(run.builder.top_stage, host, host);
run.builder.ensure(ErrorIndex { compiler });
}

fn run(self, builder: &Builder<'_>) -> PathBuf {
Expand Down Expand Up @@ -449,7 +449,7 @@ impl Step for RemoteTestServer {
}
}

#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)]
pub struct Rustdoc {
/// This should only ever be 0 or 2.
/// We sometimes want to reference the "bootstrap" rustdoc, which is why this option is here.
Expand Down