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.

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

rustdoc: auto create output directory when "--output-format json" #93099

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

tomkris
Copy link
Contributor

@tomkris tomkris commented Jan 19, 2022

This PR allows rustdoc to automatically create output directory in case it does not exist (when run with --output-format json).

This fixes rustdoc crash:

$ rustdoc --output-format json -Z unstable-options src/main.rs
error: couldn't generate documentation: No such file or directory (os error 2)
  |
  = note: failed to create or modify "doc/main.json"

error: aborting due to previous error

With this fix behavior of rustdoc --output-format json becomes consistent with rustdoc --output-format html (which already auto-creates output directory if it's missing)

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jan 19, 2022
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jsha (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 19, 2022
Copy link
Contributor

@jsha jsha left a comment

Choose a reason for hiding this comment

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

This looks great. Thank you for the contribution!

I notice in the version of this code for the HTML docs, we keep a cache of what directories have been created. If this turns out to be a significant slowdown we might want to do the same thing here too; but I don't think that's a blocker.

crate fn ensure_dir(&self, dst: &Path) -> Result<(), Error> {
let mut dirs = self.created_dirs.borrow_mut();
if !dirs.contains(dst) {
try_err!(self.fs.create_dir_all(dst), dst);
dirs.insert(dst.to_path_buf());
}
Ok(())

Comment on lines 260 to 261
create_dir_all(&out_dir)
.map_err(|error| Error { error: error.to_string(), file: out_dir.clone() })?;
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be simplified with the try_err macro. You'll also need to import PathError:

diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs
index 6fadf72f54d..12281e78ffc 100644
--- a/src/librustdoc/json/mod.rs
+++ b/src/librustdoc/json/mod.rs
@@ -18,13 +18,14 @@
 
 use rustdoc_json_types as types;
 
-use crate::clean;
 use crate::clean::types::{ExternalCrate, ExternalLocation};
 use crate::config::RenderOptions;
+use crate::docfs::PathError;
 use crate::error::Error;
 use crate::formats::cache::Cache;
 use crate::formats::FormatRenderer;
 use crate::json::conversions::{from_item_id, IntoWithTcx};
+use crate::{clean, try_err};
 
 #[derive(Clone)]
 crate struct JsonRenderer<'tcx> {
@@ -257,8 +258,7 @@ fn after_krate(&mut self) -> Result<(), Error> {
             format_version: types::FORMAT_VERSION,
         };
         let out_dir = self.out_path.clone();
-        create_dir_all(&out_dir)
-            .map_err(|error| Error { error: error.to_string(), file: out_dir.clone() })?;
+        try_err!(create_dir_all(&out_dir), out_dir);
 
         let mut p = out_dir;
         p.push(output.index.get(&output.root).unwrap().name.clone().unwrap());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion Jacob, I included it into the PR.

I added new commit on top of original one. Please let me know if that's fine or if you want me to amend original commit instead (I did not find what's rust repo policy on commit append/amend on PRs).

@tomkris
Copy link
Contributor Author

tomkris commented Jan 20, 2022

I notice in the version of this code for the HTML docs, we keep a cache of what directories have been created. If this turns out to be a significant slowdown we might want to do the same thing here too; but I don't think that's a blocker.

I may be wrong, my understanding that rustdoc --output-format json generates single json file per run (unlike rustdoc --output-format html which can generate thousands of html files and directories), and this code will run only once.

Are there cases where rustdoc will be generating multiple json files where this optimization will be beneficial?

@jsha
Copy link
Contributor

jsha commented Jan 20, 2022

my understanding that rustdoc --output-format json generates single json file per run (unlike rustdoc --output-format html which can generate thousands of html files and directories), and this code will run only once.

Aha, that makes sense. Thanks for pointing it out. I'm new to this part of the codebase.

The policy regarding squashes and merges is here: https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy.

The short version is: multiple commits are fine; squashing into a single commit is fine; the only thing that's not fine is having merge commits in your PR. And generally if you have tiny commits that aren't meaningful on their own, you're better off squashing them onto the previous one.

This PR allows rustdoc to automatically create output directory in case
it does not exist (when run with `--output-format json`).

This fixes rustdoc crash:

````
$ rustdoc --output-format json -Z unstable-options src/main.rs
error: couldn't generate documentation: No such file or directory (os error 2)
  |
  = note: failed to create or modify "doc/main.json"

error: aborting due to previous error
````

With this fix behavior of `rustdoc --output-format json` becomes consistent
with `rustdoc --output-format html` (which already auto-creates output
directory if it's missing)
@jsha
Copy link
Contributor

jsha commented Jan 20, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 20, 2022

📌 Commit 4e17170 has been approved by jsha

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 20, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2022
…askrgr

Rollup of 17 pull requests

Successful merges:

 - rust-lang#91032 (Introduce drop range tracking to generator interior analysis)
 - rust-lang#92856 (Exclude "test" from doc_auto_cfg)
 - rust-lang#92860 (Fix errors on blanket impls by ignoring the children of generated impls)
 - rust-lang#93038 (Fix star handling in block doc comments)
 - rust-lang#93061 (Only suggest adding `!` to expressions that can be macro invocation)
 - rust-lang#93067 (rustdoc mobile: fix scroll offset when jumping to internal id)
 - rust-lang#93086 (Add tests to ensure that `let_chains` works with `if_let_guard`)
 - rust-lang#93087 (Fix src/test/run-make/raw-dylib-alt-calling-convention)
 - rust-lang#93091 (⬆ chalk to 0.76.0)
 - rust-lang#93094 (src/test/rustdoc-json: Check for `struct_field`s in `variant_tuple_struct.rs`)
 - rust-lang#93098 (Show a more informative panic message when `DefPathHash` does not exist)
 - rust-lang#93099 (rustdoc: auto create output directory when "--output-format json")
 - rust-lang#93102 (Pretty printer algorithm revamp step 3)
 - rust-lang#93104 (Support --bless for pp-exact pretty printer tests)
 - rust-lang#93114 (update comment for `ensure_monomorphic_enough`)
 - rust-lang#93128 (Add script to prevent point releases with same number as existing ones)
 - rust-lang#93136 (Backport the 1.58.1 release notes to master)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 51fd48f into rust-lang:master Jan 21, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 21, 2022
@tomkris tomkris deleted the rustdoc-fix branch January 21, 2022 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants