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.

Sign up for GitHub

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

Jump to bottom

Tracking issue for -Zrustdoc-map #8296

Open
1 of 11 tasks
ehuss opened this issue May 30, 2020 · 8 comments
Open
1 of 11 tasks

Tracking issue for -Zrustdoc-map #8296

ehuss opened this issue May 30, 2020 · 8 comments
Labels
C-tracking-issue Category: A tracking issue for something unstable. S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. Z-rustdoc-map Nightly: rustdoc-map

Comments

@ehuss
Copy link
Contributor

ehuss commented May 30, 2020 •
edited
Loading

Original issue: #6279
Implementation: #8287
Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#rustdoc-map
Issues: Z-rustdoc-map Nightly: rustdoc-map

Summary
This feature adds the ability for Cargo to pass external documentation mappings to rustdoc, so that links to dependencies will use those external sites (like docs.rs). See the documentation for details on how to use it.

Unresolved issues

  • Does not properly handle renamed dependencies. The links rustdoc generates are to the package name, not the renamed name. Cargo is written to pass in package names, but if there are multiple dependencies to the same package, it won't work properly.
  • If there are multiple versions of the same package within the dep graph, rustdoc will only link to one of them. To fix this, Cargo would need to pass metadata info into rustdoc (such as the package version).
  • If a dependency is built with different features than what is on docs.rs, some links may break. This is probably unsolvable.
  • This increases the command-line length significantly. Before stabilizing, we may want to consider addressing that. I'm not sure if it would make sense to change rustdoc's interface, or to use response files?
  • Decide if Cargo should pass mappings for transitive dependencies (currently it does not). This normally isn't an issue, but can arise for re-exports (see the alt_registry test for an example). I'm not sure if this is a bug in rustdoc or not (there is a large number of issues regarding reexports and rustdoc). Cargo could include these, but this would make the command-line length even longer. Not sure what to do here.
  • The config value does not support being set via environment variables (like CARGO_CONFIG_DOC_EXTERN_MAP_REGISTRIES_CRATES_IO="…"). This would be very difficult to support, because Cargo doesn't retain the registry name in SourceId. I looked into fixing that, but it is very difficult, and hard to make it reliable. I would lean towards not bothering with this (it has similar issues as the source table).
  • Add a config option to control the default deps behavior of cargo doc. I can think of 3 options: all-deps, no-deps, direct-deps only. I think this would be useful, otherwise to use the rustdoc-map, one would always need to pass --no-deps. This would probably also need a new command-line flag to override the config option (maybe a generalization of --no-deps).
  • Consider changing the defaults. This is an important part of the UX story.
    • Default to crates-io = "https://docs.rs/". I think this should be relatively safe, since it only enhances the current behavior (people who use --no-deps will now have links instead of no links in their docs). DONE: Set docs.rs as the default extern-map for crates.io #8877
    • Change default of the deps behavior of cargo doc. I would prefer to get more community feedback to see what makes sense. Two options:
      • Direct dependencies only. I think this option is more realistic. It retains local browsing and indexing, and removes transitive dependencies which are usually not interesting. This would not engage rustdoc-map, so this is somewhat orthogonal.
      • Default to --no-deps. This would engage rustdoc-map, but is a much more drastic change. The user would not have local indexing available.
  • Should there be some way for the crate author to control the link destination? The documentation URL set in Cargo.toml is never consulted. I think this is unlikely to work with rustdoc, because it expects a certain structure to the API docs, and documentation URLs often point to user guides, or other things that aren't API docs. The #![doc(html_root_url = "...")] attribute is overridden by the --extern-html-root-url flag. This may be a good or bad thing depending on your perspective (should the user have control, or should the crate author?).

Future extensions
These things can be done after stabilization, but should be considered to ensure this feature doesn't make them more difficult.
@ehuss ehuss added C-tracking-issue Category: A tracking issue for something unstable. Z-rustdoc-map Nightly: rustdoc-map labels May 30, 2020
@anp anp mentioned this issue Jul 2, 2020
Open
@ehuss ehuss mentioned this issue Jul 15, 2020
Closed
@ehuss ehuss mentioned this issue Aug 4, 2020
Closed
@Nemo157 Nemo157 mentioned this issue Nov 11, 2020
Closed
This was referenced Nov 18, 2020
Closed
Closed
@dtolnay dtolnay mentioned this issue Nov 19, 2020
Merged
@jyn514 jyn514 mentioned this issue Nov 22, 2020
Merged
@jyn514
Copy link
Member

jyn514 commented Nov 22, 2020

