On Mon, 24 Mar 2025 19:19:01 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:
> There are some more new jar FileSystems created in Locations, but those only > typically read one file, so probably not that big deal w.r.t. this flag. Agreed. I initially changed those as well but when I realized they only read a single file I reverted. > I wonder if there's some suspicion of a particular problem in the other > branch, The `then` branch, which I've modified, was introduced in e652402ed239e171a6f87f5ef88e07d505ad3fe8 (JDK-8149757: Implement Multi-Release JAR aware JavacFileManager for javac). It became necessary to ensure exact usage of `ZipFileSystemProvider` to be able to pass in the `env` map with the `multiRelease` config. I reviewed the discussion of that change. There was a mention of the introduction of this branch, but it didn't go as far to suggest removal of the `FileSystems.newFileSystem` call, it only justified the addition of the `jarFsProvider.newFileSystem` call. https://mail.openjdk.org/pipermail/compiler-dev/2016-April/thread.html > -why is JavacFileManager.getJarFSProvider() needed? Shouldn't > FileSystems.newFileSystem(realPath, env); > be enough? (Calling an SPI directly from an API client seems > suspicious to me.) If I recall correctly, the NIO2 API design forced this one. The method you are referring to does not exist. There is only one that takes a URI, and that has very different semantics. So we had to go with the method on the provider, hence the use of getJarFSProvider. The tests introduced in e652402ed239e171a6f87f5ef88e07d505ad3fe8 don't appear to exercise the else branch. It appears there are two ways to get there: 1. Direct, programatic use of the a file manager in which the initialization sequence does not populate `multiReleaseValue` 2. Use of javac with a `.zip` as a classpath entry (I had to check, but this does work.) Side question -- what is the expected behaviour with a classpath entry with a non-lowercase extension, `.JAR`? It is allowed by `javac` but would not respect the `multi-release` setting. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24176#discussion_r2011047234
