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

Direct contributors to try stage 0 rustdoc first #71645

Merged
merged 2 commits into from
May 4, 2020

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Apr 28, 2020

After #71458, ./x.py doc --stage 0 src/libstd is (empirically) able to build the standard library docs using the rustdoc packaged with the bootstrap compiler. This means that new contributors don't need to build the compiler to locally inspect small documentation fixes. This was a roadblock for me when I first started contributing to rust and something that still regularly annoys people. We should recommend that contributors give bootstrap rustdoc a try before building the whole compiler.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(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 Apr 28, 2020
CONTRIBUTING.md Outdated
In many cases, you don't need a full `./x.py doc`, which will build the entire
stage 2 compiler. When updating documentation for the standard library, first
try `./x.py doc --stage 0 src/libstd`. Results will should appear in
`build/$TARGET/crate-docs`.
Copy link
Member

Choose a reason for hiding this comment

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

We should definitely recommend stage 1 (which should always work) if this fails, which it could, as rustdoc doesn't run on stage 0 in CI, so some annotations may be missing or otherwise incompatible with stage 0. Lack of effective cfg(bootstrap) for docs makes that a particularly thorny problem.

@Mark-Simulacrum
Copy link
Member

Other than that nit, I also am hesitant to suggest use of something that isn't guaranteed to work in official docs, but maybe I'm overly cautious. We could also see how long this takes -- maybe we can run it in CI (on the mingw-check builder)?

@ecstatic-morse
Copy link
Contributor Author

ecstatic-morse commented Apr 28, 2020

When it works, this saves around 15-30 minutes from a clean checkout, possibly more if you're not using the system LLVM. I think it's worth trying.

Ideally, we would guarantee that this works via CI as you said. Just building stage0 artifacts and documenting them takes about a minute locally, so I don't think CI time will be an issue, although I don't know all the constraints. Would doing this make upgrading the bootstrap compiler more difficult or anything?

@Mark-Simulacrum
Copy link
Member

Yeah, I definitely agree it is worth it, I'm just concerned whether instructions that might not work are more of a discouragement than very slow instructions. I'm inclined to agree we should do as you suggest though.

Can you add a stage 0 doc build to the mingw-check builder? Should be in src/ci/docker/mingw-check/Dockerfile. Probably the last thing we want to do on that builder.

We can definitely afford a few minutes of CI time there.

@nikomatsakis
Copy link
Contributor

Presumably the problem is not the CI time, but rather that we'd have to wait for rustdoc improvements to make their way into stage0 before we can take advantage of them, right?

@nikomatsakis
Copy link
Contributor

Also, if there are incompatibilities between stage0 and stage1/2 -- maybe changes to unstable features -- that is a thorny problem.

@nikomatsakis
Copy link
Contributor

I personally think this advice is ok. It's not ideal, but I think contributors can handle it.

CONTRIBUTING.md Outdated Show resolved Hide resolved
@ecstatic-morse
Copy link
Contributor Author

@nikomatsakis In some cases you will be able to hide those bits behind #[cfg(not(bootstrap))]. This is kind of a pain, especially if the offending part is in a /// comment.

I'll submit a second PR that adds ./x.py doc --stage 0 to the mingw-check build. Perhaps some rustdoc folks could weigh in on whether that's a deal-breaker or not.

@ecstatic-morse
Copy link
Contributor Author

@Mark-Simulacrum Anything I need to do here?

We're now ~2 weeks after the bootstrap version bump and ./x.py doc --stage 0 still works.

@Mark-Simulacrum
Copy link
Member

I was "waiting" (really had forgotten about this PR, sorry) on #71649 to land before we do this, but thinking a bit more I guess we don't actually need to do that since this mostly just works.

@bors r+

@bors
Copy link
Contributor

bors commented May 3, 2020

📌 Commit 5577b35 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-review Status: Awaiting review from the assignee but also interested parties. labels May 3, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2020
Rollup of 4 pull requests

Successful merges:

 - rust-lang#71645 (Direct contributors to try stage 0 rustdoc first)
 - rust-lang#71801 (Correctly check comparison operator in MIR typeck)
 - rust-lang#71844 (List Clippy as a subtree, instead of a submodule)
 - rust-lang#71864 (Update link in contributing.md)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented May 4, 2020

⌛ Testing commit 5577b35 with merge ff4df04...

@bors bors merged commit ccc123a into rust-lang:master May 4, 2020
@ecstatic-morse ecstatic-morse deleted the readme-build-doc branch May 4, 2020 16:19
bors added a commit to rust-lang-ci/rust that referenced this pull request May 7, 2020
…Simulacrum

Ensure that `./x.py doc --stage 0 src/libstd` works via CI

This was split off from rust-lang#71645, which recommends that users first try building `libstd` docs with the bootstrap `rustdoc`. This should work in most cases, but will fail if we start using a very recent `rustdoc` feature outside a `#[cfg(not(bootstrap))]`.

It would be very nice to guarantee that `./x.py doc --stage 0 src/libstd` works, since it allows documentation changes to be rendered locally without needing to build the compiler. However, it may put too big a burden on `rustdoc` developers who presumably want to dogfood new features.
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.

6 participants