Hi Christoph
On Feb 17, 2020, at 11:26 AM, Langer, Christoph <christoph.langer@sap.com> wrote:
Hi Jaikiran, Lance,
I'm a bit late to the game. It occurred to me, whether we could maybe use the streams API to process the inodes. Something like this:
diff --git a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java --- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java +++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java @@ -56,6 +56,7 @@ import java.util.jar.Attributes; import java.util.jar.Manifest; import java.util.regex.Pattern; +import java.util.stream.Stream; import java.util.zip.CRC32; import java.util.zip.Deflater; import java.util.zip.DeflaterOutputStream; @@ -1730,7 +1731,17 @@ Entry e;
// write loc - for (IndexNode inode : inodes.values()) { + + // write the manifest inode (if any) first in the loc so that the + // java.util.jar.JarInputStream can find it, since it expects it to be + // the first or second entry in the jar + final IndexNode manifestInode = getInode(getBytes("/META-INF/MANIFEST.MF")); + final Stream<IndexNode> inodeStream = manifestInode == null ? + inodes.values().stream() : + Stream.concat(Stream.of(manifestInode), + inodes.values().stream().filter(node -> !manifestInode.equals(node))); + + for (IndexNode inode : (Iterable<IndexNode>)inodeStream::iterator) { if (inode instanceof Entry) { // an updated inode e = (Entry)inode; try {
What do you think?
I am not sure the use of Streams will help us too much here as in your proposed example I believe you would end out traversing the entire list of IndexNodes twice in the case of a Manifest existing. Once when building the Stream and the other when you are walking through the IndexNodes to write them out. Best Lance
I didn't have time yet to have a closer look to the testcase. Will do tomorrow.
Cheers Christoph
-----Original Message----- From: nio-dev <nio-dev-bounces@openjdk.java.net> On Behalf Of Jaikiran Pai Sent: Montag, 17. Februar 2020 03:56 To: Lance Andersen <lance.andersen@oracle.com> Cc: nio-dev@openjdk.java.net; core-libs-dev@openjdk.java.net Subject: Re: RFR: 8211917: (zipfs) Creating or updating a JAR file system should put the MANIFEST.MF at the start
Hello Lance,
On 15/02/20 2:27 am, Lance Andersen wrote:
Hi Jaikiran,
I think the changes to ZipFileSystem are OK.
The test overall is good. I am going to streamline it a bit and remove the long lines (we try to keep lines to around 80 characters).
I'll keep that in mind for future changes. Thank you for taking care of this.
-Jaikiran
<http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen@oracle.com <mailto:Lance.Andersen@oracle.com>