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

Conversation

Projects
None yet
4 participants
@Mark-Simulacrum
Copy link
Member

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)
})

This comment has been minimized.

Copy link
@Mark-Simulacrum

Mark-Simulacrum Jul 25, 2017

Author Member

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));

This comment has been minimized.

Copy link
@Mark-Simulacrum

Mark-Simulacrum Jul 25, 2017

Author Member

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.

This comment has been minimized.

Copy link
@alexcrichton

alexcrichton Jul 25, 2017

Member

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?

This comment has been minimized.

Copy link
@Mark-Simulacrum

Mark-Simulacrum Jul 25, 2017

Author Member

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

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2017

This should fix #39505 as well

@Mark-Simulacrum Mark-Simulacrum force-pushed the Mark-Simulacrum:single-rustdoc branch from c3f1f89 to e13f515 Jul 25, 2017

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member Author

commented Jul 25, 2017

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)

This comment has been minimized.

Copy link
@alexcrichton

alexcrichton Jul 25, 2017

Member

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

This comment has been minimized.

Copy link
@Mark-Simulacrum

Mark-Simulacrum Jul 25, 2017

Author Member

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.

This comment has been minimized.

Copy link
@alexcrichton

alexcrichton Jul 25, 2017

Member

Ah right yes makes sense to me!

@alexcrichton

This comment has been minimized.

Copy link
Member

commented Jul 25, 2017

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

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member Author

commented Jul 26, 2017

@bors r=alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2017

📌 Commit 7c8debf has been approved by alexcrichton

@Mark-Simulacrum Mark-Simulacrum force-pushed the Mark-Simulacrum:single-rustdoc branch from 7c8debf to d32a8c3 Jul 26, 2017

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member Author

commented Jul 26, 2017

@bors r=alexcrichton

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

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 26, 2017

📌 Commit d32a8c3 has been approved by alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2017

⌛️ Testing commit d32a8c3 with merge f92049d...

bors added a commit that referenced this pull request Jul 27, 2017

Auto merge of #43482 - Mark-Simulacrum:single-rustdoc, r=alexcrichton
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

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2017

💔 Test failed - status-travis

Mark-Simulacrum added some commits Jul 23, 2017

Build rustdoc on-demand.
Rustdoc is no longer compiled in every stage, alongside rustc, instead
it is only compiled when requested, and generally only for the last
stage.
Don't needlessly build rustdoc for compiletest.
For most tests, rustdoc isn't needed, so avoid building it.
Make sure CFG_RELEASE_CHANNEL is always set.
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 Mark-Simulacrum force-pushed the Mark-Simulacrum:single-rustdoc branch from d32a8c3 to 288658b Jul 27, 2017

@Mark-Simulacrum Mark-Simulacrum force-pushed the Mark-Simulacrum:single-rustdoc branch from 288658b to 9ee877b Jul 27, 2017

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member Author

commented Jul 27, 2017

@bors r=alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2017

📌 Commit 9ee877b has been approved by alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2017

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

bors added a commit that referenced this pull request Jul 27, 2017

Auto merge of #43482 - Mark-Simulacrum:single-rustdoc, r=alexcrichton
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

This comment has been minimized.

Copy link
Contributor

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

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Remove hoedown from rustdoc
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.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Remove hoedown from rustdoc
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.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Remove hoedown from rustdoc
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.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Remove hoedown from rustdoc
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
You can’t perform that action at this time.