Skip to content

Commit

Permalink
Auto merge of rust-lang#9419 - ehuss:doc-meta-rebuild, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix rebuild issues with rustdoc.

This fixes two issues related to rebuilds with rustdoc:

* Switching features when running `cargo doc` would result in Cargo not rebuilding the documentation. This is because it was keeping the fingerprints in separate directories based on the features used. However, the rustdoc output isn't keyed off the metadata hash, so although the old fingerprint seemed "up to date", in reality the documentation was rewritten and needs to be rebuilt. The solution is to use a simplified hash for the fingerprint directory name.
* Removing items does not remove the files from the doc directory. This changes it to clear the package's doc directory before running rustdoc, to ensure any stale files are removed.

I'm a little concerned about potential performance impact of running `remove_dir_all`, but I think it shouldn't be too bad?

Fixes rust-lang#7370
  • Loading branch information
bors committed Apr 27, 2021
2 parents 52a0ca5 + 44c549e commit 4369396
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/context/compilation_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ fn hash_rustc_version(bcx: &BuildContext<'_, '_>, hasher: &mut StableHasher) {

/// Returns whether or not this unit should use a metadata hash.
fn should_use_metadata(bcx: &BuildContext<'_, '_>, unit: &Unit) -> bool {
if unit.mode.is_doc_test() {
if unit.mode.is_doc_test() || unit.mode.is_doc() {
// Doc tests do not have metadata.
return false;
}
Expand Down
12 changes: 10 additions & 2 deletions src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,8 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
// script_metadata is not needed here, it is only for tests.
let mut rustdoc = cx.compilation.rustdoc_process(unit, None)?;
rustdoc.inherit_jobserver(&cx.jobserver);
rustdoc.arg("--crate-name").arg(&unit.target.crate_name());
let crate_name = unit.target.crate_name();
rustdoc.arg("--crate-name").arg(&crate_name);
add_path_args(bcx.ws, unit, &mut rustdoc);
add_cap_lints(bcx, unit, &mut rustdoc);

Expand All @@ -613,7 +614,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
// it doesn't already exist.
paths::create_dir_all(&doc_dir)?;

rustdoc.arg("-o").arg(doc_dir);
rustdoc.arg("-o").arg(&doc_dir);

for feat in &unit.features {
rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat));
Expand Down Expand Up @@ -653,6 +654,13 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
}
}
}
let crate_dir = doc_dir.join(&crate_name);
if crate_dir.exists() {
// Remove output from a previous build. This ensures that stale
// files for removed items are removed.
log::debug!("removing pre-existing doc directory {:?}", crate_dir);
paths::remove_dir_all(crate_dir)?;
}
state.running(&rustdoc);

rustdoc
Expand Down
36 changes: 35 additions & 1 deletion tests/testsuite/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -863,10 +863,44 @@ fn features() {
r#"#[cfg(feature = "bar")] pub fn bar() {}"#,
)
.build();
p.cargo("doc --features foo").run();
p.cargo("doc --features foo")
.with_stderr(
"\
[COMPILING] bar v0.0.1 [..]
[DOCUMENTING] bar v0.0.1 [..]
[DOCUMENTING] foo v0.0.1 [..]
[FINISHED] [..]
",
)
.run();
assert!(p.root().join("target/doc").is_dir());
assert!(p.root().join("target/doc/foo/fn.foo.html").is_file());
assert!(p.root().join("target/doc/bar/fn.bar.html").is_file());
// Check that turning the feature off will remove the files.
p.cargo("doc")
.with_stderr(
"\
[COMPILING] bar v0.0.1 [..]
[DOCUMENTING] bar v0.0.1 [..]
[DOCUMENTING] foo v0.0.1 [..]
[FINISHED] [..]
",
)
.run();
assert!(!p.root().join("target/doc/foo/fn.foo.html").is_file());
assert!(!p.root().join("target/doc/bar/fn.bar.html").is_file());
// And switching back will rebuild and bring them back.
p.cargo("doc --features foo")
.with_stderr(
"\
[DOCUMENTING] bar v0.0.1 [..]
[DOCUMENTING] foo v0.0.1 [..]
[FINISHED] [..]
",
)
.run();
assert!(p.root().join("target/doc/foo/fn.foo.html").is_file());
assert!(p.root().join("target/doc/bar/fn.bar.html").is_file());
}

#[cargo_test]
Expand Down

0 comments on commit 4369396

Please sign in to comment.