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

rustbuild: Disable docs on cross-compiled builds #49187

Merged
merged 1 commit into from
Mar 26, 2018

Conversation

alexcrichton
Copy link
Member

This commit disables building documentation on cross-compiled compilers, for
example ARM/MIPS/PowerPC/etc. Currently I believe we're not getting much use out
of these documentation artifacts and they often take 10-15 minutes total to
build as it requires building rustdoc/rustbook and then also generating all the
documentation, especially for the reference and the book itself.

In an effort to cut down on the amount of work that we're doing on dist CI
builders in light of recent timeouts this was some relatively low hanging fruit
to cut which in theory won't have much impact on the ecosystem in the hopes that
the documentation isn't used too heavily anyway.

While initial analysis in #48827 showed only shaving 5 minutes off local builds
the same 5 minute conclusion was drawn from #48826 which ended up having nearly
a half-hour impact on the bots. In that sense I'm hoping that we can land this
and test out what happens on CI to see how it affects timing.

Note that all tier 1 platforms, Windows, Mac, and Linux, will continue to
generate documentation.

@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 Mar 19, 2018
@bors
Copy link
Contributor

bors commented Mar 19, 2018

@rust-highfive: 🔑 Insufficient privileges: Not in reviewers

@bors
Copy link
Contributor

bors commented Mar 19, 2018

@rust-highfive: 🔑 Insufficient privileges: not in try users

@alexcrichton
Copy link
Member Author

cc @rust-lang/infra, curious to hear y'all's thoughts on this change at a high level, and otherwise

r? @kennytm

@pietroalbini
Copy link
Member

Sorry, just testing one thing with this message, ignore it.

@bors
Copy link
Contributor

bors commented Mar 19, 2018

@pietroalbini: 🔑 Insufficient privileges: Not in reviewers

@pietroalbini
Copy link
Member

Sorry, another test message.

This time with two paragraphs.

@bors
Copy link
Contributor

bors commented Mar 19, 2018

@pietroalbini: 🔑 Insufficient privileges: Not in reviewers

@pietroalbini
Copy link
Member

Test message.

@kennytm
Copy link
Member

kennytm commented Mar 19, 2018

  • If the improvement is ≤5 minutes, we are going to revert this?
  • Could rustup handle the missing docs tarball properly for the tier-2 platforms — this shouldn't be an essential component?
    • Actually, could we just ship the same docs tarball to all platforms 🤔
  • cc @rust-lang/docs.

@Mark-Simulacrum
Copy link
Member

This commit disables building documentation on cross-compiled compilers, for
example ARM/MIPS/PowerPC/etc. Currently I believe we're not getting much use out
of these documentation artifacts and they often take 10-15 minutes total to
build as it requires building rustdoc/rustbook and then also generating all the
documentation, especially for the reference and the book itself.

To clarify, this is std/test docs, correct? If I as a user care about these, what do I need to do to obtain documentation for my platform? I suppose I could clone rust-lang/rust and run something like ./x.py doc --target <my target>, right?

they often take 10-15 minutes total to build as it requires building rustdoc/rustbook and then also generating all the documentation

Do we still build and distribute rustdoc for these targets? I feel like we shouldn't stop doing that.

generating all the documentation, especially for the reference and the book itself.

Do we know why the book and reference take so long? They feel much smaller than the standard library. Is there something non-standard that we're doing that could be sped up (for wins across the board, not just on these builders).


Overall I think I'm fine with this, especially if we make sure that these users can still properly obtain docs via rustup, even if those docs don't quite match their platform. It'd be great if we can figure out a way to cross-compile docs quickly and easily in the future but for now I think this is "good enough."

