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 |
| extern_crate.dependency_of, LOCAL_CRATE, | ||
| "this function should not be called on transitive dependencies" | ||
| ); | ||
| self.set_resolved_extern_crate_name(name, cnum); |
There was a problem hiding this comment.
I think it would be fine to do if extern_crate.dependency_of == LOCAL_CRATE { set_resolved_extern_crate_name } instead of splitting update_extern_crate into two function.
I doubt it will cause any perf regressions, but we can run the benchmarks.
There was a problem hiding this comment.
I've done this for clarity, not performance. Updating local crate imports and updating transitive dependencies are two different tasks.
Logically, the caller should call both:
store.set_resolved_extern_crate_name(name);
store.update_extern_crate(deps);
but it would be a bit fragile, since the caller could do one thing but forget the other, which is why I have one entrypoint for the local crate, but the transitive deps update separately.
There was a problem hiding this comment.
@petrochenkov I've split it into 117447f and dfdf5b8 to demonstrate the difference
|
This is better than I originally assumed, the crate lookup algorithm is not changed, only some already calculated data is exposed. |
petrochenkov
added
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
| The crate names in this flag are first matched against the names given in the `--extern name=` flags, | ||
| and fall back to using crates' own names otherwise (e.g. for transitive dependencies and sysroot crates). |
There was a problem hiding this comment.
Perhaps add a bit about using rlib names too to support multiple versions
|
☔ The latest upstream changes (presumably #144145) made this pull request unmergeable. Please resolve the merge conflicts. |
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` Force-injected extern crates aren't included, since they're meant for the linker only
|
The job Click to see the possible cause of the failure (guessed by this bot) |
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