On Sat, 15 Jul 2023 00:41:12 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> 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. Done. The PR has been updated with these changes. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/11943#discussion_r1264265346