Decide if Cargo should pass mappings for transitive dependencies (currently it does not). This normally isn't an issue, but can arise for re-exports (see the alt_registry test for an example). I'm not sure if this is a bug in rustdoc or not (there is a large number of issues regarding reexports and rustdoc). Cargo could include these, but this would make the command-line length even longer. Not sure what to do here.

There's another example at https://docs.rs/bevy_tiled_prototype/0.1.0/bevy_tiled_prototype/struct.TileMapChunk.html, which has broken links to Byteable and RenderResource, which come from bevy_render, re-exported through bevy: https://docs.rs/bevy/0.3.0/bevy/render/pass/trait.RenderPass.html.

I think this isn't a rustdoc bug because it has neither a documentation cache in target/doc, nor an --extern-html-root-url. So it doesn't know where to generate the links. In an ideal world, cargo would pass --extern-html-root-url only for transitive dependencies that have types visible in the direct dependencies, but I don't think cargo has a way to find out that info. Maybe rustc could expose it?

bors added a commit that referenced this issue Dec 1, 2020
@bors
Auto merge of #8877 - jyn514:rustdoc-map, r=alexcrichton
6de98d1 
Set docs.rs as the default extern-map for crates.io

Helps address #8296, specifically the bit needed for rust-lang/docs.rs#1177.
r? `@ehuss`
@jyn514
Copy link
Member

jyn514 commented Feb 19, 2021

This increases the command-line length significantly. Before stabilizing, we may want to consider addressing that. I'm not sure if it would make sense to change rustdoc's interface, or to use response files?

Rustdoc has response files since the latest nightly: rust-lang/rust#82261

@jyn514
Copy link
Member

jyn514 commented Feb 19, 2021

If there are multiple versions of the same package within the dep graph, rustdoc will only link to one of them. To fix this, Cargo would need to pass metadata info into rustdoc (such as the package version).

See also rust-lang/rust#78909, which is waiting on review.

@jyn514 jyn514 mentioned this issue Mar 4, 2021
Merged
@jyn514
Copy link
Member

jyn514 commented Mar 4, 2021 •
edited
Loading

The #![doc(html_root_url = "...")] attribute is overridden by the --extern-html-root-url flag. This may be a good or bad thing depending on your perspective (should the user have control, or should the crate author?).

I opened rust-lang/rust#82776 changing this; in practice, it should only hurt docs.rs (and I've asked the team to sign off before merging) and it gives crate authors more flexibility to point the links at their own site if they want to.

jyn514 added a commit to jyn514/rust that referenced this issue Aug 19, 2021
@jyn514
Give precedence to html_root_url over --extern-html-root-url by d…
18f0f24 
…efault, but add a way to opt-in to the previous behavior

 ## What is an HTML root url?

It tells rustdoc where it should link when documentation for a crate is
not available locally; for example, when a crate is a dependency of a
crate documented with `cargo doc --no-deps`.

 ## What is the difference between `html_root_url` and `--extern-html-root-url`?

Both of these tell rustdoc what the HTML root should be set to.
`doc(html_root_url)` is set by the crate author, while
`--extern-html-root-url` is set by the person documenting the crate.
These are often different. For example, docs.rs uses
`--extern-html-root-url https://docs.rs/crate-name/version` to ensure
all crates have documentation, even if `html_root_url` is not set.
Conversely, crates such as Rocket set `doc(html_root_url =
"https://api.rocket.rs")`, because they prefer users to view the
documentation on their own site.

Crates also set `html_root_url` to ensure they have
documentation when building locally when offline. This is unfortunate to
require, because it's more work from the library author. It also makes
it impossible to distinguish between crates that want to be viewed on a
different site (e.g. Rocket) and crates that just want documentation to
be visible offline at all (e.g. Tokio). I have authored a separate
change to the API guidelines to no longer recommend doing this:
rust-lang/api-guidelines#230.

 ## Why change the default?

