RFR: 8304006: jlink should create the jimage file in the native endian for the target platform [v21]
Jaikiran Pai
jpai at openjdk.org
Sat Jul 15 00:44:59 UTC 2023
On Fri, 14 Jul 2023 16:09:15 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>>
>> minor - remove duplicated code comment
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/JlinkTask.java line 666:
>
>> 664: return false;
>> 665: }
>> 666: return Files.isSameFile(javaBasePath, javaBaseInDefaultPath);
>
> Files.isSameFile checks the file contents, this doesn't look right here. Shouldn't this be
>
> return javaBasePath.toRealPath().equals(javaBaseInDefaultPath.toRealPath()) ;
>
>
> Also, just to say that !Files.exists should be Files.notExists.
Hello Alan,
> Files.isSameFile checks the file contents, this doesn't look right here.
I was always under the impression that `Files.isSameFile()` only checks file paths and attributes for testing whether they are same. The API doc of `Files.isSameFile()` too don't mention about contents and looking at the implementation in `UnixFileSystemProvider` and `ZipFileSystemProvider` (for example), then don't seem to check the contents either. There's however a mention in the API doc of `Files.isSameFile()` which says:
> and depending on the implementation, may require to open or access both files.
So I'm guessing that, that's what allows `FileSystemProvider` implementations to actually check the content of the files.
I'll change this to use `toRealPath()` as you noted.
> Also, just to say that !Files.exists should be Files.notExists.
I've overlooked Files.notExists. I'll update this as well.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1264264298
More information about the core-libs-dev
mailing list