-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Use TargetTriple::from_path in rustdoc #85361
Use TargetTriple::from_path in rustdoc #85361
Conversation
r? @jyn514 (rust-highfive has picked a reviewer for you, use r? to override) |
|
Can you add a test for the new behavior (maybe a run-make test)? Where is this logic for rustc, is it possible to reuse it? |
The logic for rustc is at rust/compiler/rustc_session/src/config.rs Lines 1510 to 1521 in 544d124
|
Great. Can you make that function public and use it in rustdoc instead of duplicating it? |
@bjorn3 you made the function public, but didn't use it in rustdoc. Also it would be nice to have the test. |
c707150
to
458a85c
Compare
I did |
The error in Rust-for-Linux/linux#258 (comment) confuses me (that is the error this fixes, right?) but if you can write dummy no_core test that fails before and passes after, that sounds great :) |
Yes
The problem is that when using rustc the full canonicalized path of the target spec json would be used by rustc and thus end up in the metadata of libcore and other dependencies, but when trying to use rustdoc, only the literal value of
👍 |
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.
r=me if you confirmed the test fails before this PR (I can't check right now).
@bors delegate=bjorn3
@@ -0,0 +1,9 @@ | |||
-include ../tools.mk |
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.
nit: can you use include
instead so it gives an error if the file is missing?
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.
Sure, I just copied another rustdoc test.
This comment has been minimized.
This comment has been minimized.
@bors delegate=bjorn3 |
✌️ @bjorn3 can now approve this pull request |
60579be
to
0c99e00
Compare
I manually ran the commands in the test. It failed with a nightly rustc/rustdoc. @bors r=jyn514 |
📌 Commit 0c99e003cf06ed87998b4a0a05d9a331b1709701 has been approved by |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@bors r- (until CI is passing) |
This comment has been minimized.
This comment has been minimized.
128f33b
to
b6d88d3
Compare
This comment has been minimized.
This comment has been minimized.
b6d88d3
to
f4a9ed9
Compare
f4a9ed9
to
6afc1f4
Compare
Fixed the test. @bors r=jyn514 |
📌 Commit 6afc1f4 has been approved by |
…onicalize, r=jyn514 Use TargetTriple::from_path in rustdoc This fixes the problem reported in Rust-for-Linux/linux#272 where rustdoc requires the absolute path of a target spec json instead of accepting a relative path like rustc.
…laumeGomez Rollup of 6 pull requests Successful merges: - rust-lang#85361 (Use TargetTriple::from_path in rustdoc) - rust-lang#85605 (Replace Local::new(1) with CAPTURE_STRUCT_LOCAL) - rust-lang#85631 (Move keyword primitive css dom) - rust-lang#85644 (Better English for documenting when to use unimplemented!()) - rust-lang#85650 (Add some backticks to the `rustc_middle::ty::adjustment::Adjustment` docs) - rust-lang#85657 (Remove doubled braces in non_exhaustive structs’ documentation text.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
In Rust-for-Linux#272 `realpath` was added to work around a bug in `rustdoc`, but it is not needed anymore since rust-lang/rust#85361, i.e. since Rust 1.54.0. Signed-off-by: Miguel Ojeda <[email protected]>
This fixes the problem reported in Rust-for-Linux/linux#272 where rustdoc requires the absolute path of a target spec json instead of accepting a relative path like rustc.