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 on-demand #43482

Merged
merged 8 commits into from
Jul 27, 2017
Merged

Conversation

Mark-Simulacrum
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum commented Jul 25, 2017

Fixes #43284, fixes #38318, and fixes #39505.

Doesn't directly help with #42686, since we need to rebuild just as much. In fact, this hurts it, since ./x.py doc --stage 0 will now fail. I'm not sure if it did before, but with these changes it runs into the problem where we attempt to use artifacts from bootstrap rustc with a non-bootstrap rustdoc, running into version conflicts. I believe this is solvable, but leaving for a future PR.

This means that rustdoc will no longer be compiled when compiling rustc, by default. However, it is still built from ./x.py build (for hosts, but not targets, since we don't produce compiler toolchains for them) and will be built for doc tests and crate tests.

After this, the recommended workflow if you want a rustdoc is: ./x.py build --stage 1 src/tools/rustdoc which will give you a working rustdoc in build/triple/stage1/bin/rustdoc. Note that you can add src/libstd onto the command to compile libstd as well so that the rustdoc can easily compile crates in the wild. ./x.py doc --stage 1 src/libstd will document libstd with a freshly built rustdoc (if necessary), and will not rebuild rustc on modifications to rustdoc.

r? @alexcrichton

INTERNER.intern_path(self.build.rustc_snapshot_libdir())
} else {
self.sysroot(compiler)
})
Copy link
Member Author

Choose a reason for hiding this comment

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

It's possible we should always do the above, i.e., in cargo below, but this makes rustdoc work in --stage 0 contexts.

let mut cmd = Command::new(&rustdoc);

builder.add_rustc_lib_path(compiler, &mut cmd);
let mut cmd = builder.rustdoc_cmd(builder.compiler(0, build.build));
Copy link
Member Author

Choose a reason for hiding this comment

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

The 0 here didn't change, but is somewhat suspicious. I think we may want to request a stage from the caller, who would get it from builder.top_stage in most cases, probably.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah let's change this to threading through a stage and/or compiler to here. Otherwise I think that ./x.py doc will compile rustdoc twice, once in the last stage and once in stage0?

Copy link
Member Author

Choose a reason for hiding this comment

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

