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