RFR 8229888: Zip FS does not retain permissions when updating a Zip file

[Brian Burkhalter]

Hi Lance,

No further comments beyond what Christoph wrote aside from noticing that the GROUP_EXECUTE permission is not tested so perhaps you might want to add it somewhere in the DataProvider in the test.

Brian

> On Dec 17, 2019, at 12:24 AM, Langer, Christoph <christoph.langer at sap.com> wrote:
> 
> I checked your patch. I think it is correct, I only have a few style nits which you may or may not address (no need for further webrev, though):
>  
> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java:
> Line 1793: Maybe the comment should rather be: // Set the POSIX permissions of the original Zip File if available
> Line 1804: The path to the file (need not necessarily be a zip file)
> Line 1806: I would indent the comment to the same column as “The” in the row above – makes it more readable. Also, there is a space too much between “the” and “POSIX”
> Line 1808: Indention as above
> Line 1811: Remove
> Line 1816: Insert blank between if and (
>  
> test/jdk/jdk/nio/zipfs/ZipFSPermissionsTest.java:
> Line 68: Remove
> Line 85: Remove
> Method teardown, line 97: I think this method is not needed. zipFile is already deleted before each test in method “before()” and in the end you clean up via “suiteCleanUp()”. So maybe tearDown can/should be removed?
> Line 113: Maybe you should move “Zip file” to the line before while keeping this line as a blank line
> Line 115: Same as with 113
> Line 120: Space between if and (
> Line 181: Same as in 120
> Method getPosixAttributes, line 199: I think you should remove the try/catch UnsupportedOperationException. If the test is run on a non-posix fs, the whole test is skipped in setup() anyways.
>  
> Cheers
> Christoph
>  
>  
> From: nio-dev <nio-dev-bounces at openjdk.java.net <mailto:nio-dev-bounces at openjdk.java.net>> On Behalf Of Lance Andersen
> Sent: Donnerstag, 12. Dezember 2019 17:48
> Cc: nio-dev <nio-dev at openjdk.java.net <mailto:nio-dev at openjdk.java.net>>
> Subject: Re: RFR 8229888: Zip FS does not retain permissions when updating a Zip file
>  
> Attached is the updated patch for addressing the permissions based on feedback from Alan:  http://cr.openjdk.java.net/~lancea/8229888/webrev.02/index.html <http://cr.openjdk.java.net/~lancea/8229888/webrev.02/index.html>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20191217/36dc026a/attachment.htm>