Interestingly, not without the full bootstrap option enabled. ./x.py doc requests two things: compile::Rustdoc { target_compiler: stage = 1 } and stage = 0. However, both of those call into the same ToolBuild, constructing a "stage 1" rustdoc (there's not really a way to make a stage 0 rustdoc, so we just pretend that what you want is a stage1 rustdoc). I'll still add the threading code, though.

@petrochenkov
Copy link
Contributor

This should fix #39505 as well

@Mark-Simulacrum
Copy link
Member Author

Unfortunately we don't currently check before passing --rustdoc-path to compiletest, so it doesn't fix that (yet). Do only rustdoc and run-make tests use rustdoc?

let build_compiler = if target_compiler.stage == 0 {
target_compiler
} else {
builder.compiler(target_compiler.stage - 1, target_compiler.host)
Copy link
Member

Choose a reason for hiding this comment

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

Hm I'm not sure I quite understand the minus one here, could you elaborate on why this was needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure! I'll add a comment as well once you're satisfied with my explanation. rustdoc is basically just a fancy rustc, so what we do here is mostly the same as in the compile::Assemble step. When building rustdoc for a given stageN/bin we want to do so with the compiler for stage(N-1)/bin so we subtract 1 here. Before, this was done implicitly since rustdoc was built alongside the rustc by that stage(N-1) compiler.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right yes makes sense to me!

@alexcrichton
Copy link
Member

Looks great! Just a few minor nits but otherwise r=me. Very easy to read/review with the recent refactoring!

@Mark-Simulacrum
Copy link
Member Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Jul 26, 2017

📌 Commit 7c8debf has been approved by alexcrichton

@Mark-Simulacrum
Copy link
Member Author

@bors r=alexcrichton

Missed a few cases of passing the wrong compiler, should be fixed now.

@bors
Copy link
Contributor

bors commented Jul 26, 2017

📌 Commit d32a8c3 has been approved by alexcrichton

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 26, 2017
@bors
Copy link
Contributor

bors commented Jul 27, 2017

⌛ Testing commit d32a8c3e2c8f2322c642bbd448b9c654cb68540f with merge f92049dc328cabe6d366077f9943f59f834d2124...

@bors
Copy link
Contributor

bors commented Jul 27, 2017

💔 Test failed - status-travis

Rustdoc is no longer compiled in every stage, alongside rustc, instead
it is only compiled when requested, and generally only for the last
stage.
For most tests, rustdoc isn't needed, so avoid building it.
Previously we'd build libsyntax without it when documenting and that'd
cause us to recompile it when building normally for no reason.
@Mark-Simulacrum
Copy link
Member Author

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Jul 27, 2017

📌 Commit 9ee877b has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 27, 2017

⌛ Testing commit 9ee877b with merge 5cc1baa...

bors added a commit that referenced this pull request Jul 27, 2017
Compile rustdoc on-demand

Fixes #43284, fixes #38318, and fixes #39505.

Doesn't directly help with #42686, since we need to rebuild just as much. In fact, this hurts it, since `./x.py doc --stage 0` will now fail. I'm not sure if it did before, but with these changes it runs into the problem where we attempt to use artifacts from bootstrap rustc with a non-bootstrap rustdoc, running into version conflicts. I believe this is solvable, but leaving for a future PR.

This means that rustdoc will no longer be compiled when compiling rustc, by default. However, it is still built from `./x.py build` (for hosts, but not targets, since we don't produce compiler toolchains for them) and will be built for doc tests and crate tests.

After this, the recommended workflow if you want a rustdoc is: `./x.py build --stage 1 src/tools/rustdoc` which will give you a working rustdoc in `build/triple/stage1/bin/rustdoc`. Note that you can add `src/libstd` onto the command to compile libstd as well so that the rustdoc can easily compile crates in the wild. `./x.py doc --stage 1 src/libstd` will document `libstd` with a freshly built rustdoc (if necessary), and will not rebuild rustc on modifications to rustdoc.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Jul 27, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 5cc1baa to master...

@bors bors merged commit 9ee877b into rust-lang:master Jul 27, 2017
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018
Is it really time? Have our months, no, *years* of suffering come to an end? Are we finally able to cast off the pall of Hoedown? The weight which has dragged us down for so long?

-----

So, timeline for those who need to catch up:

* Way back in December 2016, [we decided we wanted to switch out the markdown renderer](rust-lang#38400). However, this was put on hold because the build system at the time made it difficult to pull in dependencies from crates.io.
* A few months later, in March 2017, [the first PR was done, to switch out the renderers entirely](rust-lang#40338). The PR itself was fraught with CI and build system issues, but eventually landed.
* However, not all was well in the Rustdoc world. During the PR and shortly after, we noticed [some differences in the way the two parsers handled some things](rust-lang#40912), and some of these differences were major enough to break the docs for some crates.
* A couple weeks afterward, [Hoedown was put back in](rust-lang#41290), at this point just to catch tests that Pulldown was "spuriously" running. This would at least provide some warning about spurious tests, rather than just breaking spontaneously.
* However, the problems had created enough noise by this point that just a few days after that, [Hoedown was switched back to the default](rust-lang#41431) while we came up with a solution for properly warning about the differences.
* That solution came a few weeks later, [as a series of warnings when the HTML emitted by the two parsers was semantically different](rust-lang#41991). But that came at a cost, as now rustdoc needed proc-macro support (the new crate needed some custom derives farther down its dependency tree), and the build system was not equipped to handle it at the time. It was worked on for three months as the issue stumped more and more people.
  * In that time, [bootstrap was completely reworked](rust-lang#43059) to change how it ordered compilation, and [the method by which it built rustdoc would change](rust-lang#43482), as well. This allowed it to only be built after stage1, when proc-macros would be available, allowing the "rendering differences" PR to finally land.
  * The warnings were not perfect, and revealed a few [spurious](rust-lang#44368) [differences](rust-lang#45421) between how we handled the renderers.
  * Once these were handled, [we flipped the switch to turn on the "rendering difference" warnings all the time](rust-lang#45324), in October 2017. This began the "warning cycle" for this change, and landed in stable in 1.23, on 2018-01-04.
  * Once those warnings hit stable, and after a couple weeks of seeing whether we would get any more reports than what we got from sitting on nightly/beta, [we switched the renderers](rust-lang#47398), making Pulldown the default but still offering the option to use Hoedown.

And that brings us to the present. We haven't received more new issues from this in the meantime, and the "switch by default" is now on beta. Our reasoning is that, at this point, anyone who would have been affected by this has run into it already.
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
4 participants