zig icon indicating copy to clipboard operation
zig copied to clipboard

std.Target.Query: Fix `parse` test on ABIs like `gnueabi`, `gnuabi64`, etc.

Open alexrp opened this issue 1 year ago • 2 comments

The zigTriple() implementation simply returns gnu when a glibc version is provided but a more specific ABI isn't.

https://github.com/ziglang/zig/blob/bb70501060a8bfff25818cf1d80491d724f8a634/lib/std/Target/Query.zig#L433-L445

I do realize I sound like a broken record at this point, but this is another problem we have because we conflate libc and ABI.

Note: I think this test has been broken for a long time, but it was not failing in CI because you need to be testing for a target like arm-linux-gnueabi, mips64-linux-gnuabi64, etc, and AFAIK the CI machines do not have non-native glibcs installed (which is a bit of a shame IMO, but I digress).

alexrp avatar Aug 17 '24 11:08 alexrp

You're trying to fix this on arm-linux-gnueabi but then the expected value is wrong because the zip triple should be "arm-linux-gnueabi".

andrewrk avatar Aug 23 '24 08:08 andrewrk

The issue is that the resulting Zig triple can't become *-*-gnueabi; there is simply not enough information provided for zigTriple() to determine whether it should return gnu, gnuabi64, gnueabi, etc for the ABI. So right now, it just returns gnu. And if we're targeting e.g. arm-linux-gnueabi (or gnueabihf, gnuabi64, ...), that means we construct the expected target triple native-native-gnueabi.2.1.1 which will not match native-native-gnu.2.1.1 as returned by zigTriple().

So as I see it, either a) the test needs to be fixed like I've done here, or b) the test is actually unworkable and should be deleted.

alexrp avatar Aug 23 '24 08:08 alexrp

There's a third option, you can skip the test on a non-applicable host.

andrewrk avatar Aug 23 '24 19:08 andrewrk

That works too I suppose. Although with #20690 accepted, the "API" component of the target becomes just gnu anyway, so in that light, perhaps it makes sense to take the PR as-is? :shrug: I don't actually have a strong opinion either way; I'd just like this test to stop failing my local multi-glibc builds.

alexrp avatar Aug 23 '24 22:08 alexrp

I see your point, OK :+1:

andrewrk avatar Aug 24 '24 05:08 andrewrk