RFR 8144355: JDK 9 changes to ZipFileSystem to support multi-release jar files

[Steve Drach]

Hi,

I updated the patch to the changeset to address Paul’s concern with a windows specific solution, as well some of Alan’s concerns.  Please let me know if this patch (instead of the previous one) can be applied to the changeset.

Thanks
Steve

diff --git a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java
--- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java
+++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java
@@ -37,8 +37,12 @@
 import java.util.jar.Attributes;
 import java.util.jar.Manifest;
 
-/*
- * Adds aliasing to ZipFileSystem to support multi-release jar files
+/**
+ * Adds aliasing to ZipFileSystem to support multi-release jar files.  An alias map
+ * is created by {@link JarFileSystem#createVersionedLinks(int)}.  The map is then
+ * consulted when an entry is looked up in {@link JarFileSystem#getEntry0(byte[])}
+ * to determine if the entry has a corresponding versioned entry.  If so, the
+ * versioned entry is returned.
  *
  * @author Steve Drach
  */
@@ -55,7 +59,8 @@
 
     JarFileSystem(ZipFileSystemProvider provider, Path zfpath, Map<String,?> env) throws IOException {
         super(provider, zfpath, env);
-        lookup = path -> path;
+        lookup = path -> path;  // lookup needs to be set before isMultiReleaseJar is called because
+                                // it eventually calls getEntry0
         if (isMultiReleaseJar()) {
             int version;
             Object o = env.get("multi-release");
@@ -87,7 +92,7 @@
         }
     }
 
-    /*
+    /**
      * create a map of aliases for versioned entries, for example:
      *   version/PackagePrivate.class -> META-INF/versions/9/version/PackagePrivate.class
      *   version/PackagePrivate.java -> META-INF/versions/9/version/PackagePrivate.java
@@ -117,7 +122,7 @@
         return path -> aliasMap.get(IndexNode.keyOf(path));
     }
 
-    /*
+    /**
      * create a sorted version map of version -> inode, for inodes <= max version
      *   9 -> META-INF/versions/9
      *  10 -> META-INF/versions/10
@@ -135,7 +140,7 @@
         return map;
     }
 
-    /*
+    /**
      * extract the integer version number -- META-INF/versions/9 returns 9
      */
     private Integer getVersion(byte[] name, int offset) {
@@ -147,7 +152,7 @@
         }
     }
 
-    /*
+    /**
      * walk the IndexNode tree processing all leaf nodes
      */
     private void walk(IndexNode inode, Consumer<IndexNode> process) {
@@ -160,7 +165,7 @@
         }
     }
 
-    /*
+    /**
      * extract the root name from a versioned entry name
      *   given inode for META-INF/versions/9/foo/bar.class
      *   and prefix META-INF/versions/9/
diff --git a/test/jdk/nio/zipfs/MultiReleaseJarTest.java b/test/jdk/nio/zipfs/MultiReleaseJarTest.java
--- a/test/jdk/nio/zipfs/MultiReleaseJarTest.java
+++ b/test/jdk/nio/zipfs/MultiReleaseJarTest.java
@@ -65,9 +65,12 @@
         creator.buildUnversionedJar();
         creator.buildMultiReleaseJar();
         creator.buildShortMultiReleaseJar();
-        uvuri = new URI("jar:file:" + userdir + "/unversioned.jar");
-        mruri = new URI("jar:file:" + userdir + "/multi-release.jar");
-        smruri = new URI("jar:file:" + userdir + "/short-multi-release.jar");
+        String ssp = Paths.get(userdir, "unversioned.jar").toUri().toString();
+        uvuri = new URI("jar", ssp , null);
+        ssp = Paths.get(userdir, "multi-release.jar").toUri().toString();
+        mruri = new URI("jar", ssp, null);
+        ssp = Paths.get(userdir, "short-multi-release.jar").toUri().toString();
+        smruri = new URI("jar", ssp, null);
         entryName = className.replace('.', '/') + ".class";
     }



> On Dec 18, 2015, at 12:28 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
> 
> HI Steve,
> 
> I find the use of the system property awkward.
> 
> Did you try doing:
> 
>  new File(...).toURI()
> 
> ?
> 
> I noticed you are already creating Path instances for deletion, so it might be even better to do Path.toURI() e.g.:
> 
>  uvuri = “jar:” + Paths.get(userdir, "unversioned.jar”).toURI();
> 
> rather than:
> 
>  uvuri = “jar:” + new FIle(userdir, "unversioned.jar”).toURI();
> 
> Paul.
> 
>> On 18 Dec 2015, at 03:20, Steve Drach <steve.drach at oracle.com> wrote:
>> 
>> Hi,
>> 
>> jprt found a bug in one of my tests when run on windows.  The fix is simple as you can see in the patch below.  I also took the opportunity to add a comment to a line in the source code.  Please let me know if these patches can be added to the changeset.
>> 
>> Thanks
>> Steve
>> 
>> diff --git a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java
>> --- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java
>> +++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java
>> @@ -55,7 +55,8 @@
>> 
>>     JarFileSystem(ZipFileSystemProvider provider, Path zfpath, Map<String,?> env) throws IOException {
>>         super(provider, zfpath, env);
>> -        lookup = path -> path;
>> +        lookup = path -> path;  // lookup needs to be set before isMultiReleaseJar is called because
>> +                                // it eventually calls getEntry0
>>         if (isMultiReleaseJar()) {
>>             int version;
>>             Object o = env.get("multi-release");
>> diff --git a/test/jdk/nio/zipfs/MultiReleaseJarTest.java b/test/jdk/nio/zipfs/MultiReleaseJarTest.java
>> --- a/test/jdk/nio/zipfs/MultiReleaseJarTest.java
>> +++ b/test/jdk/nio/zipfs/MultiReleaseJarTest.java
>> @@ -65,9 +65,15 @@
>>         creator.buildUnversionedJar();
>>         creator.buildMultiReleaseJar();
>>         creator.buildShortMultiReleaseJar();
>> -        uvuri = new URI("jar:file:" + userdir + "/unversioned.jar");
>> -        mruri = new URI("jar:file:" + userdir + "/multi-release.jar");
>> -        smruri = new URI("jar:file:" + userdir + "/short-multi-release.jar");
>> +        String udir;
>> +        if (System.getProperty("os.name").startsWith("Windows")) {
>> +            udir = "/" + userdir.replace('\\','/');
>> +        } else {
>> +            udir = userdir;
>> +        }
>> +        uvuri = new URI("jar:file:" + udir + "/unversioned.jar");
>> +        mruri = new URI("jar:file:" + udir + "/multi-release.jar");
>> +        smruri = new URI("jar:file:" + udir + "/short-multi-release.jar");
>>         entryName = className.replace('.', '/') + ".class";
>>     }
>> 
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8144355
>>> 
>>> Change to modules.xml: http://cr.openjdk.java.net/~sdrach/8144355/top/webrev.01/index.html
>>> 
>>> Changes to ZipFileSystem: http://cr.openjdk.java.net/~sdrach/8144355/jdk/webrev.01/index.html
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20151218/4d4d8ee0/attachment.html>