Support multiple crate versions in --extern-html-root-url #143465
Conversation
|
r? @SparrowLii rustbot has assigned @SparrowLii. Use |
rustbot
added
S-waiting-on-review
kornelski
changed the title
|
And f9f6a70 is just a bit of redundant code I've noticed in the process, too small to give it a separate PR. |
This comment has been minimized.
This comment has been minimized.
|
cc @aDotInTheVoid @obi1kenobi - i haven’t checked if this will help distinguish versions for the json backend too but i think it might |
|
Yes, the URL is in the JSON, and can be creatively used to identify the crates. |
|
to be clear i do not think html-root-url should be used for this, i’m saying the new metadata you’ve added to CrateRoot might allow the json backend to emit structured metadata. |
|
#76296 doesn't demonstrate the entirety of how rustdoc has issues with resolving crate versions. There's also the case of non-renamed transitively reëxported multiple versions of a single package name. |
|
I think the name resolution method used here should be documented in the rustdoc book |
Tracks association between `self.sess.opts.externs` (aliases in `--extern alias=rlib`) and resolved `CrateNum` Intended to allow Rustdoc match the aliases in `--extern-html-root-url` rust-lang#76296 Force-injected extern crates aren't included, since they're meant for the linker only
| Crate names in this flag refer to the names in the [extern prelude], and support crates renamed | ||
| via the `--extern` `rustc` flag. |
There was a problem hiding this comment.
I think there's a few improvements that could be made here:
- the original crate name is used as a fallback if there is no match in the extern prelude
- the fact that renaming crates via
--externlogically follows from the fact we are using the extern prelude - renaming via
extern crate asis also supported. - note that cargo's package renaming works via
--externand is thus supported.
I think this should be split into at least two sentences, one tersely and precisely describing the behavior (extern prelude with fallback), and another describing the consequence of this behavior (e.g. support for various methods of renaming, backwards compatibility)
| name: Symbol, | ||
| extern_crate: ExternCrate, | ||
| ) { | ||
| debug_assert_eq!(extern_crate.dependency_of, LOCAL_CRATE); |
There was a problem hiding this comment.
| debug_assert_eq!(extern_crate.dependency_of, LOCAL_CRATE); | |
| // this function should not be called on transitive dependencies | |
| debug_assert_eq!(extern_crate.dependency_of, LOCAL_CRATE); |
| cx.cache.extern_locations.insert(e.crate_num, location); | ||
| } | ||
| } | ||
|
|
||
| // Cache where all our extern crates are located | ||
| // FIXME: this part is specific to HTML so it'd be nice to remove it from the common code |
There was a problem hiding this comment.
if the doc url is emitted in the json, doesn't that make this fixme comment wrong? if so, i would replace it with a note saying that this needs to be common because it is actually emitted by both backends.
| //@ aux-crate:extern_prelude_name=empty.rs | ||
|
|
||
| extern crate extern_prelude_name as internal_ident_name; | ||
|
|
||
| //@ has extern_html_alias/index.html | ||
| //@ has - '//a/@href' 'https://renamed.example.com/empty/index.html' | ||
| pub use internal_ident_name as yet_different_name; |
There was a problem hiding this comment.
internal_ident_nameis a misleading name, as it actually does exist in extern prelude as an alias (i.e.::internal_ident_namewould be valid here, in this case both exist in the extern prelude)- we should test that
extern crate asaliases are handled correctly - we should test the fallback behavior (using crate name)
Doing all this would probably require at least a second aux-crate.
Rustdoc's
--extern-html-root-urlused to usetcx.crate_name()to identify crates, but that used crates' internal names from their metadata, instead of names given to them in--extern. That was confusing, because both--extern…arguments seem related and use similar syntax. Crucially, this didn't work correctly with Cargo's package aliases or multiple versions of crates.sess.opts.externslacksCrateNum, andResolver.extern_preludegets destroyed beforerustdochas a chance to see it, so I've had to save this mapping inCStore.Just in case, I've kept the previous mapping by crate name as a fallback for crates that weren't matched by their extern name.
Fixes #76296