Add atypical targets to CI

[notgull]

This adds some Tier-2 and Tier-3 targets to CI, so we can detect
breakage in these platforms early.

cc #3735

• Tested on all platforms changed
• Added an entry to the changelog module if knowledge of this change could be valuable to users
• Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
• Created or updated an example program if it would help users understand this functionality
• Updated feature matrix, if new features were added or implemented

[madsmtm]

Hmm, not sure about this, our CI is already quite big, I'd rather cut down on it. Though I guess that doing it this way (only running cargo check, and doing it in a single "job") helps somewhat.

I'll note that Cargo supports compiling for multiple targets simultaneously (pass --target multiple times), might be worthwhile to check if that's faster (vs. the debugging experience if one target fails, of course).

[notgull]

Hmm, not sure about this, our CI is already quite big, I'd rather cut down on it. Though I guess that doing it this way (only running cargo check, and doing it in a single "job") helps somewhat.

The tradeoff is that otherwise we are completely unaware of such CI failures until they're reported to us. I figure that doing a check here is a good compromise between "no checks" and, say, using qemu to run a full test suite.

I'll note that Cargo supports compiling for multiple targets simultaneously (pass --target multiple times), might be worthwhile to check if that's faster (vs. the debugging experience if one target fails, of course).

The issue is that we also want to test various feature combinations as well, but at least this will narrow it down to three calls.

[kchibisov]

I'd suggest to add only platforms which are only actually different, as in, you should cover possibilities and not all the cases.

For example, c_char is either i8 or u8, so you need some target where it's u8 (arm). You can also try to spot other differences, like where int/float, etc are different, but I don't really think you'll find anything other than c_char flip.

[kchibisov]

As for musl, we need a musl based image, I guess.

[notgull]

I'd suggest to add only platforms which are only actually different, as in, you should cover possibilities and not all the cases.

The issue is that platforms may eventually become different. For instance FreeBSD regularly has breaking changes where fundamental types change. So in order to insulate us from unexpected breakage we need to cast a pretty wide net.

I'm glad to reduce the permutations here, but I think that the net this PR casts is just wide enough to capture all the permutations we care about (e.g. x86_64 vs aarch64 vs s390x, the various BSDs). So I would appreciate guidance on which cases we are fine with breaking for users.

[kchibisov]

The issue is that platforms may eventually become different

If the platform does so, then all the C stack has the issues as well. In general, I'd advice to have some sort of a lint that detects that the matching type alias is used, and not just the original type. Because the issue you're trying to prevent was caused by the fact that typedef was not used, but rather the original type.

[notgull]

In general, I'd advice to have some sort of a lint that detects that the matching type alias is used, and not just the original type.

Does this lint exist in the current version of Rust? I'm not sure this works, and the only real way to check this is to actually call cargo check.

[kchibisov]

There's no way and no indications in clippy I've also opened an issue rust-lang/rust-clippy#12988 to at least suggest correct types and not expanded.

[kchibisov]

For now, I'd suggest to just test arm build, because it covers the c_char difference.

[notgull]

For now, I'd suggest to just test arm build, because it covers the c_char difference.

I've added the other builds for the following reasons:

• musl, because it's a popular libc target and allows us to support Alpine Linux.
• gnux32, which is used frequently in multiarch scenarios.
• FreeBSD/NetBSD/OpenBSD as they are popular unix flavors.
• riscv, as it is a less used architecture but still used frequently in some applications (i.e. riscv computers are gaining more popularity)
• s390x/sparc64 as a representative sample for big-endian systems.

[kchibisov]

To make things work on alpine, you should target alpine in particular, since every musl setup has their own quirks. I know that winit works just fine on alpine though.

I'm not sure how often x32 is even used, I think only gentoo has it as an option and that's about it.

FreeBSD and other BSDs should be tested on CI, not their target, because it's not really representative IIRC.

riscv has nothing special when it comes to testing.

s390x is a mainframe IIRC and I'm not sure that winit should target those, since they likely don't have graphics in the common sense. Especially given that you exclude Solaris.

I'd just suggest to add arm64 on CI and if you really want to have a separate CI (maybe powered by sr.ht) with FreeBSD, alpine. I can register the runner if you really want to.

[daxpedda]

Personally I don't mind having the CI run a bit longer. I think there are still some low-hanging fruit to improve the total time, so hopefully I will get to that soon.

I think it would also be nice to document these targets as "T2" somewhere as well.

[daxpedda]

Its actually more performant to split these into multiple steps, or use a matrix in this case, I believe.

[kchibisov]

The point is that they add nothing valuable other than build CI time, only arm64 is sufficient to test 99% of the edge cases, because of different char.

[daxpedda]

The point is that they add nothing valuable other than build CI time, only arm64 is sufficient to test 99% of the edge cases, because of different char.

I believe that this is adding "nothing valuable" has already been disputed by:

I'd suggest to add only platforms which are only actually different, as in, you should cover possibilities and not all the cases.

The issue is that platforms may eventually become different. For instance FreeBSD regularly has breaking changes where fundamental types change. So in order to insulate us from unexpected breakage we need to cast a pretty wide net.

While I agree that this is pretty messed up, if this is true, then checking these platforms does indeed provide some value, as long as we don't have a lint in place which would mitigate this issue otherwise (or some other way to prevent it).

I think the true discussion that should be happening here is to compare the added CI time vs the benefit. Though I agree that the benefits are quite low.

@notgull I think we can better discuss this if you would try to parallelize the checks as I noted in my review (or find out that I was wrong about that being better) and compare the runtime to our current CI and tell us how it went.

[notgull]

I think the true discussion that should be happening here is to compare the added CI time vs the benefit. Though I agree that the benefits are quite low.

I'd argue not being blindsided by build failures is a pretty big benefit.

@notgull I think we can better discuss this if you would try to parallelize the checks as I noted in my review (or find out that I was wrong about that being better) and compare the runtime to our current CI and tell us how it went.

There isn't much of a point; checks are pretty fast and all of the jobs complete way before the mainline test jobs anyways.

[daxpedda]

I think the true discussion that should be happening here is to compare the added CI time vs the benefit. Though I agree that the benefits are quite low.

I'd argue not being blindsided by build failures is a pretty big benefit.

I believe that @kchibisov made a good point that the only thing we are checking are bunch of type permutations, which should be covered by much fewer targets. If you disagree or think we are missing something here, it would be great if you elaborate so we can agree on the value provided here.

@notgull I think we can better discuss this if you would try to parallelize the checks as I noted in my review (or find out that I was wrong about that being better) and compare the runtime to our current CI and tell us how it went.

There isn't much of a point; checks are pretty fast and all of the jobs complete way before the mainline test jobs anyways.

Looking at the last run the new checks not only lasted the longest, but also ran the last. So I'm a bit confused about your statement.

From the timings it seems to me that parallelizing this step should speed up the overall time significantly.

Considering that all the data is here, I would appreciate if we manage to use it to make the conversation more constructive.

[daxpedda]

• Why are we using --release?
• The output of -vv seems to make the outout quiet hard to parse to me.
• Why --all-targets? What exactly are we testing in this case?