RFR: 8356645: Javac should utilize new ZIP file system read-only access mode

Jan Lahoda jlahoda at openjdk.org
Wed Jun 11 06:04:32 UTC 2025


On Thu, 5 Jun 2025 00:03:46 GMT, Chen Liang <liach at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/file/Locations.java line 1463:
>> 
>>> 1461:                                 return null;
>>> 1462:                             }
>>> 1463:                             fs = jarFSProvider.newFileSystem(p, FSInfo.readOnlyJarFSEnv(null));
>> 
>> Happy to consider either commenting the null here (e.g. `readOnlyJarFSEnv(/* releaseVersion */ null)`), or adding a no-arg version of the method in FSInfo.
>
> This seems like a fallback path, so don't think we should add a no-arg version to promote skipping the release version.

I think just `readOnlyJarFSEnv(null)` is OK.

>> test/langtools/tools/javac/platform/VerifyCTSymClassFiles.java line 68:
>> 
>>> 66:         // Expected to always be a ZIP filesystem.
>>> 67:         try (FileSystem fs = FileSystems.newFileSystem(ctSym, FSInfo.readOnlyJarFSEnv(null))) {
>>> 68:             // Check that the file system is read only (not true if not a zip file system).
>> 
>> Another case where I'm not 100% sure if it should be an assert, or a runtime exception/error.
>
> jtreg runs tests with -ea, so either works fine.

For me: not relying on `assert` is preferred, esp. in tests. I.e. what is here currently seems good to me.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25639#discussion_r2139261006
PR Review Comment: https://git.openjdk.org/jdk/pull/25639#discussion_r2139265131


More information about the compiler-dev mailing list