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

Run rustdoc --check in CI to prevent broken intra-doc links #2557

Closed
sharnoff opened this issue Oct 4, 2022 · 1 comment · Fixed by #4711
Closed

Run rustdoc --check in CI to prevent broken intra-doc links #2557

sharnoff opened this issue Oct 4, 2022 · 1 comment · Fixed by #4711

Comments

@sharnoff
Copy link
Member

sharnoff commented Oct 4, 2022

I recently discovered that rustdoc has a --check flag to avoid generating documentation, and just produce warnings/errors. We currently have a number of broken internal doc links, so (after fixing them) it'd be great to automatically prevent more from being added.

Problems

Overall, running this right now might be a bit hacky:

Sample command

Currently, the following correctly runs, but requires nightly:

cargo +nightly rustdoc -p pageserver --lib -- -Z unstable-options --check

Notes:

  • -Z unstable-options is required for --check.
  • -p pageserver is required because rustdoc expects only a single crate
  • --lib is required because pageserver has both lib and bin targets, and rustdoc needs just one
  • (not shown) We have to also have to either pass --deny rustdoc::all or --deny rustdoc::$lint (for each lint) in order to cause rustdoc to exit with a non-zero status code

In general, requiring an extra nightly version in CI might be too much. It's "just" checking the code for dependencies + target crate, so not nearly as much as a full compilation -- it takes ~1.4 minutes on my machine (8 core/16 thread) to run the command above. It's then ~1 second for running on an additional crate (e.g., safekeeper).

In terms of possible nightly issues: I think we can mostly avoid stability issues by pinning to a particular version. We're also not compiling anything, so there's a lot of potential instability avoided there as well. It also might be possible to do something extraordinarily hacky and use nightly features with our stable tools (e.g, as in https://fasterthanli.me/articles/why-is-my-rust-build-so-slow#how-much-time-are-we-spending-on-these-steps). The really hacky version looks like:

RUSTC_BOOTSTRAP=1 cargo rustdoc -p pageserver --lib -- -Z unstable-options --check

It performs an initial run much faster (17.5s).

@sharnoff sharnoff added t/bug Issue Type: Bug and removed t/bug Issue Type: Bug labels Oct 4, 2022
@arpad-m
Copy link
Member

arpad-m commented Jul 13, 2023

I have filed #4711 to fix the warnings generated by rustdoc. As explained in that PR, running rustdoc currently requires nightly due to dependencies, but I think after update of opentelemetry, it should be possible to run cargo doc in CI without a --check param and then just not do anything with the result.

arpad-m added a commit that referenced this issue Jul 15, 2023
## Problem

`cargo +nightly doc` is giving a lot of warnings: broken links, naked
URLs, etc.

## Summary of changes

* update the `proc-macro2` dependency so that it can compile on latest
Rust nightly, see dtolnay/proc-macro2#391 and
dtolnay/proc-macro2#398
* allow the `private_intra_doc_links` lint, as linking to something
that's private is always more useful than just mentioning it without a
link: if the link breaks in the future, at least there is a warning due
to that. Also, one might enable
[`--document-private-items`](https://doc.rust-lang.org/cargo/commands/cargo-doc.html#documentation-options)
in the future and make these links work in general.
* fix all the remaining warnings given by `cargo +nightly doc`
* make it possible to run `cargo doc` on stable Rust by updating
`opentelemetry` and associated crates to version 0.19, pulling in a fix
that previously broke `cargo doc` on stable:
open-telemetry/opentelemetry-rust#904
* Add `cargo doc` to CI to ensure that it won't get broken in the
future.

Fixes #2557

## Future work
* Potentially, it might make sense, for development purposes, to publish
the generated rustdocs somewhere, like for example [how the rust
compiler does
it](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_driver/index.html).
I will file an issue for discussion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants