From 0a275abec64398ece3ebba1a5db3efa24f49728b Mon Sep 17 00:00:00 2001 From: ozkanonur Date: Sun, 13 Nov 2022 21:30:36 +0300 Subject: [PATCH 1/2] copy doc output files by format r=ozkanonur Signed-off-by: ozkanonur --- src/bootstrap/doc.rs | 59 +++++++++++++++---- .../rustdoc-verify-output-files/Makefile | 36 +++++++++++ .../rustdoc-verify-output-files/src/lib.rs | 1 + 3 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 src/test/run-make/rustdoc-verify-output-files/Makefile create mode 100644 src/test/run-make/rustdoc-verify-output-files/src/lib.rs diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index c7d21bf3cdb..fd6f3926817 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -551,6 +551,49 @@ fn doc_std( extra_args: &[&OsStr], requested_crates: &[String], ) { + // `cargo` uses the same directory for both JSON docs and HTML docs. + // This could lead to cross-contamination when copying files into the specified `out` directory. + // For example: + // ```bash + // x doc std + // x doc std --json + // ``` + // could lead to HTML docs being copied into the JSON docs output directory. + // To avoid this issue, we copy generated docs instead of whole directory by + // checking doc format and generated files. + fn cp_docs_by_doc_format( + format: &DocumentationFormat, + builder: &Builder<'_>, + src: &Path, + dst: &Path, + ) { + for f in builder.read_dir(src) { + let path = f.path(); + let name = path.file_name().unwrap(); + let dst = dst.join(name); + + if t!(f.file_type()).is_dir() && format == &DocumentationFormat::HTML { + t!(fs::create_dir_all(&dst)); + cp_docs_by_doc_format(format, builder, &path, &dst); + } else { + let _ = fs::remove_file(&dst); + let extension = path.extension().and_then(OsStr::to_str); + + match format { + DocumentationFormat::HTML if extension != Some("json") => { + builder.copy(&path, &dst) + } + DocumentationFormat::JSON + if extension == Some("json") || name.to_str() == Some(".stamp") => + { + builder.copy(&path, &dst) + } + _ => {} + } + } + } + } + builder.info(&format!( "Documenting stage{} std ({}) in {} format", stage, @@ -568,18 +611,6 @@ fn doc_std( // We will then copy the files from this directory into the final `out` directory, the specified // as a function parameter. let out_dir = builder.stage_out(compiler, Mode::Std).join(target.triple).join("doc"); - // `cargo` uses the same directory for both JSON docs and HTML docs. - // This could lead to cross-contamination when copying files into the specified `out` directory. - // For example: - // ```bash - // x doc std - // x doc std --json - // ``` - // could lead to HTML docs being copied into the JSON docs output directory. - // To avoid this issue, we clean the doc folder before invoking `cargo`. - if out_dir.exists() { - builder.remove_dir(&out_dir); - } let run_cargo_rustdoc_for = |package: &str| { let mut cargo = builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "rustdoc"); @@ -605,7 +636,9 @@ fn doc_std( } } - builder.cp_r(&out_dir, &out); + if !builder.config.dry_run() { + cp_docs_by_doc_format(&format, builder, &out_dir, &out); + } } #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] diff --git a/src/test/run-make/rustdoc-verify-output-files/Makefile b/src/test/run-make/rustdoc-verify-output-files/Makefile new file mode 100644 index 00000000000..bfabbbc6586 --- /dev/null +++ b/src/test/run-make/rustdoc-verify-output-files/Makefile @@ -0,0 +1,36 @@ +include ../../run-make-fulldeps/tools.mk + +OUTPUT_DIR := "$(TMPDIR)/rustdoc" +TMP_OUTPUT_DIR := "$(TMPDIR)/tmp-rustdoc" + +all: + # Generate html docs + $(RUSTDOC) src/lib.rs --crate-name foobar --crate-type lib --out-dir $(OUTPUT_DIR) + + # Copy first output for to check if it's exactly same after second compilation + cp -R $(OUTPUT_DIR) $(TMP_OUTPUT_DIR) + + # Generate html docs once again on same output + $(RUSTDOC) src/lib.rs --crate-name foobar --crate-type lib --out-dir $(OUTPUT_DIR) + + # Check if everything exactly same + $(DIFF) -r -q $(OUTPUT_DIR) $(TMP_OUTPUT_DIR) + + # Generate json doc on the same output + $(RUSTDOC) src/lib.rs --crate-name foobar --crate-type lib --out-dir $(OUTPUT_DIR) -Z unstable-options --output-format json + + # Check if expected json file is generated + [ -e $(OUTPUT_DIR)/foobar.json ] + + # TODO + # We should re-generate json doc once again and compare the diff with previously + # generated one. Because layout of json docs changes in each compilation, we can't + # do that currently. + # + # See https://github.com/rust-lang/rust/issues/103785#issuecomment-1307425590 for details. + + # remove generated json doc + rm $(OUTPUT_DIR)/foobar.json + + # Check if json doc compilation broke any of the html files generated previously + $(DIFF) -r -q $(OUTPUT_DIR) $(TMP_OUTPUT_DIR) diff --git a/src/test/run-make/rustdoc-verify-output-files/src/lib.rs b/src/test/run-make/rustdoc-verify-output-files/src/lib.rs new file mode 100644 index 00000000000..5df7576133a --- /dev/null +++ b/src/test/run-make/rustdoc-verify-output-files/src/lib.rs @@ -0,0 +1 @@ +// nothing to see here From 7e28df9561cbfb98c0b5a7f4868823709c1914c1 Mon Sep 17 00:00:00 2001 From: ozkanonur Date: Sun, 20 Nov 2022 15:51:50 +0300 Subject: [PATCH 2/2] refactor doc copying process Signed-off-by: ozkanonur --- src/bootstrap/builder.rs | 8 +++++- src/bootstrap/doc.rs | 56 +++++++--------------------------------- 2 files changed, 16 insertions(+), 48 deletions(-) diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 31158870f39..7ee80eceb95 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -1345,7 +1345,13 @@ pub fn cargo( let my_out = match mode { // This is the intended out directory for compiler documentation. Mode::Rustc | Mode::ToolRustc => self.compiler_doc_out(target), - Mode::Std => out_dir.join(target.triple).join("doc"), + Mode::Std => { + if self.config.cmd.json() { + out_dir.join(target.triple).join("json-doc") + } else { + out_dir.join(target.triple).join("doc") + } + } _ => panic!("doc mode {:?} not expected", mode), }; let rustdoc = self.rustdoc(compiler); diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index fd6f3926817..e267d179797 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -551,49 +551,6 @@ fn doc_std( extra_args: &[&OsStr], requested_crates: &[String], ) { - // `cargo` uses the same directory for both JSON docs and HTML docs. - // This could lead to cross-contamination when copying files into the specified `out` directory. - // For example: - // ```bash - // x doc std - // x doc std --json - // ``` - // could lead to HTML docs being copied into the JSON docs output directory. - // To avoid this issue, we copy generated docs instead of whole directory by - // checking doc format and generated files. - fn cp_docs_by_doc_format( - format: &DocumentationFormat, - builder: &Builder<'_>, - src: &Path, - dst: &Path, - ) { - for f in builder.read_dir(src) { - let path = f.path(); - let name = path.file_name().unwrap(); - let dst = dst.join(name); - - if t!(f.file_type()).is_dir() && format == &DocumentationFormat::HTML { - t!(fs::create_dir_all(&dst)); - cp_docs_by_doc_format(format, builder, &path, &dst); - } else { - let _ = fs::remove_file(&dst); - let extension = path.extension().and_then(OsStr::to_str); - - match format { - DocumentationFormat::HTML if extension != Some("json") => { - builder.copy(&path, &dst) - } - DocumentationFormat::JSON - if extension == Some("json") || name.to_str() == Some(".stamp") => - { - builder.copy(&path, &dst) - } - _ => {} - } - } - } - } - builder.info(&format!( "Documenting stage{} std ({}) in {} format", stage, @@ -607,15 +564,22 @@ fn cp_docs_by_doc_format( ); } let compiler = builder.compiler(stage, builder.config.build); + + let target_doc_dir_name = if format == DocumentationFormat::JSON { "json-doc" } else { "doc" }; + let target_dir = + builder.stage_out(compiler, Mode::Std).join(target.triple).join(target_doc_dir_name); + // This is directory where the compiler will place the output of the command. // We will then copy the files from this directory into the final `out` directory, the specified // as a function parameter. - let out_dir = builder.stage_out(compiler, Mode::Std).join(target.triple).join("doc"); + let out_dir = target_dir.join(target.triple).join("doc"); let run_cargo_rustdoc_for = |package: &str| { let mut cargo = builder.cargo(compiler, Mode::Std, SourceType::InTree, target, "rustdoc"); compile::std_cargo(builder, target, compiler.stage, &mut cargo); cargo + .arg("--target-dir") + .arg(&*target_dir.to_string_lossy()) .arg("-p") .arg(package) .arg("-Zskip-rustdoc-fingerprint") @@ -636,9 +600,7 @@ fn cp_docs_by_doc_format( } } - if !builder.config.dry_run() { - cp_docs_by_doc_format(&format, builder, &out_dir, &out); - } + builder.cp_r(&out_dir, &out); } #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]