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