search

rust-lang / docs.rs

crates.io documentation generator
https://docs.rs
MIT License
976 stars 195 forks source link

Link to dependency of dependency #1588

Closed djeedai closed 6 months ago

djeedai commented 2 years ago

This is a follow-up issue from a discussion on Discord with @Nemo157 about a bug in a crate's doc trying to link a type in another crate, which is actually a re-export from a third crate (dependency-of-dependency).

Description

The situation is as follow:

*I'm using "facade" in this issue as what I think it means, a "top-level" crate that re-export many other "internal" ones, but maybe I misunderstood the meaning given in rust-lang/rust#22083. If so, sorry for the confusion, and please take "facade" as the meaning I just described.

The result is that unfortunately links on docs.rs are broken, rendering verbatim instead of being actual links to the HTML page of the documentation of the type.

Bug example

_Note: To avoid confusion, the example is from my crate bevy_tweening, which is not an official Bevy crate, and not part of the official repo. So in particular that crate has no direct access to internal Bevy crates, and should be seen as "third-party"._

See an example of published crate here, where the [Entity] link toward the top of the page doesn't link anywhere (the HTML link contains the verbatim Rust path bevy::ecs::entity::Entity). The source code those docs are generated from is:

https://github.com/djeedai/bevy_extra/blob/f9c8049fb7674ea314a2c1a9c59a2b316d5d0abf/crates/bevy_tweening/src/lib.rs#L32

For reference (as for some reason GitHub refuses to expand the above link into code) the link is:

/// Animate the position ([`Transform::translation`]) of an [`Entity`]:
///
/// ...
///
/// [`Entity`]: bevy::ecs::entity::Entity

The Entity type is defined in the internal crate bevy_ecs, then re-exported in the facade crate bevy under the bevy::ecs::entity::Entity path.

Repro steps

I'm mentioning it here for completeness because @Nemo157 provided the command to run to replicate what docs.rs does, to test this:

Workaround

A workaround would be to directly link the internal type. Unfortunately since Bevy uses a facade crate, a third-party crate typically does not directly take a dependency to those Bevy internal crates, only to the facade, so simply changing the link makes the build fail. This means in addition of changing all broken links to target internal Bevy crates, a crate maintainer has to add a docs.rs-specific feature to depend on all those internal crates for documentation purpose only.

The html_root_url attribute was also mentioned, and I'm chatting with the Bevy docs team (CC: @alice-i-cecile) to see if we can try it, but we haven't yet.

Related issues

As explained by @Nemo157 on Discord, the scenario detailed above is likely related to rust-lang/rust#22083 and rust-lang/rfcs#3011, although it was under our impression that those issues do not cover entirely the problem at hand, and in particular the dependency-of-dependency linking.

djeedai commented 2 years ago

Note: unfortunately the workaround does not work for Bevy, I believe due to some bug in the Component macro derive. I'm not sure this can be fixed, so that makes this issue all the more annoying for Bevy crate maintainers.

jyn514 commented 2 years ago

I don't quite understand why this happens (I would expect -Zrustdoc-map to fix any issue with facade crates), but if you can reproduce it locally then it's a rustdoc bug and not a docs.rs bug; you should report it in rust-lang/rust.

Nemo157 commented 2 years ago

The issue is with -Zrustdoc-map being shallow; so technically this is a cargo bug (IIRC there's no upstream issue directly about this, just an unresolved question on the tracking issue). But it's also a docs.rs bug since our use of -Zrustdoc-map should be transparent to users; and we're almost certainly the majority users of the feature currently so it's likely up to us to fix it.

Nemo157 commented 2 years ago

Ah, and I remember what the related issues are about now. If we fix -Zrustdoc-map to support deep linking; then this will not actually support what bevy wants, which is to shallowly link to the facade rather than their internal crates.

jyn514 commented 1 year ago

cc https://github.com/rust-lang/cargo/issues/8296#issuecomment-1200527696, https://github.com/rust-lang/rust/issues/42301.

Ah, and I remember what the related issues are about now. If we fix -Zrustdoc-map to support deep linking; then this will not actually support what bevy wants, which is to shallowly link to the facade rather than their internal crates.

@dtolnay had an interesting suggestion in #2081:

If rustdoc in --no-deps mode is building the docs of crate A (tracing-opentelemetry) and has an intra doc link to opentelemetry::trace::SpanKind in crate B (opentelemetry) which is a re-export of the type opentelemetry_api::trace::SpanKind from crate C (opentelemetry_api), the following situations could occur:

If rustdoc has --extern-html-root-url=B=... but not --extern-html-root-url=C=..., today it generates a broken link. Is it possible / would it be reasonable to generate a link to the correct path under B's docs which will perform the necessary redirect to C's docs?

I think that would do the correct thing for bevy, without needing https://github.com/rust-lang/rfcs/issues/3011 to be fixed first, but it will do the wrong thing for non-facade crates, so making -Zrustdoc-map include transitive dependencies in the future would be a regression for bevy.

As an aside, this issue is large/complicated enough and spread across sufficiently many projects that I think any change here should involve an RFC.

Nemo157 commented 6 months ago

Since a few nightlies ago, -Zrustdoc-map has been including transitive dependencies too, (https://github.com/rust-lang/cargo/pull/13481), so links should be working, but will go all the way to the origin crate, not the facade.

syphar commented 6 months ago

@Nemo157 so this issue can be closed? Or is something is missing?

Or do we just need a rebuild?

Nemo157 commented 6 months ago

Yeah I guess so, the main thing we somewhat controlled about getting the html-root-url passed through has been fixed, having links to the facade instead of the origin requires a new rustdoc feature (tracked in https://github.com/rust-lang/rfcs/issues/3011) and shouldn't require any docs.rs changes.