RFR: 8211917: (zipfs) Creating or updating a JAR file system should put the MANIFEST.MF at the start

[Lance Andersen]

Hi Christoph

> On Feb 17, 2020, at 11:26 AM, Langer, Christoph <christoph.langer at 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 at openjdk.java.net> On Behalf Of Jaikiran
>> Pai
>> Sent: Montag, 17. Februar 2020 03:56
>> To: Lance Andersen <lance.andersen at oracle.com>
>> Cc: nio-dev at openjdk.java.net; core-libs-dev at 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 at oracle.com <mailto:Lance.Andersen at oracle.com>