rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.91k stars 12.52k forks source link

Check tier 3 targets in CI #109099

Open jyn514 opened 1 year ago

jyn514 commented 1 year ago

NOTE: this issue does not affect the documented target tier policy of the Rust project. We reserve the right to break tier 3 targets at any time, according to the guidelines at https://doc.rust-lang.org/nightly/rustc/target-tier-policy.html#tier-3-target-policy.

Currently, maintaining std for tier 3 targets is very painful for t-libs: std::sys is highly platform dependent, and we have no tier 2 targets that use sys::unsupported so it's hard to tell if it even compiles. To reduce the maintenance burden, I propose we start running x check --target foo for each tier 3 target, to make sure it compiles.

Note that this does not involve distributing any release artifacts; users wanting to target tier 3 platforms will still need to build rustc from source or use -Zbuild-std, but it's more likely the standard library will at least compile, even if we make no guarantees about whether it will run correctly.

We have a few options about how to test this:

  1. Run this on PR CI only, which acts as a janky "allow-fail" action.
  2. Run this in bors merges, which requires it to pass for the PR to merge. To allow the target to fail to compile, we'd manually disable it in the src/ci test runner file.
  3. Add an allow-fail action which runs on bors merges, which doesn't block the merge but still lets us know when the target fails to build. I'm a little hesitant about this because I think it's rare people will actually look at the CI for the master branch, but maybe it's good enough.

I suggest that we wait to decide until the PR fixing these targets, since that will let us know the extent of the current breakage, and gives us a rough estimate of how much effort it will be to keep these compiling (option 2).

I'm very curious to hear @rust-lang/libs's opinion on this, particularly on the testing strategy and whether they think this will increase or decrease the maintenance burden.

cc https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/.60x.20check.60.20for.20tier.203.20targets


Mentoring instructions:

  1. Add a new CI job which runs x check --stage 1 for each tier 3 platform. https://github.com/rust-lang/rust/blob/4a2de63ae3d395481017aafbc37690c091d41d32/src/doc/rustc/src/platform-support.md#L211-L322 should be up to date; feel free to post here or on zulip if there are targets that are supported by the compiler but not in that document. 1a. maybe we should add a tool to keep those in sync?
  2. Put up a PR so the CI job runs. Fix compile errors as appropriate. Some targets may not compile on all hosts (e.g. iOS will likely need a macOS host, any MSVC target will need an MSVC host); either find a way to add support or add more runners so all targets are covered. Try to avoid spending too much time on any single target; if it takes too long it's ok to skip it for now. If it ends up being an ongoing issue to support a specific target we may want to consider asking one of the target maintainers (and if they're unresponsive, or there are no maintainers, making a t-compiler MCP to remove it).

cc @joboet, I think you said you were working on making it easier to cross-check targets from any host.

the8472 commented 1 year ago

Add an allow-fail action which runs on bors merges, which doesn't block the merge but still lets us know when the target fails to build. I'm a little hesitant about this because I think it's rare people will actually look at the CI for the master branch, but maybe it's good enough.

Maybe that could be added to the bors comment that happens when tests finish?

cuviper commented 1 year ago

Non-blocking feedback comments will need to propagate back to PRs that were rolled up too. That will need some state tracking, so it doesn't keep nagging if a tier-3 target stays broken for a while.

jyn514 commented 1 year ago

If we're going to do state tracking then IMO we should go all the way to the existing toolstate infrastructure, so we're not reinventing it from scratch.

m-ou-se commented 1 year ago

This was discussed two weeks ago in the library team meeting. There was a 3:2 split between

a. "just add the tier 3 checks as regular CI checks, and document that manually disabling tier 3 checks is fine", and b. "use something like the existing toolstate tracking (that doesn't block CI)".

(See Zulip.)

I voted for the second option, but maybe we should just try the first option to see how that works out. Maybe the kind of breakage that happens for those targets will be simple and small enough to not be an issue. (And if it does become an issue, we can change it in the future.)

The first option does probably become quite annoying if we don't have a good way to run those checks manually though. So I think we need local ./x.py check would "Just Work™" when cross compiling for all tier 3 targets, and/or there needs to be a way to ask bors to run those checks, or those checks should always run on PRs directly (or at least those that touch the standard library).

thomcc commented 1 year ago

The first option does probably become quite annoying if we don't have a good way to run those checks manually though.

How many (if any) targets currently supriously fail to check the parts of the stdlib they support[^1] if you deinit the llvm submodule? I think doing so fixes all of the failures I knew of (because we no longer try to build the compiler-rt code in compiler-builtins), but there might be some weird ones I don't know about.

Obviously even if the answer is "all of them", that's not a good long term solution (it's annoying and non-obvious to have to deinit it).

[^1]: E.g. obviously no_std-only targets should only check core (or core+alloc if they support alloc).

jyn514 commented 1 year ago

The first option does probably become quite annoying if we don't have a good way to run those checks manually though. So I think we need local ./x.py check would "Just Work™" when cross compiling for all tier 3 targets, and/or there needs to be a way to ask bors to run those checks, or those checks should always run on PRs directly (or at least those that touch the standard library).

this should be the case once https://github.com/rust-lang/rust/pull/102579 is merged