RFR: 8316304: (fs) Add support for BasicFileAttributes.creationTime() for Linux

[Andrew John Hughes]

On Mon, 18 Sep 2023 17:05:11 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

> Please review this nio change which adds `BasicFileAttributes.creationTime()` support for Linux by means of the `statx` Linux specific system call. The patch does a dynamic lookup of the function and if it is available, uses it to set the support birth time capability in `UnixNativeDispatcher`. When `statx` is not available, it won't use it and fall back to the old behaviour on such systems. It should, however, compile fine on Linux systems not supporting the `statx` system call.
> 
> Testing:
> - [x] GHA (MacOS X failure seems unrelated)
> - [x] `java.nio` tests.
> - [x] Manual builds and tests on older glibc versions (2.17 => fallback, 2.28+ => works). Compiles fine and falls back to the not supported case as it was before this patch.
> 
> Thoughts?

Using either `statx` or `stat64` makes sense to me. I can see why you might want the safety of having the other attributes available on a system which thinks it has `statx`, but fails for some reason, but that also makes the situation hard to debug. Do you have no creation time because `statx` is not supported or because it failed? Also, in the proposed patch, the failing `statx` would fail the entire function anyway, so you wouldn't get the other attributes from `stat64` as far as I can see.

Looking at `statx`, the fields pretty much map onto the `stat64` ones with `st_` replaced with `stx_`. The exceptions are `rdev` and `dev` which `statx` provides split into major and minor. The `makedev` function can be used to convert this pair into a `dev_t`. 
We also seem to be missing use of nanoseconds for birthtime. Was this absent on Mac OS? It would be good to make this consistent with a/m/ctime and provide greater accuracy.

Using one or the other seems like it might also reduce some of the duplication here. It is a bit odd to have all attributes initialised in `prepAttributes` but then another adjusted outside that in three different places. Maybe we could instead just wrap `prepAttributes` in a wrapper function which calls `copyStatAttributes` or `copyStatXAttributes` as appropriate?  It seems feasible you could even copy the `statx` fields into a `stat64` buffer and re-use `prepAttributes` as is.

I don't see how moving this to a Linux-specific class would work. Would that not mean a lot of duplication with the `stat64` handling, not to mention some Linux systems may still need to use `stat64`? It's not like there isn't a whole bunch of platform-specific code in the file already and the separate class seems more appropriate for cases where the feature is only available on Linux, not where the feature can be handled slightly differently on some Linux systems.

Could `STATX_POSSIBLY_AVAILABLE` not just be replaced with `defined(__linux__)` for clarity? Or maybe that is part of the intention that `statx` may potentially be available on other platforms in future?

I don't really get the point about backports. According to [the build platform Wiki page](https://wiki.openjdk.org/display/Build/Supported+Build+Platforms), Oracle still build JDK 20 against a OEL 6 or 7 buildroot, which means a `glibc` that doesn't support `statx`. I expect most builds of 22 will be in an environment without `statx`, but plenty will run in one where it is.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15792#issuecomment-1728194468