Make statx available for all Linux targets

[alyssais]

I think all this should be available on all Linux targets now, but I'm not very familiar with uClibc, so I'm not sure about it.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) soon.

Please see the contribution instructions for more information.

[alyssais]

Oh, is there no musl/uClibc CI?

[Amanieu]

We do have it but it is only run by bors, not the normal CI for PRs.

@bors r+

[bors]

📌 Commit d37715d has been approved by Amanieu

[bors]

⌛ Testing commit d37715d with merge 32413d6...

[bors]

💔 Test failed - checks-actions

[Amanieu]

You need to change libc-test/build.rs to include the statx header file in the C tests.

[JohnTitor]

Ping @alyssais, could you address the above comment?

[bors]

☔ The latest upstream changes (presumably #2808) made this pull request unmergeable. Please resolve the merge conflicts.

[germag]

AFIAK the current version of musl (1.2.3) doesn't implement a wrapper forSYS_statx.
But, I think it's worth leaving the rest (struct statx, constant, etc) available for all linux target

[alyssais]

My mistake, you're correct.

[germag]

I'm quite interested in this PR

[alyssais]

I've rebased the PR and updated it to drop the statx function, which as @germag pointed out, I was wrong about being present on Musl. Hopefully this also avoids the need to add a header to build.rs, although I'm not sure.

[alyssais]

Thinking about it some more, is it right to expose even the structs if Musl doesn't? I thought it did, but it turns out the struct statx in Musl is internal-only, so to use it in a C program one would have to import <linux/stat.h>. What if Musl came up with its own statx API that used different structs?

[germag]

Thinking about it some more, is it right to expose even the structs if Musl doesn't? I thought it did, but it turns out the struct statx in Musl is internal-only, so to use it in a C program one would have to import <linux/stat.h>. What if Musl came up with its own statx API that used different structs?

if that were to happen, which I don't think it would, they would be incompatible with both the glibc and linux, since the statx struct is defined in the kernel headers.

[germag]

sorry for stepping in, are there any additional problems with this PR?

[JohnTitor]

You need to change libc-test/build.rs to include the statx header file in the C tests.

The diff doesn't include the change mentioned above, have you tested that libc-test passes?

[bors]

☔ The latest upstream changes (presumably #2941) made this pull request unmergeable. Please resolve the merge conflicts.

[alyssais]

Thinking about it some more, is it right to expose even the structs if Musl doesn't? I thought it did, but it turns out the struct statx in Musl is internal-only, so to use it in a C program one would have to import <linux/stat.h>. What if Musl came up with its own statx API that used different structs?

if that were to happen, which I don't think it would, they would be incompatible with both the glibc and linux, since the statx struct is defined in the kernel headers.

I followed up on this in the #musl IRC channel, and the consensus there was that there's no guarantee that kernel headers and libc headers will be compatible for the same functionality, and that the libc crate probably shouldn't expose interfaces that come straight from the kernel but are not exposed by libc.

If libc does this in other places, then maybe it's okay, but I'd really like to hear the thoughts of a libc crate maintainer on this specific issue before I go further with this, since I'm not confident it's the way to go.

[workingjubilee]

Mmm. I don't think we should be exposing this for all Linux targets. Mister Musl is correct, here. For instance, there's AT_{ElfAuxval} constants that are exposed by musl and glibc on a target-neutral basis, but that are not defined in a target-neutral way by the kernel. Those libcs have made their own commitments, effectively, on how to make that work going forward, in the (admittedly unlikely) event the kernel introduces a new definition which clashes. I believe it's okay for us to "round up" a bit for certain definitions, but the lack of support from a notable and widely-used libc makes me hesitate. Fixing a struct definition can be easier than fixing a function definition, but it is definitely still annoying to do and liable to be highly breaking.

[JohnTitor]

Triage: The arguments above make some sense to me. Let's close this PR for now and continue the discussion if someone wants to support it strongly. Thanks @alyssais for the PR anyway!