-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rustdoc: Fix source-links for files with absolute-paths #31835
Conversation
53bd2ee
to
78ca2e2
Compare
-include ../tools.mk | ||
|
||
all: | ||
printf "#[path=\"%s/%s\"] pub mod baz;\n" `pwd` "bar/baz.rs" > foo.rs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this could be a unix-only rustdoc test which has a module with the path as /dev/null
? That would avoid run-make and you could still check for the module page, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't think of that, but it seems like rustdoc doesn't generate files for empty public modules.
That did make another bug clear though, namely that absolute paths not contained in the root directory are still broken just like before.
(I'm gonna try to put these in a src/__root__
directory; for example /dev/null
-> src/__root__/dev/null.html
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm ok, I just try to avoid run-make like the plague as makefiles are basically impossible to get right on both windows and unix...
@bors: r+ 78ca2e256ed86906f5d295a129f3d51e124dbbeb |
78ca2e2
to
0db42c3
Compare
I pushed an update so that this also works for absolute paths not contained in the root directory. That would unfortunately be even worse to test, so I didn't include one. (works locally though) |
p.to_path_buf() | ||
} | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems kinda sketchy, perhaps the iterator below could use components
on Path
to only look at the Component::Normal
parts?
0db42c3
to
4037a7d
Compare
(updated) I've preserved the |
@bors: r+ 4037a7da77e529a421c4fe32e910f26abacae1af |
⌛ Testing commit 4037a7d with merge 4159981... |
💔 Test failed - auto-win-gnu-32-nopt-t |
`clean_srcpath` tries to make the source-path relative to `src_root`, but this didn't work since `src_root` itself wasn't absolute.
4037a7d
to
cf76fcf
Compare
I think I've made your This has required commit mitaa@27ca250, because |
fixes #26995
r? @alexcrichton