fix: pass --extern-html-root-takes-precedence in addition to --extern-html-root-url on rustdoc invocation#16882
fix: pass --extern-html-root-takes-precedence in addition to --extern-html-root-url on rustdoc invocation#16882motorailgun wants to merge 1 commit into
Conversation
|
r? @epage rustbot has assigned @epage. Use Why was this reviewer chosen?The reviewer was selected based on:
|
cb08442 to
c030749
Compare
…ern-html-root-url` on rustdoc invocation
c030749 to
d14f0f5
Compare
| #[ignore = "Broken, temporarily disabled until https://github.com/rust-lang/rust/pull/82776 is resolved."] | ||
| #[cargo_test] | ||
| // #[cargo_test(nightly, reason = "--extern-html-root-url is unstable")] | ||
| #[cargo_test(nightly, reason = "--extern-html-root-url is unstable")] |
There was a problem hiding this comment.
I feel like this is understood backwards here.
rust-lang/rust#82776 was the reason we disabled the test. See #9824
(I personally don't really have the context of the back-and-forth of these flags though)
There was a problem hiding this comment.
Oh, sorry I didn't dig deeper into git blame. As per my understanding (reading through discussions and histories), --extern-html-root-takes-precedence preserves original behavior that cargo has/had been expecting, so I believe passing this to rustdoc would solve this breakage...?
There was a problem hiding this comment.
Not really. rust-lang/rust#82776 added --extern-html-root-takes-precedence and
rust-lang/rust#82776 (comment)
The flag here (--extern-html-root-takes-precedence) might be fine, I was just trying to understand what this was being added for. If it was for cargo, then I think this PR wasn't what we really needed. But if it was added for other users, then that would be good to know those use cases.
There was a problem hiding this comment.
Oops, I think I miss-read that. So IIUC there's still no fix for these cases on rustdoc's side?
There was a problem hiding this comment.
haven't really looked deeper though I believe so.
There was a problem hiding this comment.
Thanks, closing PR for now. Thank you for taking your time reviewing this!
The fix mentioned in tests (rust-lang/rust#82776) is already merged upstream, so I believe this works now.