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

Rustdoc fingerprinting issues #9336

Closed
ehuss opened this issue Apr 8, 2021 · 5 comments · Fixed by #9404
Closed

Rustdoc fingerprinting issues #9336

ehuss opened this issue Apr 8, 2021 · 5 comments · Fixed by #9404
Labels
A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug Command-doc

Comments

@ehuss
Copy link
Contributor

ehuss commented Apr 8, 2021

#8640 has been causing some problems with rustbuild (see rust-lang/rust#83914 and rust-lang/rust#83530 (comment)). It was reverted in beta (#8640) to give more time to figure out some solutions.

Some possible ideas:

  1. Use rustdoc's --resource-suffix flag to force the common resources to use different filenames instead of deleting the the directory. I have concerns about this approach, as it may cause resource files to accumulate (when different versions are used), and may cause confusion if different packages are documented independently with different versions (the search indexes won't be shared, crates.js may be out of sync, etc.).
  2. Only delete the doc directory if the fingerprint changes. Don't delete the doc directory if the fingerprint is missing.
  3. Only delete existing files in the doc directory, and ignore any hidden files (don't delete the directory itself).

I'm leaning towards option 2, though I'm uncertain if it solves the problems (needs testing).

cc @CPerezz and @jyn514, if you maybe have other ideas or thoughts.

@ehuss ehuss added C-bug Category: bug Command-doc A-rebuild-detection Area: rebuild detection and fingerprinting labels Apr 8, 2021
@jyn514
Copy link
Member

jyn514 commented Apr 8, 2021

Only delete the doc directory if the fingerprint changes. Don't delete the doc directory if the fingerprint is missing.

This seems fine to me as long as we make sure it works 😉

I have concerns about this approach, as it may cause resource files to accumulate (when different versions are used)

Isn't this the same as any other compiler artifact (#5026)? I don't see why we should hold rustdoc to a higher standard than rustc. The shared files are tiny compared to compiler artifacts anyway.

Although I just realized --resource-suffix isn't stable, so it's not a great solution anyway :/ sorry for the noise.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 9, 2021

We do plan to eventually fix #5026 (and there has been movement on that, such as changing nightly to reuse filenames). A key difference is that rustdoc has problems with the shared files that rustc does not have. When the format changes, it can cause confusing errors for users (such as discussed here). Cargo knows which files rustc creates, but rustdoc is a black box, so it is harder for cargo to know how to clean up. It was anticipated that this would be a relatively easy fix, but apparently not as easy as hoped.

I do appreciate the suggestion, I don't consider it noise. I think the flag has a lot of potential and I still consider it a good option. There's just a lot of unknowns to consider (such as how people use rustdoc, and the unusual things like what rustbuild does). The more ideas the better! 😄

@jyn514
Copy link
Member

jyn514 commented Apr 9, 2021

Cargo knows which files rustc creates, but rustdoc is a black box, so it is harder for cargo to know how to clean up.

FYI you can find which files rustdoc creates since a few nightlies ago with --emit=unversioned-shared-resources,toolchain-shared-resources (rust-lang/rust#83478). I wasn't planning to stabilize that but I'd be willing to think about it if it's useful for cargo :)

@ehuss
Copy link
Contributor Author

ehuss commented Apr 24, 2021

I opened #9404 with a proposal that takes multiple approaches.

@bors bors closed this as completed in a3ff382 Apr 26, 2021
@jsha
Copy link
Contributor

jsha commented Sep 21, 2022

FYI I stumbled across this and wanted to cross-link rust-lang/rust#98413. My goal is that with that work, and some followups for dynamic files, we will get to a point where rustdoc does the right thing out of the box: static files are unique and identified by hashes; invocation-specific files have a suffix that disambiguates them by crate version and (probably) by rustdoc version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rebuild-detection Area: rebuild detection and fingerprinting C-bug Category: bug Command-doc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants