-
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
Distribute rust-docs-json via rustup. #102042
Distribute rust-docs-json via rustup. #102042
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon. Please see the contribution instructions for more information. |
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 with complete profile removed
We may also want to remove the check in bootstrap that gates building these on unstable_features(), since we just won't ship the component via the bulid-manifest nightly-only gate.
src/tools/build-manifest/src/main.rs
Outdated
|
||
// JSON docs are not yet stable, therefore we only include them in the nightly complete | ||
// profile for the time being | ||
self.extend_profile("complete", &mut manifest.profiles, &["rust-json-docs"]); |
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.
I wouldn't include them in the complete profile, nightly or not. In general the complete profile isn't really something we need to extend further IMO.
Just to be clear, these are still unstable? Is there a particular reason it doesn't have a preview tag? What is the relationship with #101799? Did that PR only add the ability to build them, but did nothing with the results? Are these built for all targets that have std docs built? |
@ehuss that PR built the JSON and added it to static.RLO, but I didn't realize changing the manifest it's a separate step, so currently is only possible to download the json manually, not through rustup. Using Yes, this is for all targets that have docs built. Yes, the json format is still unstable (similar to save-analysis, but we plan to stabilize it in the near future). |
Based on the comments above:
Let me know if you have further comments @Mark-Simulacrum! |
src/tools/build-manifest/src/main.rs
Outdated
@@ -318,6 +318,7 @@ impl Builder { | |||
package!("rust-mingw", MINGW); | |||
package!("rust-std", TARGETS); | |||
self.package("rust-docs", &mut manifest.pkg, HOSTS, DOCS_FALLBACK); | |||
self.package("rust-json-docs-preview", &mut manifest.pkg, HOSTS, DOCS_FALLBACK); |
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, so looking at this, can we rename the component to rust-docs-json-preview? I think keeping that common prefix will help with sorting the files etc and feels a little cleaner to me. I think we call them "JSON docs" in some places, so if you disagree I don't feel strongly at all about this and we can keep the current name (renaming it is more of a pain down the line).
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.
"JSON docs" feels more natural than "docs JSON", but I see your point. I don't feel strongly about either - @jyn514?
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.
The other 3-compenent name is llvm-tools-preview
. The other two doc components are rust-docs
and rustc-docs
.
rustc-docs-json
and rustc-json-docs
both seem like reasonable extensions to me, so I don't think there's an indicator there. But going from most specific to least specific seems good to me; rust-json-docs
kind of sounds like documentation about save-analysis or something. So I have a slight preference for rust-docs-json
.
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.
I've renamed everything to rust-docs-json
and squashed all commits 👍🏻
3d205fd
to
e572d51
Compare
@bors r+ rollup |
…to-rustup, r=Mark-Simulacrum Distribute rust-docs-json via rustup. I am not 100% sure on how to treat `rust-json-docs` in `target_host_combination`. I went along with a similar strategy to the one used for `rust-docs`, but looking for guidance there.
…to-rustup, r=Mark-Simulacrum Distribute rust-docs-json via rustup. I am not 100% sure on how to treat `rust-json-docs` in `target_host_combination`. I went along with a similar strategy to the one used for `rust-docs`, but looking for guidance there.
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#100734 (Split out async_fn_in_trait into a separate feature) - rust-lang#101664 (Note if mismatched types have a similar name) - rust-lang#101815 (Migrated the rustc_passes annotation without effect diagnostic infrastructure) - rust-lang#102042 (Distribute rust-docs-json via rustup.) - rust-lang#102066 (rustdoc: remove unnecessary `max-width` on headers) - rust-lang#102095 (Deduplicate two functions that would soon have been three) - rust-lang#102104 (Set 'exec-env:RUST_BACKTRACE=0' in const-eval-select tests) - rust-lang#102112 (Allow full relro on powerpc64-unknown-linux-gnu) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
I am not 100% sure on how to treat
rust-json-docs
intarget_host_combination
. I went along with a similar strategy to the one used forrust-docs
, but looking for guidance there.