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

Compile rustdoc less often. #73883

Merged
merged 2 commits into from
Jul 2, 2020
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Jun 30, 2020

Previously rustdoc was built 3 times with x.py test:

  1. stage2 (using stage1 compiler) for compiletest tests (stage1-tools copied to stage2).
  2. stage1 (using stage0 compiler) for std crate tests (stage0-tools copied to stage1).
  3. stage2 test (using stage2 compiler) for rustdoc crate tests and error_index_generator (stage2-tools).

This PR removes the majority of number 3, where it will instead use the stage1 compiler, which will share the artifacts from number 1.

This matches the behavior of the libstd crate tests. I don't think it is entirely necessary to run the tests using stage2.

At -j2, the last build step goes from about 300s to 70s on my machine. It's not a huge win, but shaving 4 minutes isn't bad.

The other two builds would be pretty difficult (or undesired or impossible) to unify. It looks like std tests use stage1 very intentionally (see force_use_stage1 and its history), and compiletests use the top stage very intentionally.

Unfortunately the linkchecker builds all docs at stage2 (stage2-tools), which means a few build script artifacts are not shared. It's not really clear to me how to fix that (because it uses default_doc, there doesn't seem to be any control over the stages).


For x.py doc, rustdoc was previously built three times (with compiler-docs):

  1. stage2 (using stage1 compiler) for normal documentation output (stage1-tools copied to stage2).
  2. stage1 (using stage0 compiler) for compiler-docs
  3. stage2 (using stage2 compiler) for error_index_generator (stage2-tools)

This PR combines these so that they consistently use the "top stage" rustdoc. I don't know why the compiler-docs was written to use stage minus one, but it seems better to be consistent across the doc steps.


I've tried to test this with a variety of commands (x.py doc, x.py test, different --stage flags, full-bootstrap, setting --target, etc.) to try to make sure there aren't significant regressions here. It's tricky since there are so many variables, and this stuff is difficult for me to fully understand.

Closes #70799 (I think)

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 30, 2020
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

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

Can you check that we still avoid building rustdoc in stage0? (i.e. x.py test --stage 0 src/libstd)

Other than that though this looks reasonable, though I agree it's pretty annoying to try and get this right. Maybe we can eventually come up with some kind of solution to that.

// 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.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 30, 2020
@ehuss
Copy link
Contributor Author

ehuss commented Jun 30, 2020

Can you check that we still avoid building rustdoc in stage0? (i.e. x.py test --stage 0 src/libstd)

I can confirm that it uses the beta rustc/rustdoc.

As for testing, should I add a few, similar to the builder tests? I'm not sure if it will really capture everything (like the implicit rustdoc dependencies in error_index_generator), but I'd be happy to add them.

@Mark-Simulacrum
Copy link
Member

Yeah, that'd be great! You should be able to pretty much add to the existing tests fairly easily, and it should capture ~everything I believe in terms of dependencies.

@ehuss
Copy link
Contributor Author

ehuss commented Jul 1, 2020

OK, I added a few tests. I feel like there could be a lot more, but this seemed like a good start.

@Mark-Simulacrum
Copy link
Member

Looks like a great start, yes. Unfortunate about the -1 behavior on rustdoc::run but not sure we can do much... well, for a future PR anyway. Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Jul 2, 2020

📌 Commit 0b9bc79 has been approved by Mark-Simulacrum

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 2, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 2, 2020
…arth

Rollup of 10 pull requests

Successful merges:

 - rust-lang#73414 (Implement `slice_strip` feature)
 - rust-lang#73564 (linker: Create GNU_EH_FRAME header by default when producing ELFs)
 - rust-lang#73622 (Deny unsafe ops in unsafe fns in libcore)
 - rust-lang#73684 (add spans to injected coverage counters, extract with CoverageData query)
 - rust-lang#73812 (ast_pretty: Pass some token streams and trees by reference)
 - rust-lang#73853 (Add newline to rustc MultiSpan docs)
 - rust-lang#73883 (Compile rustdoc less often.)
 - rust-lang#73885 (Fix wasm32 being broken due to a NodeJS version bump)
 - rust-lang#73903 (Changes required for rustc/cargo to build for iOS targets)
 - rust-lang#73938 (Optimise fast path of checked_ops with `unlikely`)

Failed merges:

r? @ghost
@bors bors merged commit 7b2f44a into rust-lang:master Jul 2, 2020
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x.py: running "doc" after "build" always rebuilds rustdoc
5 participants