@@ -486,7 +488,8 @@ impl Step for Rustc {
println!("Building stage{} compiler artifacts ({} -> {})",
compiler.stage, &compiler.host, target);

let stage_out = builder.stage_out(compiler, Mode::Librustc);
let stage_out = builder.stage_out(compiler, Mode::Librustc)
Copy link
Member

Choose a reason for hiding this comment

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

hm, instead of joining on target we probably want to call cargo_out here instead. Does that seem reasonable?

@GuillaumeGomez
Copy link
Member

Is it removing doc tests or just doc build? Because I think it's quite important to keep doc tests.

@steveklabnik
Copy link
Member

steveklabnik commented Mar 20, 2018

I'd love to eventually live in a universe where we don't generate docs at all, but instead, let people just generate them locally per-platform. This would go along with "std-aware cargo"; you'd get a custom std for your platform, same with docs. I rarely use my local docs, but installing the rust-docs component is one of the longest steps in updating rust on windows, since we generate so many files.

In the meantime, I think it's fine to not ship these docs, but I'm also not a heavy user of these platforms, so I am not exactly the target audience.

@alexcrichton
Copy link
Member Author

@kennytm

If the improvement is ≤5 minutes, we are going to revert this?

I think so yeah, if it really is that cheap then it's probably fine to keep running.

Could rustup handle the missing docs tarball properly for the tier-2 platforms — this shouldn't be an essential component?

Sounds like not a bad idea!

@Mark-Simulacrum

To clarify, this is std/test docs, correct?

Correct! And yes it can always be regenerated locally if desired.

Do we still build and distribute rustdoc for these targets?

Correct yeah, we just don't build rustbook/rustdoc for the host

Do we know why the book and reference take so long?

I think it's beause at this point there's hundreds of pages to render and spawning that many process/processing all that markdown takes awhile.

(plus I think basically 0 of it is platform specific)

@GuillaumeGomez

Is it removing doc tests or just doc build?

Just the doc build on cross-compiled builders

@Mark-Simulacrum
Copy link
Member

This sounds good to me then. We should probably notify/ask users on internals or users to make sure that no one is actually depending on these existing, and also verify with @onur (?) that docs.rs won't break (I don't expect it to, though).

@QuietMisdreavus
Copy link
Member

Just the doc build on cross-compiled builders

Were the cross-compiled builders running doctests to begin with? Those still need a rustdoc build.

There's some part of me that's wary about not building/shipping docs for these targets, but if there's nothing platform-specific about the docs that would get built there (for example std::os would be unique since we only host Windows/Linux/Unix on doc.rlo) then it's probably okay.

@kennytm
Copy link
Member

kennytm commented Mar 20, 2018

@QuietMisdreavus Those are dist builders which won't run any tests, yes.

Not generating docs should allow us to skip building the stage2-host rustdoc I think? It will still build the stage2-cross-compiled rustdoc build because we need to ship rustdoc 😄

@onur
Copy link
Member

onur commented Mar 20, 2018

@Mark-Simulacrum this doesn't affect docs.rs.

@kennytm
Copy link
Member

kennytm commented Mar 22, 2018

I assume everyone is fine with landing this.

@bors r+

@bors
Copy link
Contributor

bors commented Mar 22, 2018

📌 Commit 1f4f68f has been approved by kennytm

@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 Mar 22, 2018
@alexcrichton
Copy link
Member Author

@bors: r=kennytm

@bors
Copy link
Contributor

bors commented Mar 25, 2018

📌 Commit d150d33 has been approved by kennytm

@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 Mar 25, 2018
@bors
Copy link
Contributor

bors commented Mar 25, 2018

⌛ Testing commit d150d33d2612d002345f719b096ea481d93d3343 with merge 3ee28d5d2230f42d20e6a5704772b3fccc43166d...

@bors
Copy link
Contributor

bors commented Mar 25, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 25, 2018
This commit disables building documentation on cross-compiled compilers, for
example ARM/MIPS/PowerPC/etc. Currently I believe we're not getting much use out
of these documentation artifacts and they often take 10-15 minutes total to
build as it requires building rustdoc/rustbook and then also generating all the
documentation, especially for the reference and the book itself.

In an effort to cut down on the amount of work that we're doing on dist CI
builders in light of recent timeouts this was some relatively low hanging fruit
to cut which in theory won't have much impact on the ecosystem in the hopes that
the documentation isn't used too heavily anyway.

While initial analysis in rust-lang#48827 showed only shaving 5 minutes off local builds
the same 5 minute conclusion was drawn from rust-lang#48826 which ended up having nearly
a half-hour impact on the bots. In that sense I'm hoping that we can land this
and test out what happens on CI to see how it affects timing.

Note that all tier 1 platforms, Windows, Mac, and Linux, will continue to
generate documentation.
@alexcrichton
Copy link
Member Author

@bors: r=kennytm

@bors
Copy link
Contributor

