-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Change --extern-html-root-url to use crate source path as key #78909
Conversation
As well as testing the ability to override the url, run the same testcase with the `doc(html_root_url)` specified url.
r? @jyn514 (rust_highfive has picked a reviewer for you, use r? to override) |
This seems to expose build details a little bit - does cargo even provide a way to get the .so file that corresponds to a crate? I wouldn't want to make this breaking change without knowing there's a way to actually use it, if you could put together a demo of a cargo-based project that would make me feel a lot better. cc @eddyb - this changes rustdoc to look up info about a crate by the source path ( |
See the link to my |
It needs significant cleanup, but rust-lang/docs.rs@master...Nemo157:issue-76296 mostly implements this on the docs.rs side. |
Hm, using extern paths is an interesting approach! It does neatly solve the problem of having some unique identifier to correlate each extern crate. Another approach I had thought about was to encode the It would be nice to have some strategy to also fix #56169. I'm not sure how this could fit in with that, or if that will need to be solved with a completely different approach. I would be concerned about comparing paths directly. Using absolute paths tends to be tricky and runs the risk of not working in various situations. Regarding the concern about long command-line paths, I'm thinking that the best option is to use a response file. I'm a little uncomfortable doing that in Cargo (it tends to be a pain to work with), but I'm not sure if there are many other options. Even though I am the author of the The documentation should probably be updated. |
r? @ehuss |
One thought I just had to reduce the command size back a bit; we probably don't need the absolute path, given the metadata suffix just using the filename would probably be enough, so that would only add an extra ~26 characters per crate. |
☔ The latest upstream changes (presumably #79104) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
Triage: There's merge conflicts now. |
Ping from triage |
@Nemo157 any updates on this? |
Closing this as inactive |
This is the most correct solution I see to #76296, this allows setting the html-root-url for all crates in the dependency tree, no matter how many duplicates of a specific crate name exist.
I have prototyped the associated docs.rs changes in my script that builds in the same fashion as it: Nemo157/dotfiles@d452627. Before this is merged we would want to do the same for docs.rs itself (I'll make a PR for this shortly), it's forwards-compatible to start emitting both sets of flags and remove the current ones later, extra unused flags are just ignored.
fixes #76296