Hi, here is a little update to my latest webrev: http://cr.openjdk.java.net/~clanger/webrevs/8213031.11/
I had to make modifications to fix test errors and enhance testing: a) in initOwner/initGroup the UOE has to be catched and handled explicitly as it is never wrapped into PAE because it is a RuntimeException. b) updated the test to expect the zip file's group as default group c) added a sanity check to read and extract a zip file created by zipfs with posix permissions set via java.util.zip.ZipFile Best regards Christoph > -----Original Message----- > From: Langer, Christoph > Sent: Dienstag, 21. Mai 2019 17:24 > To: Alan Bateman <alan.bate...@oracle.com> > Cc: nio-dev <nio-...@openjdk.java.net>; Java Core Libs <core-libs- > d...@openjdk.java.net> > Subject: RE: RFR 8213031: (zipfs) Add support for POSIX file permissions > > Hi Alan, > > Thank you for your comments. Here comes the next update... increasing the > turnaround time a little bit 😊 > > http://cr.openjdk.java.net/~clanger/webrevs/8213031.10/ > > > > Thanks. I think you've addressed most of my points. The only thing that > > isn't clear is the group owner as I thought we had converged on using > > the zip file's group owner. If I read the code correctly then it uses > > the file owner (and makes the assumption that defaultOwner is > > initialized before initGroup is called). > > Ok, makes sense. I've updated the coding such that the zip's file owner > would be the default owner, in case it can be retrieved. > > > In passing, the name of the PosixFileAttributeView implementation should > > probably be ZipPosixFileAttributeView rather than > > ZipFilePosixAttributeViewPosix. Also EntryPosix extends Entry, should > > probably be PosixEntry. Not important as these are internal but noticed > > them when scanning the changes. > > I changed the class names, hopefully to your liking. > > > Also in passing, there are several places using > > AccessController.doPrivileged that are bit ugly due to the casting. The > > doPrivileged methods are awkward to use with lambda expressions (not > > your doing) but one approach that I find readable is to separate the > > creation of the action, e.g. file the zip file owner it could be: > > > > PrivilegedExceptionAction<UserPrincipal> pa = () -> > Files.getOwner(zfpath); > > return AccessController.doPrivileged(pa). > > I updated these places. Seems more readable indeed. > > > In the same area, the code in initOwner catches UOE but that will always > > be wrapped in PAE. > > Fixed. > > > > I have updated module-info a little bit to reflect the latest changes. Is > > > it > > now time to focus on the CSR? > > > > > The "For extended POSIX support ..." paragraph has the property name > > from a previous iteration so I assume you'll update that. I think it > > would be using to put quotes around the names too. It also specifies the > > fallback group owner to be the file owner but I think we converged on > > use the zip file's group owner where possible. > > > > A small bit of word smiting required but I think this is really close to > > writing the CSR. > > I have updated the doc in module-info.java quite a bit. Please check. > > Is it time to work on the CSR now? How shall we proceed there? > > Thanks > Christoph