bors commented Mar 26, 2018

📌 Commit bd28641 has been approved by kennytm

@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 Mar 26, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 26, 2018
rustbuild: Disable docs on cross-compiled builds

This commit disables building documentation on cross-compiled compilers, for
example ARM/MIPS/PowerPC/etc. Currently I believe we're not getting much use out
of these documentation artifacts and they often take 10-15 minutes total to
build as it requires building rustdoc/rustbook and then also generating all the
documentation, especially for the reference and the book itself.

In an effort to cut down on the amount of work that we're doing on dist CI
builders in light of recent timeouts this was some relatively low hanging fruit
to cut which in theory won't have much impact on the ecosystem in the hopes that
the documentation isn't used too heavily anyway.

While initial analysis in rust-lang#48827 showed only shaving 5 minutes off local builds
the same 5 minute conclusion was drawn from rust-lang#48826 which ended up having nearly
a half-hour impact on the bots. In that sense I'm hoping that we can land this
and test out what happens on CI to see how it affects timing.

Note that all tier 1 platforms, Windows, Mac, and Linux, will continue to
generate documentation.
TimNN added a commit to TimNN/rust that referenced this pull request Mar 26, 2018
rustbuild: Disable docs on cross-compiled builds

This commit disables building documentation on cross-compiled compilers, for
example ARM/MIPS/PowerPC/etc. Currently I believe we're not getting much use out
of these documentation artifacts and they often take 10-15 minutes total to
build as it requires building rustdoc/rustbook and then also generating all the
documentation, especially for the reference and the book itself.

In an effort to cut down on the amount of work that we're doing on dist CI
builders in light of recent timeouts this was some relatively low hanging fruit
to cut which in theory won't have much impact on the ecosystem in the hopes that
the documentation isn't used too heavily anyway.

While initial analysis in rust-lang#48827 showed only shaving 5 minutes off local builds
the same 5 minute conclusion was drawn from rust-lang#48826 which ended up having nearly
a half-hour impact on the bots. In that sense I'm hoping that we can land this
and test out what happens on CI to see how it affects timing.

Note that all tier 1 platforms, Windows, Mac, and Linux, will continue to
generate documentation.
bors added a commit that referenced this pull request Mar 26, 2018
Rollup of 7 pull requests

- Successful merges: #48693, #48932, #49103, #49170, #49187, #49346, #49353
- Failed merges:
@bors bors merged commit bd28641 into rust-lang:master Mar 26, 2018
@bdrewery
Copy link
Contributor

This breaks rustup update as well with --force.

# rustup update
info: syncing channel updates for 'stable-x86_64-unknown-freebsd'
info: syncing channel updates for 'nightly-x86_64-unknown-freebsd'
info: latest update on 2018-03-28, rust version 1.26.0-nightly (9c9424de5 2018-03-27)
error: component 'rust-docs' for 'x86_64-unknown-freebsd' is unavailable for download
info: checking for self-updates

       stable-x86_64-unknown-freebsd unchanged - rustc 1.24.1 (d3ae9a9e0 2018-02-27)
  nightly-x86_64-unknown-freebsd update failed - rustc 1.26.0-nightly (75af15ee6 2018-03-20)
# RUST_BACKTRACE=1 rustup update --force
info: syncing channel updates for 'stable-x86_64-unknown-freebsd'
info: syncing channel updates for 'nightly-x86_64-unknown-freebsd'
info: latest update on 2018-03-28, rust version 1.26.0-nightly (9c9424de5 2018-03-27)
thread 'main' panicked at 'components available', /checkout/src/libcore/option.rs:874:4
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:68
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:57
             at /checkout/src/libstd/panicking.rs:381
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:397
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:577
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:538
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:522
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:498
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:71
   9: core::option::expect_failed
             at /checkout/src/libcore/option.rs:874
  10: rustup_dist::manifestation::Manifestation::update
  11: rustup_dist::dist::update_from_dist_
fatal runtime error: failed to initiate panic, error 5
zsh: abort (core dumped)  RUST_BACKTRACE=1 rustup update --force

@kennytm
Copy link
Member

kennytm commented Mar 28, 2018

@bdrewery Ah sorry for that. Would you mind filing an issue so it could be properly tracked?

@bdrewery
Copy link
Contributor

Moved to #49462

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.