Hi Alan,

I've got a new iteration for the zipfs POSIX support and addressed your 
concerns: http://cr.openjdk.java.net/~clanger/webrevs/8213031.9/

The new change bases on the proposal for JDK-8222276, which I created to factor 
out changes unrelated to POSIX to facilitate easier reviewing of this patch.

In details:

> One thing that is puzzling is that
> ZipFileAttributeView/ZipFileAttributes extend
> PosixFileAttributeView/PosixFileAttributes. I don't think that will work
> because the "zip" view is supported by default whereas "posix" view
> needs the file system to be created with enablePosixFileAttributes=true.

I separated ZipFileAttributeView and ZipFileAttributeViewPosix. I also created 
a class EntryPosix inside ZipFileSystem.java that extends the default Entry 
class and additionally implements PosixFileAttributes. It is used, depending on 
whether Posix support is turned on or not. I unfortunately had to touch all the 
places where new Entry objects are created and either create a Posix entry or a 
default Entry.


> I saw your comment about naming the file permissions attribute
> "storedPermissions" in the zip view but if the zip and posix view are
> separated then I think it would be clearer to name it "permissions" in
> the zip view. If code is using Files.getAttribute then it will need the
> view name so they won't get mixed up.

It's always "permissions" now. In case, it is a default ZipFileSystem, the 
attribute is optional. If supportPosix is true, it'll always be available using 
default values.

> You asked about the fallback when defaultOwner/defaultGroup aren't set.
> If getOwner fails with an IOException then I think that exception should
> be propagated. The UOE case will fallback to the value of "user.name". I
> think the fallback for the group owner should be the file owner rather
> than "
> "<zipfs_default>". The default.policy file will need to be updated to
> grant jdk.zipfs RuntimePermission("accessUserInfo") so that you won't
> need to deal with security exceptions.

Ok, done that 😊

> As regards next steps then I think we need to agree the spec changes, as
> in the the javadoc in module-info.java. Once we have the spec agreed
> then the CSR can be updated and finalized. The code review can be done
> in parallel of course. I think Lance is going to help review the changes.

I have updated module-info a little bit to reflect the latest changes. Is it 
now time to focus on the CSR?

Thanks
Christoph

Reply via email to