<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"Segoe UI Emoji";
panose-1:2 11 5 2 4 2 4 2 2 3;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0cm;
mso-margin-bottom-alt:auto;
margin-left:0cm;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
mso-fareast-language:EN-US;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:70.85pt 70.85pt 2.0cm 70.85pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="DE" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US">Hi,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US">I’ve amended the jdk.zipfs module documentation in src/jdk.zipfs/share/classes/module-info.java to document the new behavior (e.g. support of PosixFileAttributeView) as requested by
Alan. I’ve also updated the CSR.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Webrev: <a href="http://cr.openjdk.java.net/~clanger/webrevs/8213031.4/">
http://cr.openjdk.java.net/~clanger/webrevs/8213031.4/</a><o:p></o:p></span></p>
<p class="MsoNormal">CSR: <a href="https://bugs.openjdk.java.net/browse/JDK-8213082">
https://bugs.openjdk.java.net/browse/JDK-8213082</a><o:p></o:p></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Please review both, CSR and changeset and let me know if this is good now or what else needs to be addressed.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal">Thanks and best regards,<o:p></o:p></p>
<p class="MsoNormal">Christoph<span lang="EN-US" style="mso-fareast-language:EN-US"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US">From:</span></b><span lang="EN-US"> Volker Simonis <volker.simonis@gmail.com>
<br>
<b>Sent:</b> Freitag, 21. Dezember 2018 17:34<br>
<b>To:</b> Langer, Christoph <christoph.langer@sap.com><br>
<b>Cc:</b> Java Core Libs <core-libs-dev@openjdk.java.net>; security-dev <security-dev@openjdk.java.net>; SHEN, XUEMING <xueming.shen@oracle.com>; Alan Bateman <Alan.Bateman@oracle.com>; nio-dev <nio-dev@openjdk.java.net><br>
<b>Subject:</b> Re: RFR 8213031: (zipfs) Add support for POSIX file permissions<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<p class="MsoNormal">Hi Christoph,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">thanks for updating the change. I think it is in a good state now and ready to go!<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Also the documentation in the CSR for this issue (<a href="https://bugs.openjdk.java.net/browse/JDK-8213082">https://bugs.openjdk.java.net/browse/JDK-8213082</a>) is greatly appreciated and answers all the questions which have been raised
so far. So if there are still any open questions I'd recommend that any potential reviewer consults the CSR at
<a href="https://bugs.openjdk.java.net/browse/JDK-8213082">https://bugs.openjdk.java.net/browse/JDK-8213082</a> first.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Thank you and best regards,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Volker<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Fri, Dec 21, 2018 at 3:31 PM Langer, Christoph <<a href="mailto:christoph.langer@sap.com">christoph.langer@sap.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<p class="MsoNormal" style="margin-bottom:12.0pt">Hi all,<br>
<br>
here comes the updated webrev: <a href="http://cr.openjdk.java.net/~clanger/webrevs/8213031.3/" target="_blank">
http://cr.openjdk.java.net/~clanger/webrevs/8213031.3/</a><br>
<br>
I've rebased the change to the current state of the JDK depot. Thanks to Volker, the test has been enhanced and now also tests more copy operations (from zip file system to zip file system and from zip file system to default file system and back).<br>
<br>
The points below were also addressed:<br>
<br>
> ZipFileAttributeView.java<br>
> - can you please throw an AssertionError() for the default branch in<br>
> the switch expression in the "ZipFileAttributeView.name()".<br>
<br>
The default branch will return "basic". Looking at the code it should not be jumped to anyway.<br>
<br>
> ZipFileSystem.java<br>
> - please also put @Override annotations on the method implementations<br>
> from ZipFileAttributes (e.g. "compressedSize()", "crc()", etc...) and<br>
> a comment at the place where the implementation of the<br>
> "PosixFileAttributes" methods starts.<br>
<br>
Done, I also reordered the methods - first "basic" then "posix" then "zip".<br>
<br>
> ZipUtils.java<br>
> - just a question: isn't it possible to reuse<br>
> sun.nio.fs.UnixFileModeAttribute.toUnixMode() and the corresponding<br>
> constants from sun.nio.fs.UnixConstants instead of redefining them<br>
> here? You could export them from java.base to jdk.zipfs but the<br>
> problem is probably that at least sun.nio.fs.UnixConstants is a<br>
> generated class which only gets generated on Unix systems, right ?<br>
<br>
You've already answered yourself <span style="font-family:"Segoe UI Emoji",sans-serif">
😊</span> These classes only exist on Unix type JDKs.<br>
<br>
> ZipFileAttributes.java<br>
> - it seems a little odd that you've added "setPermissions()" to<br>
> ZipFileAttributes. All the attribute classes (i.e.<br>
> BasicFileAttributes, PosixFileAttributes) have no setters. Isn't it<br>
> possible to implement "ZipFileAttributeView.setPermissions()" by<br>
> calling "path.setPermissions()" (as this is done in<br>
> "ZipFileAttributeView.setTimes()") and handle everything in<br>
> ZipFileSyste (as this is done with "setTimes()").<br>
<br>
Good catch & thanks for providing the better implementation.<br>
<br>
<br>
I think this version is quite final now and thoroughly tested. I hope to get some valid reviews soon...<br>
<br>
Thanks<br>
Christoph<o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</body>
</html>