On Mon, 19 May 2025 15:18:12 GMT, David Beaumont <d...@openjdk.org> wrote:
>> Adding read-only support to ZipFileSystem.
>>
>> The new `accessMode` environment property allows for readOnly and readWrite
>> values, and ensures that the requested mode is consistent with what's
>> returned.
>>
>> This involved a little refactoring to ensure that "read only" state was set
>> initially and only unset at the end of initialization if appropriate.
>>
>> By making 2 methods return values (rather than silently set non-final fields
>> as a side effect) it's now clear in what order fields are initialized and
>> which are final (sadly there are still non-final fields, but only a split of
>> this class into two types can fix that, since determining multi-jar support
>> requires reading the file system).
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Don't use JUnit utils in TestNg.
src/jdk.zipfs/share/classes/module-info.java line 288:
> 286: * isReadOnly()} will always return {@code true}. Creating
> a
> 287: * <em>read-only</em> file system requires the underlying
> ZIP file to
> 288: * already exist, and is incompatible with {@code
> "create"=true}.
I think it would be clearer to drop "and is incompatible with {@code
"create"=true}". The sentence that follows makes it clear that
accessMode=readWrite && create=true cause IAE to be thrown.
src/jdk.zipfs/share/classes/module-info.java line 291:
> 289: * Specifying {@code create} as {@code true} and {@code
> accessMode} as
> 290: * {@code readOnly} will cause an {@code
> IllegalArgumentException}
> 291: * to be thrown when the ZIP filesystem is created.
"created" sounds past tense, so making it "when creating the ZIP file system"
would work better here. Same comment on the text further down.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2096133967
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2096134802
