Tracking Issue: Add rustc-guide to toolstate

[mark-i-m]

(According to the plan in rust-lang-nursery/rust-toolstate#5)

cc @nikomatsakis @kennytm @Michael-F-Bryan

@kennytm Could you tag this with Help-Wanted?

Instructions

Background: We have a cool tool called toolstate which posts a message on PRs that break dependent crates (e.g. clippy). For example, it might generate a post like this. There is also a web portal: https://rust-lang-nursery.github.io/rust-toolstate/

Objective: Currently the rustc-guide is not tracked by toolstate, but we want it to be. This will help us detect when the compiler changes and the guide needs to be updated. However, some infrastructure work is needed for this to work.

Steps

• Add mdbook and mdbook-linkcheck as dependencies for the rust-lang/rust build. These are used to build and check links in the rustc-guide repo.
  • TBH, I don't know how best to do this. We already use mdbook to build The Book and other things, but it's not clear to me how we make sure they are installed... (Perhaps @kennytm has ideas?)
• Add an ./x.py test src/doc/rustc-guide command
  1. Create a new struct RustcGuide in src/bootstrap/test.rs.
  2. impl Step for RustcGuide to run mdbook build in src/doc/rustc-guide
  - Make sure that this test is not part of the standard build process or it will block PRs on the rust-lang/rust repo. IIUC, this can be done in the implementation of should_run and make_run in Step (see the documentation in src/bootstrap/builder.rs
  3. Update Builder::get_step_descriptions
  4. You may or may not need to do other stuff to wire things together... not sure. In the end, you should be able to do ./x.py test src/doc/rustc-guide to run the book build and linkchecker on the guide.
• Update the toolstate config to add rustc-guide, as done here: https://github.com/rust-lang/rust/pull/59038/files
  • Add me (@mark-i-m) under the MAINTAINERS section. (@nikomatsakis do you want to be added too?)

Feel free to ping me on this thread if you need help :)

[andrehjr]

Hi there, is anyone working on this already? @mark-i-m

Anyway, I played with this for a bit, ./x.py test src/doc/rustc-guide seems to be running the doc tests with success. https://github.com/rust-lang/rust/compare/master...andrehjr:add-rustc-guide-to-toolstate?expand=1

I'm just not sure about how to add mdbook-linkcheck as dep. And if the changes above are enough. If they are, I'm happy to open the PR! My first to rust <3

[mark-i-m]

Hi @andrehjr! You are the first person to work on this :) Thanks!

That looks like a great start, but it causes the entire compiler to be built before rustc-guide is tested, which is not necessary. Also, I’m not sure if actually tests rustc-guide or just libstd.

You will probably need to manually define RustcGuide and impl Step for it. I did a bit more digging, and I think such an implementation can look similar to the one for test::ErrorIndex, which also requires calling an external tool. Except in our case, we want to call a tool called rustbook, which calls mdbook (src/tools/rustbook/).

It looks like rustbook calls mdbook programmatically, so we might need to find a way to call mdbook-linkcheck programmatically too. My guess is that we will also need to add linkcheck as a dependency of rustbook somehow... If I have time tomorrow I can try to learn more, or if you are feeling adventurous feel free to explore :)

[mark-i-m]

A quick note: it looks like you can use the linkchecker as a library: https://docs.rs/mdbook-linkcheck/0.2.3/mdbook_linkcheck/fn.check_links.html

My guess is that you can get the RenderContext from the mdbook crate, which is already imported in rustbook. So it looks like it should be possible to add a “linkcheck mode” to rustbook.

Feel free to ping me if you need anything :)

[andrehjr]

Thanks for the tips @mark-i-m !

I thought I could get away using the test_book! macro like the other books.

Interesting! I'll dig more into this today.

[andrehjr]

Hi @mark-i-m I played with it a bit more, I think I'm on the right track. Just need some refactoring :) And ensure everything is working as expected.

I was able to call the check_links programmatically passing a RenderContext from the md_book. Also note that mdbook-linkcheck only supports mdbook > 2.

I ended up adding a new subcommand on mdbook just for linkchecks, I can tweak it into the other command if needed.

Locally, I'm getting a few intermittent 'request timeouts' for a few links every now and then. Maybe it's just my internet connection.

I did not push Cargo.lock because it added a bunch of stuff from macOS, what can I do in those cases?

Also, it seems like when you 'build' the book it runs the linkcheck automatically if it's configured on book.toml.

The way I did here, it just loads up the book using mdbook and linkchecks it (without fully building it/generating .htmls)

[mark-i-m]

@andrehjr Just took a look, and it looks good 👍

Also note that mdbook-linkcheck only supports mdbook > 2

Sorry, I forgot to mention this. Could you add in the command line description for the rustbook linkcheck subcommand you created (i.e. Run linkcheck with mdbook 2)?

Locally, I'm getting a few intermittent 'request timeouts' for a few links every now and then.

There may also be some rate-limiting somewhere because I see failures every once in a while both on my laptop and on CI. I think it is probably ok for now. If it gets to be an issue later, we can do some deeper debugging.

I did not push Cargo.lock because it added a bunch of stuff from macOS, what can I do in those cases?

It looks like those are transitive dependencies of reqwest, which is the crate mdbook-linkcheck uses to make HTTPS requests. They are used for macOS bindings to Apple's security frameworks (for TLS). I think it is ok to commit the updated Cargo.lock, but thanks very much for being careful!

Also, it seems like when you 'build' the book it runs the linkcheck automatically if it's configured on book.toml.

The way I did here, it just loads up the book using mdbook and linkchecks it (without fully building it/generating .htmls)

Yes, that is correct. On the rustc-guide repo, that is how we get Travis to run the linkchecker. In that case, we also want to build the book itself so that we can host it, but for toolstate, just linkchecking is fine.

[andrehjr]

Cool! I updated the command line description!

There's an issue when building rustbook on travisci:

[00:53:18] error: failed to run custom build command for `openssl-sys v0.9.40`
[00:53:18] process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/release/build/openssl-sys-aaef693af933fbb3/build-script-main` (exit code: 101)
[00:53:18] --- stdout
[00:53:18] cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_LIB_DIR
[00:53:18] cargo:rerun-if-env-changed=OPENSSL_LIB_DIR
[00:53:18] cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_INCLUDE_DIR
[00:53:18] cargo:rerun-if-env-changed=OPENSSL_INCLUDE_DIR
[00:53:18] cargo:rerun-if-env-changed=X86_64_UNKNOWN_LINUX_GNU_OPENSSL_DIR
[00:53:18] cargo:rerun-if-env-changed=OPENSSL_DIR