In the past, docs.rs has been the main user of `--extern-html-root-url`.
However, it's useful for other projects as well. In particular, Cargo
wants to pass it by default when running `--no-deps`
(rust-lang/cargo#8296).

Unfortunately, for these other use cases, the priority order is
inverted. They want to give *precedence* to the URL the crate picks, and
only fall back to the `--extern-html-root` if no `html_root_url` is
present. That allows passing `--extern-html-root` unconditionally,
without having to parse the source code to see what attributes are
present.

For docs.rs, however, we still want to keep the old behavior, so that
all links on docs.rs stay on the site.
bors added a commit to rust-lang-ci/rust that referenced this issue Aug 21, 2021
@bors
Auto merge of rust-lang#82776 - jyn514:extern-url-fallback, r=Guillau…
b1928aa 
…meGomez

Give precedence to `html_root_url` over `--extern-html-root-url` by default, but add a way to opt-in to the previous behavior

## What is an HTML root url?

It tells rustdoc where it should link when documentation for a crate is
not available locally; for example, when a crate is a dependency of a
crate documented with `cargo doc --no-deps`.

 ## What is the difference between `html_root_url` and `--extern-html-root-url`?

Both of these tell rustdoc what the HTML root should be set to.
`doc(html_root_url)` is set by the crate author, while
`--extern-html-root-url` is set by the person documenting the crate.
These are often different. For example, docs.rs uses
`--extern-html-root-url https://docs.rs/crate-name/version` to ensure
all crates have documentation, even if `html_root_url` is not set.
Conversely, crates such as Rocket set `doc(html_root_url =
"https://api.rocket.rs")`, because they prefer users to view the
documentation on their own site.

Crates also set `html_root_url` to ensure they have
documentation when building locally when offline. This is unfortunate to
require, because it's more work from the library author. It also makes
it impossible to distinguish between crates that want to be viewed on a
different site (e.g. Rocket) and crates that just want documentation to
be visible offline at all (e.g. Tokio). I have authored a separate
change to the API guidelines to no longer recommend doing this:
rust-lang/api-guidelines#230.

 ## Why change the default?

In the past, docs.rs has been the main user of `--extern-html-root-url`.
However, it's useful for other projects as well. In particular, Cargo
wants to pass it by default when running `--no-deps`
(rust-lang/cargo#8296).

Unfortunately, for these other use cases, the priority order is
inverted. They want to give *precedence* to the URL the crate picks, and
only fall back to the `--extern-html-root` if no `html_root_url` is
present. That allows passing `--extern-html-root` unconditionally,
without having to parse the source code to see what attributes are
present.

For docs.rs, however, we still want to keep the old behavior, so that
all links on docs.rs stay on the site.
@jyn514
Copy link
Member

jyn514 commented Sep 11, 2021

This increases the command-line length significantly. Before stabilizing, we may want to consider addressing that. I'm not sure if it would make sense to change rustdoc's interface, or to use response files?

FWIW, rustdoc supports response files since 1.52: rust-lang/rust#82261

@jyn514 jyn514 mentioned this issue Sep 11, 2021
Merged
@jyn514 jyn514 mentioned this issue Jun 20, 2022
Open
@jyn514
Copy link
Member

jyn514 commented Jul 31, 2022 •
edited
Loading

Decide if Cargo should pass mappings for transitive dependencies (currently it does not). This normally isn't an issue, but can arise for re-exports (see the alt_registry test for an example). I'm not sure if this is a bug in rustdoc or not (there is a large number of issues regarding reexports and rustdoc). Cargo could include these, but this would make the command-line length even longer. Not sure what to do here.

I don't think this is a rustdoc bug, see #8296 (comment).

The way cargo manages this for normal dependencies is by passing -L dependency=target/debug/deps, which only needs a single flag for all dependencies. I wonder if we can do something like that here? Add some --extern-html-root-url-format=https://docs.rs/{package_name}/{version}/x86_64-unknown-linux-gnu flag to rustdoc? it would need to account for host vs target deps (probably just proc macros) somehow, but that seems doable, and it would avoid passing hundreds more flags than cargo currently passes.

@Nemo157
Copy link
Member

Nemo157 commented Aug 1, 2022

s/crate_name/package_name, other than that that sounds like a good approach if the package name is recorded in the .rmeta for rustdoc to use (or if it's plausible to add it).

@jyn514
Copy link
Member

jyn514 commented Aug 1, 2022

Currently it's not recorded, but I think it's plausible to add :) cargo already sets it through CARGO_PKG_NAME so we just need to change rustc to add it to metadata.

This was referenced Mar 11, 2023
Closed
Closed
@ehuss ehuss added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. labels Apr 25, 2023
@epage epage moved this to Unstable, baking in Cargo Roadmap Sep 5, 2023
@ehuss ehuss mentioned this issue Nov 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-tracking-issue Category: A tracking issue for something unstable. S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. S-waiting-on-feedback Status: An implemented feature is waiting on community feedback for bugs or design concerns. Z-rustdoc-map Nightly: rustdoc-map
Projects
Cargo Roadmap
Status: Unstable, baking
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ehuss @Nemo157 @jyn514