On Fri, 17 Oct 2025 10:07:37 GMT, Alan Bateman <[email protected]> wrote:
>>> @AlanBateman are you ok with letting the original >>> [c6f1d5f](https://github.com/openjdk/jdk/commit/c6f1d5f374bfa9bde75765391d5dae0e8e28b4ab) >>> reviewers know of this fix and take a look? Or do you think further >>> discussion is needed somewhere else? >> >> Have you had time to try using the file key to detect the recursive include >> case? > >> Hi @AlanBateman, I'm resuming the work on this one. >> >> > Have you had time to try using the file key to detect the recursive >> > include case? >> >> By "file key", do you mean `Files.readAttributes(Path.of("..."), >> BasicFileAttributes.class).fileKey()`? This works on _Linux_ but returns >> `null` on _Windows_. > > Can you try Files.isSameFile? It will use the file key on all platforms. If > the file located by the included path, and the previous files encountered, > are accessible then it can be loadFromPath.. @AlanBateman: `Files::isSameFile` works well for circular includes detection (in both _Linux_ and _Windows_). I've updated the code to use it (see a8d865c4985e6660655b27df28e76882855b2087). The new code also avoids resolving a path, except in the following cases: * When we need to resolve a relative include * For clarity to the user, in logging messages * For clarity to the user, in exception messages In those cases, I kept `File::getCanonicalPath` as the resolution method, because is the only one that is able to handle the 7abb62c069ad35f4ec34f6cd9b9f6d05febceecc variant. @seanjmullan: the new code avoids path resolution whenever possible (except logging and exception messages), so it represents the most conservative approach (except for any possible regression of the new `Files::isSameFile` usage). Please give it a look (or let Weijun/Valerie know). ------------- PR Comment: https://git.openjdk.org/jdk/pull/24465#issuecomment-3470640775
