Skip to content
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

Move platform support to the rustc book. #75431

Merged
merged 3 commits into from
Aug 13, 2020
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Aug 12, 2020

This moves the Platform Support page from the forge to the rustc book. There are several reasons for doing this:

  • The forge is not really oriented towards end-users (it mostly contains infrastructure, governance and policy, internal team pages, etc.). This platform support page is useful to user to know which targets are supported.
  • This page can now be updated in-sync with any PRs that add or remove a target, or change its status.
  • This is now automatically checked on CI to verify the list does not get out of sync. Currently it only checks the presence/absence of an entry, but more sophisticated checks could be added in the future.

I'm not 100% certain this is the best location, but I think it fits. I'd like to see the rustc guide continue to grow, including things like linking information and more platform-specific details.

A few updates:
- Some minor wording and formatting changes.
- Remove the `cargo` column.
- Explain the columns up-front.
- Add no-wrap on the target-triple, which looks better to me.
- Minor mention on how to install support for a built-in target via rustup.
@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 12, 2020
@ehuss
Copy link
Contributor Author

ehuss commented Aug 12, 2020

cc @Mark-Simulacrum per discussion at rust-lang/rust-forge#413 (comment).

@Mark-Simulacrum
Copy link
Member

I'm going to set myself as reviewer here, but I would also like to cc @rust-lang/compiler and @rust-lang/release as the primary teams involved with decisions around platform support. I agree with @ehuss that this is probably a pretty good place.

r? @Mark-Simulacrum

r=me with CI fixed

@ehuss
Copy link
Contributor Author

ehuss commented Aug 12, 2020

I did not know that the AppleOS targets don't exist on Linux. I pushed a fix so the test only runs on macOS. I feel a little uncomfortable with that, because in the future if any target is added that doesn't exist on macOS, it won't work. It also seems a little hacky. I can't think of too many other better options. One would be to just manually scan the supported_targets! by just reading that file. WDYT?

@Mark-Simulacrum
Copy link
Member

I think it may be worth noting that we don't support full cross-compilation (if I'm following you correctly) in the Markdown file somehow, and then the collector can check subsets appropriately (e.g., macOS would test that everything we expect on macOS is indeed present, Linux would do the same, etc.)

@ehuss
Copy link
Contributor Author

ehuss commented Aug 12, 2020

Nah. The issue is that I'm using rustc --print=target-list to determine if the page contains the correct entries. macOS is the only platform that contains the full list (those ios/tvos targets are simply missing on Linux). The test fundamentally needs a complete list in order to determine if something is missing or should be removed.

@Mark-Simulacrum
Copy link
Member

Right, yes, my point is that if those targets are only present on macOS (i.e., cannot be cross compiled from Linux) we should note that on the platform support page.

Then, when the test runs on macOS, it will check to verify the presence of those targets. On Linux it will still check for all other targets, but ignore the mac-specific ones. I am also fine hard coding this into the test runner for now.

@ehuss
Copy link
Contributor Author

ehuss commented Aug 12, 2020

Oh, I see! Good point. I'll make that change, though it probably won't be till tomorrow. Thanks for the suggestion!

@pietroalbini
Copy link
Member

I'm wondering... would this be a good moment to automatically generate the page?

@ehuss
Copy link
Contributor Author

ehuss commented Aug 12, 2020

Updated with the apple exception.

@pietroalbini Do you have ideas on how that would work? I had thought about moving to some kind of structured format (like it used to be), but it didn't seem to really improve things.

The concern with computing it is that the data needs to reside somewhere. The tier status doesn't exist anywhere else, and I would be reluctant to add it to the spec definition. Whether or not host and std vs no_std is only encoded through various shell scripts in the CI directory, and I don't think that's really parseable. And the "notes" section would need to go somewhere.

One idea I had was to query the rustup manifest to at least check the host and std status, but that would need to be done outside of regular CI runs since the publish happens separately. One possibility is to set up a scheduled task (like once a day) that could query it and then send an alert if something is off (email? issue? zulip?). However, that would only be useful for targets that change their status, and that is extremely rare AFAIK.

Copy link
Member

@Mark-Simulacrum Mark-Simulacrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me with nit fixed

This is a script for validating the platform support page in the rustc book.

The script takes two arguments, the path to the Platform Support source page,
and the second argument is the path to `rustc`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this to a doc comment in main.rs?

@ehuss
Copy link
Contributor Author

ehuss commented Aug 12, 2020

@bors r=Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Aug 12, 2020

📌 Commit ce71747 has been approved by Mark-Simulacrum

@bors bors 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 Aug 12, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Aug 13, 2020
…lacrum

Move platform support to the rustc book.

This moves the [Platform Support](https://forge.rust-lang.org/release/platform-support.html) page from the forge to the rustc book. There are several reasons for doing this:

* The forge is not really oriented towards end-users (it mostly contains infrastructure, governance and policy, internal team pages, etc.). This platform support page is useful to user to know which targets are supported.
* This page can now be updated in-sync with any PRs that add or remove a target, or change its status.
* This is now automatically checked on CI to verify the list does not get out of sync. Currently it only checks the presence/absence of an entry, but more sophisticated checks could be added in the future.

I'm not 100% certain this is the best location, but I think it fits. I'd like to see the rustc guide continue to grow, including things like linking information and more platform-specific details.
@bors
Copy link
Contributor

bors commented Aug 13, 2020

⌛ Testing commit ce71747 with merge d69b099...

@bors
Copy link
Contributor

bors commented Aug 13, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing d69b099 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 13, 2020
@bors bors merged commit d69b099 into rust-lang:master Aug 13, 2020
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants