PR CI: check-build the library for more targets

[RalfJung]

@saethlin and @onur-ozkan made it so that ./x.py check library --target=aarch64-pc-windows-msvc now works on all hosts, without special magic Windows tooling being needed. We should use that in PR CI to catch some more standard library build issues. In particular we are currently not covering

• Windows MSVC (we do check GNU)
• macOS

(Or, well, it turns out we run Miri tests for these targets so we cover them in x86_64-gnu-tools. But that seems to be a bit too accidental. And Miri doesn't complain if there are warnings, only if there are errors. And this sets cfg(miri) which changes some codepaths.)

@rust-lang/infra which runner would be the best one to add this to? AFAIK the mingw-check runner exists mostly to give us some test coverage for library builds on a Windows target, so maybe that would be a good one?

[saethlin]

Sysroot builds are nearly serial, and our runners have 8 vCPUs each. While Cargo supports multi-target builds and parallelizes them effectively, other tools like cargo-miri and bootstrap do not.

I've poked around the code in bootstrap for passing multiple targets down to the cargo invocation, but found it too challenging for me to do personally. I suspect the bootstrap team has enough understanding to do this much more easily.

[saethlin]

For anyone who wants a demo of how close-at-hand this is, you can just run this locally today:

cargo +nightly check -Zbuild-std --target=aarch64-unknown-linux-gnu --target=i686-pc-windows-gnu --target=i686-pc-windows-msvc --target=i686-unknown-linux-gnu --target=x86_64-apple-darwin --target=x86_64-pc-windows-gnu --target=x86_64-pc-windows-msvc --target=x86_64-unknown-linux-gnu

[onur-ozkan]

I believe this is more related to the infra team. Bootstrap team don't have any access to CI environments.

[RalfJung]

It's not about access to any environments, all files that need changing are in this repo. It's about adjusting our CI scripts to run these checks. No idea who's usually maintaining them...

[onur-ozkan]

It's not about access to any environments, all files that need changing are in this repo. It's about adjusting our CI scripts to run these checks. No idea who's usually maintaining them...

Right :facepalm: my bad.

[RalfJung]

@saethlin hm, this way of cross-checking does not seem to work for all hosts though...

$ ./x.py check library --target i686-pc-windows-gnu
Building bootstrap
    Finished dev [unoptimized] target(s) in 0.03s
thread 'main' panicked at src/core/sanity.rs:58:13:

couldn't find required command: "i686-w64-mingw32-gcc"

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Build completed unsuccessfully in 0:00:00

[onur-ozkan]

I think we can bypass target sanity check with BOOTSTRAP_SKIP_TARGET_SANITY for x check. Something like BOOTSTRAP_SKIP_TARGET_SANITY=1 x check library --target=i686-pc-windows-gnu should work I believe.

[RalfJung]

Yeah, I just thought that wouldn't be necessary any more. I guess I misunderstood.

[saethlin]

The target sanity checks were added so that you don't fail late when bootstrap does cc detection internally.

We removed the cc detection and use of its results in check builds. So the sanity checks aren't doing anything useful for check builds. Maybe there's enough context available when they're done to skip them for check builds.

[onur-ozkan]

So the sanity checks aren't doing anything useful for check builds.

Is this true even in build scripts?

[saethlin]

Aren't build scripts compiled for the host?

[onur-ozkan]

Aren't build scripts compiled for the host?

We do this sanity check for host too and bootstrap should complain about it if they are missing. I guess we can optimize it by performing the sanity check only for the host if it's a check build.

[ChrisDenton]

After testing some more, BOOTSTRAP_SKIP_TARGET_SANITY=1 works for most targets but not all. E.g. I'm not sure why but aarch64-apple-ios doesn't seem to honour BOOTSTRAP_SKIP_TARGET_SANITY.

I guess we can optimize it by performing the sanity check only for the host if it's a check build.

I didn't test this properly but I did hack out the sanity check and linker configuration which seemed to work for all targets (albeit with check errors on some, which is not unexpected).

[tgross35]

In https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/PSA.3A.20.2E.2Fx.20check.20can.20work.20for.20any.20target, running checks for T2 and T3 targets was discussed. Where would be the place to do this?

https://github.com/rust-lang/rust/pull/122401 used the main mingw-check, but it's probably not worth running T2/T3 on every PR.

Cc @Mark-Simulacrum

[Mark-Simulacrum]

No strong opinion. We should aim to minimize impact on overall build times, but otherwise just picking shortest builders feels reasonable. There's probably also a discussion to be had about cost/benefit -- increasing checks to some extent is a shift in our target tiering.

Tier 2 targets already largely get build-tested as part of the dist builders for them, so not sure there's sufficient value in additional checks in PR CI there. Tier 3 targets aren't, but that's sort of by intent; that tier specifically does not come with any CI costs.