Fix relative path handling for --extern-html-root-url by arferreira · Pull Request #152977 · rust-lang/rust

arferreira · 2026-02-22T16:38:12Z

When --extern-html-root-url receives a relative path (like ../), rustdoc uses it as a literal prefix regardless of page depth. This works at depth 1 (crate/index.html) but breaks at depth 2+ (crate/module/struct.Foo.html) because the relative path doesn't account for the extra nesting.

This patch detects relative vs absolute URLs in the Remote branch of url_parts and prepends the necessary ../ segments based on the current page depth. Absolute URLs (https://...) and server-absolute paths (/docs/...) are unchanged.

This makes relative paths a viable option for --extern-html-root-url, which they weren't before.

Related to #152917, which exposed that relative paths weren't handled correctly.

cc @eggyal

rustbot · 2026-02-22T16:38:16Z

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: rustdoc
rustdoc expanded to 9 candidates
Random selection from GuillaumeGomez, camelid, fmease, lolbinarycat, notriddle

eggyal

Nice work—thanks for this! I have a couple of small suggestions, and one question that's entirely down to my own lack of familiarity with the rustdoc codebase.

View changes since this review

eggyal · 2026-02-22T19:51:26Z

src/librustdoc/html/format.rs

    match cache.extern_locations[&def_id.krate] {
        ExternalLocation::Remote(ref s) => {
-            *is_remote = true;
+            let is_abs = s.contains("://") || s.starts_with('/');


There's actually the (uncommon) possibilities that someone specifies a network-path reference URI (which would begin // without the scheme: prefix); or a relative-path reference that happens to include :// somewhere within the path (for example ../this_is_silly://but_perfectly_valid).

Perhaps instead use a regex, such as that from Appendix B of RFC 3986, and treat the URI as absolute if (i) either of the scheme or authority components are present; or (ii) the path component begins with /.

Moreover, that's a check we only need to perform once when creating the ExternalLocation::Remote—so perhaps move it into an is_absolute boolean of the variant and avoid recomputing it every time we create a URL for that crate?

Updated — is_absolute is now computed once in to_remote using proper RFC 3986 scheme validation and stored in the variant. Also renamed is_remote to is_absolute throughout, which should answer your second question too.

eggyal · 2026-02-22T19:58:51Z

src/librustdoc/html/format.rs

        ExternalLocation::Remote(ref s) => {
-            *is_remote = true;
+            let is_abs = s.contains("://") || s.starts_with('/');
+            *is_remote = is_abs;


I'm not sure what this is_remote variable does? Is it the case that extern crates that are reached by a relative path should set it to false?

Yes, relative paths set it to false so that root_path still gets prepended in make_href, same as local paths. The name is confusing, your suggestion to store is_absolute in the variant would clean this up.

eggyal · 2026-02-22T20:00:12Z

src/librustdoc/html/format.rs

+                let extra = relative_to.len().saturating_sub(1);
+                let mut b: UrlPartsBuilder = iter::repeat_n("..", extra).collect();


I think we might be able to use let mut b = href_relative_parts(&[], relative_to); here instead?

It takes &[Symbol] so the types don't line up directly, and passing an empty slice to mean "go to root" is a bit of a semantic stretch IMO. The explicit repeat_n is clearer about intent here.

eggyal

Amazing, thanks again :) I've only really the one concern - that we might also need to consider relative paths when using such ExternalLocation::Remote.url elsewhere, but perhaps that's moot if they're guaranteed only to be output when based at the crate root?

View changes since this review

eggyal · 2026-02-23T01:22:21Z

src/librustdoc/clean/types.rs

+            let is_absolute = url.starts_with('/')
+                || url.split_once("://").is_some_and(|(scheme, _)| {
+                    scheme.bytes().next().is_some_and(|b| b.is_ascii_alphabetic())
+                        && scheme
+                            .bytes()
+                            .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'+' | b'-' | b'.'))
+                });


Makes me wonder whether we should do further sanity checking here, eg ensuring that url does not have query or fragment parts - but that can be dealt with as a separate issue/PR.

Agreed, worth a follow-up.

eggyal · 2026-02-23T01:25:06Z

src/librustdoc/html/format.rs

+        ExternalLocation::Remote { ref url, .. } => {
            // `ExternalLocation::Remote` always end with a `/`.
-            format!("{s}{path}", path = fmt::from_fn(|f| path.iter().joined("/", f)))
+            format!("{url}{path}", path = fmt::from_fn(|f| path.iter().joined("/", f)))


Are these links guaranteed only to appear based from the crate root, or do we also need to add appropriate levels of ../ for the relative url case here (and in the other few places I've commented "ditto" below)? If adjustment is required then I guess the code to do that can be factored out somewhere (a method on ExternalLocation perhaps)?

Yeahh, these other sites have the same gap. Before this PR none of them handled relative paths either. I'll expand the fix to cover them.

Okay, factored out a remote_url_prefix helper and applied it to the macro href and primitive link sites. The source link in context.rs and the make_href/root_path interaction are more involved since source pages have different depth semantics, so I'll handle those in a follow-up.

eggyal · 2026-02-23T01:26:24Z

src/librustdoc/html/format.rs

                        let cname_sym = ExternalCrate { crate_num: def_id.krate }.name(cx.tcx());
                        let builder: UrlPartsBuilder =
-                            [s.as_str().trim_end_matches('/'), cname_sym.as_str()]
+                            [url.as_str().trim_end_matches('/'), cname_sym.as_str()]


eggyal · 2026-02-23T01:29:06Z

src/librustdoc/html/render/context.rs

-                ExternalLocation::Remote(ref s) => {
-                    root = s.to_string();
+                ExternalLocation::Remote { ref url, .. } => {
+                    root = url.to_string();


If we can't properly handle relative extern locations here for now, could we add a FIXME: comment?

Added FIXME comments on both sites.

eggyal · 2026-02-23T01:32:34Z

src/librustdoc/json/mod.rs

                            name: e.name(self.tcx).to_string(),
                            html_root_url: match external_location {
-                                ExternalLocation::Remote(s) => Some(s.clone()),
+                                ExternalLocation::Remote { url, .. } => Some(url.clone()),


As above, perhaps add a FIXME: here?

eggyal · 2026-02-23T01:48:22Z

src/librustdoc/html/format.rs

-    if !is_remote && let Some(root_path) = root_path {
+    if !is_absolute && let Some(root_path) = root_path {
        let root = root_path.trim_end_matches('/');
        url_parts.push_front(root);
    }


Sorry, one more thing. If we have a url like ../../core/index.html that is relative to the current page, won't prefixing it with root cause it to break (unless root happens to be a path to the current directory)?

Sorry, can we add a FIXME: here too?

src/librustdoc/html/format.rs

src/librustdoc/clean/types.rs

src/librustdoc/html/format.rs

Co-authored-by: Michael Howell <michael@notriddle.com>

notriddle · 2026-02-24T03:56:47Z

tests/rustdoc-html/extern/extern-html-root-url-relative.rs

While looking for more bugs, I wrote a few test cases that you probably want to incorporate. All of them passed, so it seems fine. Just bringing them up:

diff --git a/tests/rustdoc-html/extern/extern-html-root-url-relative.rs b/tests/rustdoc-html/extern/extern-html-root-url-relative.rs index df6ebf1aedd..ba2b50c6bf2 100644 --- a/tests/rustdoc-html/extern/extern-html-root-url-relative.rs +++ b/tests/rustdoc-html/extern/extern-html-root-url-relative.rs @@ -1,4 +1,4 @@ -//@ compile-flags:-Z unstable-options --extern-html-root-url core=../ --extern-html-root-takes-precedence +//@ compile-flags:-Z unstable-options --extern-html-root-url core=../ --extern-html-root-takes-precedence --generate-link-to-definition // At depth 1 (top-level), the href should be ../core/... //@ has extern_html_root_url_relative/index.html @@ -9,7 +9,19 @@ // At depth 2 (inside a module), the href should be ../../core/... pub mod nested { //@ has extern_html_root_url_relative/nested/index.html - //@ has - '//a/@href' '../../core/iter/index.html' + //@ has - '//a/@href' '../../core/future/index.html' #[doc(no_inline)] - pub use std::iter; + pub use std::future; } + +// Also depth 2, but for an intra-doc link. +//@ has extern_html_root_url_relative/intra_doc_link/index.html +//@ has - '//a/@href' '../../core/ptr/fn.write.html' +/// [write](<core::ptr::write()>) +pub mod intra_doc_link { +} + +// link-to-definition +//@ has src/extern_html_root_url_relative/extern-html-root-url-relative.rs.html +//@ has - '//a/@href' '../../core/iter/index.html' +//@ has - '//a/@href' '../../core/future/index.html'

Added, thanks for the extra coverage.

notriddle · 2026-02-24T21:38:25Z

@bors r+

rust-bors · 2026-02-24T21:38:28Z

📌 Commit b3a41f2 has been approved by notriddle

It is now in the queue for this repository.

Rollup of 12 pull requests Successful merges: - #149169 (ptr::replace: make calls on ZST null ptr not UB) - #150562 (Fix doc link used in suggestion for pinning self) - #152418 (`BTreeMap::merge` optimized) - #152679 (rustc_expand: improve diagnostics for non-repeatable metavars) - #152952 (mGCA: improve ogca diagnostic message ) - #152977 (Fix relative path handling for --extern-html-root-url) - #153017 (Implement debuginfo for unsafe binder types) - #152868 (delete some very old trivial `Box` tests) - #152922 (rustc_public: Make fields that shouldn't be exposed visible only in `rustc_public`) - #153032 (Fix attribute parser and kind names.) - #153051 (Migration of `LintDiagnostic` - part 3) - #153060 (Give a better error when updating a submodule fails)

@eggyal

Rollup merge of #152977 - arferreira:fix-extern-html-relative-depth, r=notriddle Fix relative path handling for --extern-html-root-url When `--extern-html-root-url` receives a relative path (like `../`), rustdoc uses it as a literal prefix regardless of page depth. This works at depth 1 (`crate/index.html`) but breaks at depth 2+ (`crate/module/struct.Foo.html`) because the relative path doesn't account for the extra nesting. This patch detects relative vs absolute URLs in the `Remote` branch of `url_parts` and prepends the necessary `../` segments based on the current page depth. Absolute URLs (`https://...`) and server-absolute paths (`/docs/...`) are unchanged. This makes relative paths a viable option for `--extern-html-root-url`, which they weren't before. Related to #152917, which exposed that relative paths weren't handled correctly. cc @eggyal

rustbot assigned notriddle Feb 22, 2026

eggyal suggested changes Feb 22, 2026

View reviewed changes

rustbot added the A-rustdoc-json Area: Rustdoc JSON backend label Feb 23, 2026

eggyal reviewed Feb 23, 2026

View reviewed changes

arferreira force-pushed the fix-extern-html-relative-depth branch from 51db628 to 2f68c59 Compare February 23, 2026 13:09

eggyal reviewed Feb 23, 2026

View reviewed changes

src/librustdoc/html/format.rs Outdated Show resolved Hide resolved

Fix relative path handling for --extern-html-root-url

b9614b2

arferreira force-pushed the fix-extern-html-relative-depth branch from 2f68c59 to b9614b2 Compare February 23, 2026 14:59

notriddle reviewed Feb 23, 2026

View reviewed changes

src/librustdoc/clean/types.rs Outdated Show resolved Hide resolved

notriddle reviewed Feb 23, 2026

View reviewed changes

src/librustdoc/html/format.rs Show resolved Hide resolved

Update src/librustdoc/clean/types.rs

0da2c0c

Co-authored-by: Michael Howell <michael@notriddle.com>

notriddle reviewed Feb 24, 2026

View reviewed changes

Add test cases for intra-doc links

b3a41f2

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 24, 2026

jhpratt mentioned this pull request Feb 25, 2026

Rollup of 12 pull requests #153074

Merged

rust-bors bot merged commit 10edd16 into rust-lang:main Feb 25, 2026
11 checks passed

arferreira mentioned this pull request Feb 25, 2026

Local rustc-docs could make use of local pages for std types #151496

Open

		let extra = relative_to.len().saturating_sub(1);
		let mut b: UrlPartsBuilder = iter::repeat_n("..", extra).collect();

Uh oh!

Conversation

arferreira commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Feb 22, 2026

Uh oh!

eggyal left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eggyal left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eggyal Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

notriddle commented Feb 24, 2026

Uh oh!

rust-bors bot commented Feb 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

arferreira commented Feb 22, 2026 •

edited

Loading

eggyal left a comment •

edited by rustbot

Loading

eggyal left a comment •

edited by rustbot

Loading

eggyal Feb 23, 2026 •

edited

Loading