Skip to content

x.py test linkchecker builds rustc twice #76371

Closed

Description

@jyn514

Found in https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/tool.20linkchecker.20needs.20stage.202.20compiler.20.3F: Currently, x.py doc uses --stage 0, but x.py linkchecker uses --stage 2. This causes enormous recompiles for no reason; the two should match.

Activity

  1. added
    T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
    C-bugCategory: This is a bug.
    on Sep 5, 2020
  2. ehuss commented on Sep 5, 2020

    @ehuss
    Contributor

    I believe x.py test src/tools/linkchecker defaults to runs at stage 1 like all other tests. Is there some config or something causing it to run at stage 2?

    I'd be slightly concerned running it at stage 0, since some doc tools need to link with the compiler, they would be using an old compiler in that case. For quick checks, it should be OK, but like running any tests at stage 0, it is incomplete.

    Also, I believe not all docs can be built at stage 0 (for example, if you have compiler-docs enabled), which could cause confusing errors. That should probably have some check to skip or error with compiler docs at stage 0.

  3. jyn514 commented on Sep 5, 2020

    @jyn514
    MemberAuthor

    Hmm, this is a good point. @lzutao what do you think?

  4. jyn514 commented on Sep 5, 2020

    @jyn514
    MemberAuthor

    I'd be slightly concerned running it at stage 0, since some doc tools need to link with the compiler, they would be using an old compiler in that case. For quick checks, it should be OK, but like running any tests at stage 0, it is incomplete.

    In particular, they'd be missing recent fixes to intra-doc links which would fix broken links, yeah.

  5. tesuji commented on Sep 5, 2020

    @tesuji
    Contributor

    I don't actually recall the stage 2 std or stage 2 compiler was built with linkchecker,
    it actually took long enough that I don't want to run it again.

    My config.toml:

    ccache = true
    ninja = true
    targets = "AArch64;ARM;Mips;X86"
    cargo = "/home/lzutao/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo"
    rustc = "/home/lzutao/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc"
    submodules = false
    local-rebuild = true
    codegen-units = 0
    incremental = true
    llvm-config = "/home/lzutao/.local/bin/llvm-config"
  6. ehuss commented on Sep 11, 2020

    @ehuss
    Contributor

    After looking at this in more detail, I think the issue is that rustc is built twice. The second build is due to the error-index-generator, which links to rustc and rustdoc in order to reuse the markdown renderer. I'm not sure if there's a particularly good way to work around that. AFAIK, linking to rustc must use a stage1 build. The only alternative I can think of is using a different renderer (either mdbook, or something custom using pulldown-cmark?). I'm not sure that would be really worth it, but something to consider.

  7. Mark-Simulacrum commented on Sep 11, 2020

    @Mark-Simulacrum
    Member

    You can definitely link to rustc libraries in any stage - not sure what you mean by stage 1 there.

    In the long run I think mdbook is probably a good idea for the error index though.

  8. ehuss commented on Sep 11, 2020

    @ehuss
    Contributor

    Yea, I was thinking about it more, and I was mistaken. I was thinking about ABI issues (with stage1-tools built with a stage1 compiler linking against stage0-rustc built with a stage0 compiler). But I guess it should use the same trick that rustdoc does to handle this (I think error-index-generator would need to replicate most of this logic to use stage-1).

    A problem with mdbook is that it can't output to a single file. The URL https://doc.rust-lang.org/error-index.html might need to change to something like https://doc.rust-lang.org/error-index/index.html, which could be a little disruptive (at least needing a redirect). I'm also curious if it would be better to have each error on a separate page, or continue with one large page. (I'm uncertain about how slow a single page would be, considering mdbook does client-side syntax highlighting.) I'm also uncertain if anyone ever visits the error-index page at all (seems a bit silly to put effort into something nobody uses).

    @Mark-Simulacrum do either of those approaches seem preferable (or neither)?

  9. jyn514 commented on Sep 11, 2020

    @jyn514
    MemberAuthor

    I'm also uncertain if anyone ever visits the error-index page at all (seems a bit silly to put effort into something nobody uses).

    Maybe a simpler solution would be to make the error-index opt-in instead of opt-out, so that it's only generated by CI? Then you wouldn't have to mess with it to get x.py test linkchecker --stage 0 working.

  10. Mark-Simulacrum commented on Sep 11, 2020

    @Mark-Simulacrum
    Member

    I think the redirect shouldn't be too hard, I wouldn't see that as a problem. Distinct pages are a bit annoying, though I imagine that 99% of the time people are looking at just one error of the error index rather than all of them at the same time.

    I think personally I'd prefer to make the error index generator not use rustdoc/rustc as a library. Maybe an easy way of doing that is to just roll the single file into rustdoc itself behind a forever unstable -Z flag or something, rather than trying to re-engineer a bunch of mdbook stuff.

  11. changed the title [-]Default stage for `x.py test linkchecker` should match `doc`[/-] [+]`x.py test linkchecker` builds rustc twice[/+] on Sep 30, 2020
  12. ehuss commented on Jan 31, 2021

    @ehuss
    Contributor

    I posted #81603 to avoid the second build with --stage=1. I think there is still value to re-architect how error-index-generator works, but I wanted to do a smaller change for now rather than invest time in redesigning the tool.

  13. added 2 commits that reference this issue on Feb 2, 2021
  14. jyn514 commented on May 1, 2021

    @jyn514
    MemberAuthor

    @ehuss this should be fixed since #81603, right?

  15. ehuss commented on May 1, 2021

    @ehuss
    Contributor

    The original issue should be fixed, so I think this can be closed. I kept it open for the idea of rewriting error-index-generator to use a different technique. However, I personally have no plans to do that.

  16. jyn514 commented on May 1, 2021

    @jyn514
    MemberAuthor

    Ok, I'll close this issue then - the linkchecker is already somewhat tracked by #80096.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: This is a bug.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions