RFR: 8316304: (fs) Add support for BasicFileAttributes.creationTime() for Linux [v11]
Alan Bateman
alanb at openjdk.org
Mon Oct 9 18:38:06 UTC 2023
On Mon, 9 Oct 2023 13:12:58 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?
>
> Severin Gehwolf has updated the pull request incrementally with two additional commits since the last revision:
>
> - Keep wrapper function signature compatible to statx
> - Refactor test so as to use Linker from JEP 454.
>
> @enablePreview still needed as of yet. This can go once
> JDK-8312522 integrates.
The update to UnixNativeDispatcher.c looks okay now.
test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTime.java line 30:
> 28: * @library ../.. /test/lib
> 29: * @build jdk.test.lib.Platform
> 30: * @enablePreview
It doesn't matter but I think the convention is to put `@enablePreview` before `@library` and `@build`.
test/jdk/java/nio/file/attribute/BasicFileAttributeView/CreationTime.java line 96:
> 94: // Creation time read depends on statx system call support
> 95: supportsCreationTimeRead = Linker.nativeLinker().defaultLookup().find("statx").isPresent();
> 96: System.out.println("[Linux] Debug: supportsCreationTimeRead == " + supportsCreationTimeRead);
I think you left a debug statement here, or if you didn't then maybe drop the work "Debug" as it looks out of place here.
-------------
Marked as reviewed by alanb (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/15792#pullrequestreview-1665083160
PR Review Comment: https://git.openjdk.org/jdk/pull/15792#discussion_r1350674479
PR Review Comment: https://git.openjdk.org/jdk/pull/15792#discussion_r1350675005
More information about the nio-dev
mailing list