On Mon, 12 May 2025 10:16:33 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:
>
> Fix comment based on current behaviour.
src/jdk.zipfs/share/classes/module-info.java line 160:
> 158: * will always be opened <em>read-write</em> (see {@code
> "accessMode"}
> 159: * below), regardless of whether the underlying ZIP already
> existed or
> 160: * not.
In the default provider, "read-only && create" ignores the create option so the
open fails if the file does not exist.
For the zipfs provider then doing the same, or having this combination be an
error, is okay. I think it might be a bit too surprising to have "read-only &&
create" create a read-write file system.
src/jdk.zipfs/share/classes/module-info.java line 281:
> 279: * Even if a zip file system is writable ({@code fs.isReadOnly()
> == false}),
> 280: * this says nothing about whether individual files can be created
> or
> 281: * modified, simply that it might be possible.
Initially I thought this was to allow for POSIX file permissions but not sure
after reading it a second time. Can you say if this is what you mean?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2084997941
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2085009060
