cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Tracking issue for -Zrustdoc-map

Open ehuss opened this issue 5 years ago • 5 comments

Original issue: #6279 Implementation: #8287 Documentation: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#rustdoc-map Issues: https://github.com/rust-lang/cargo/labels/Z-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.
    • [X] 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: #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.

  • Single tab browsing. This would be a mode where the std docs are merged with the local crate's docs so that the std docs are shown in the same place (and included in the index). This could be expressed with something like doc.extern-map.std = "include" or something like that. (Or maybe just use build-std?) https://github.com/rust-lang/cargo/issues/6279#issuecomment-629670934
  • Direct-dependencies only. Often transitive dependencies aren't that interesting, and take up a lot of space in the output, and clog the search index. Some users want the ability to (locally) document their package + direct dependencies only. I think this could be implemented with some kind of command-line flag, perhaps with a config setting in the [doc] table. --extern-html-root-url flag will automatically handle second-level dependencies. #2025 #2801 #6421 https://crates.io/crates/cargo-makedocs
  • Manual-exclusions. Sometimes there are specific dependencies that are very expensive to document locally, but you still want everything else. I think this could be implemented with a command-line flag (--exclude winapi?), and the rustdoc-map feature would automatically link those excluded crates' items to docs.rs. This could also be added to the [doc] table. #4049

ehuss avatar May 30 '20 17:05 ehuss

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?

jyn514 avatar Nov 22 '20 19:11 jyn514

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: https://github.com/rust-lang/rust/pull/82261

jyn514 avatar Feb 19 '21 19:02 jyn514

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 https://github.com/rust-lang/rust/pull/78909, which is waiting on review.

jyn514 avatar Feb 19 '21 19:02 jyn514

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 https://github.com/rust-lang/rust/pull/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 avatar Mar 04 '21 20:03 jyn514

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: https://github.com/rust-lang/rust/pull/82261

jyn514 avatar Sep 11 '21 15:09 jyn514

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 https://github.com/rust-lang/cargo/issues/8296#issuecomment-731837505.

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.

jyn514 avatar Jul 31 '22 23:07 jyn514

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).

Nemo157 avatar Aug 01 '22 10:08 Nemo157

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.

jyn514 avatar Aug 01 '22 12:08 jyn514