mbien commented on code in PR #3860:
URL: https://github.com/apache/netbeans/pull/3860#discussion_r929788481


##########
platform/o.n.bootstrap/src/org/netbeans/JarClassLoader.java:
##########
@@ -77,6 +78,22 @@
  */
 public class JarClassLoader extends ProxyClassLoader {
     private static Stamps cache;
+    private static final Name MULTI_RELEASE = new Name("Multi-Release");
+    private static final int BASE_VERSION = 8;
+    private static final int RUNTIME_VERSION;
+
+    static {
+        int version;
+        try {
+                Object runtimeVersion = 
Runtime.class.getMethod("version").invoke(null);
+                version = (int) 
runtimeVersion.getClass().getMethod("major").invoke(runtimeVersion);
+        }
+        catch (Throwable ex) {
+                version = BASE_VERSION;
+        }
+        RUNTIME_VERSION = version;
+    }    

Review Comment:
   looks good to me but fix indentation please. It would be also good to use 
default NB formatting for brace positioning.
   
   The rest of the file does *not* use this formatting



##########
platform/o.n.bootstrap/src/org/netbeans/JarClassLoader.java:
##########
@@ -574,6 +606,17 @@ protected URL doGetResource(String name) throws 
IOException  {
         @Override
         protected byte[] readClass(String path) throws IOException {
             try {
+                if (isMultiRelease() && RUNTIME_VERSION != BASE_VERSION)
+                {
+                    int ver = RUNTIME_VERSION;
+                    while (ver > BASE_VERSION) {
+                        byte[] data = archive.getData(this, 
"META-INF/versions/" + ver + "/" + path);
+                        if (data != null) {
+                            return data;
+                        }
+                        ver--;
+                    }
+                }

Review Comment:
   This might scale badly. Lets say we load lucene which has a 8 and a 9 
version and run on JDK 19.
   
   This will call `getData()` (which isn't trivial) 10 times for each class 
before it finds something. I would put all versions contained in 
`META-INF/versions/` into an array during init and go through the array 
backwards. This would produce hits with fewer attempts on average.



##########
platform/o.n.bootstrap/test/unit/src/org/netbeans/JarClassLoaderTest.java:
##########
@@ -414,4 +425,65 @@ public static void initialize(String path, Semaphore sync) 
{
 
         public @Override void checkPermission(Permission perm, Object ctx) {}
     }
+
+    public void testMultiReleaseJar() throws Exception {
+        clearWorkDir();
+        File classes = new File(getWorkDir(), "classes");
+        classes.mkdirs();
+        ToolProvider.getSystemJavaCompiler()
+                    .getTask(null, null, d -> { throw new 
IllegalStateException(d.toString()); }, Arrays.asList("-d", 
classes.getAbsolutePath()), null,
+                             Arrays.asList(new 
SourceFileObject("test/Impl.java", "package test; public class Impl { public 
static String get() { return \"base\"; } }"),
+                                           new 
SourceFileObject("api/API.java", "package api; public class API { public static 
String run() { return test.Impl.get(); } }")))
+                    .call();
+        File classes9 = new File(new File(new File(classes, "META-INF"), 
"versions"), "9");
+        classes9.mkdirs();
+        ToolProvider.getSystemJavaCompiler()
+                    .getTask(null, null, d -> { throw new 
IllegalStateException(d.toString()); }, Arrays.asList("-d", 
classes9.getAbsolutePath(), "-classpath", classes.getAbsolutePath()), null,
+                             Arrays.asList(new 
SourceFileObject("test/Impl.java", "package test; public class Impl { public 
static String get() { return \"9\"; } }")))
+                    .call();
+        Map<String, byte[]> jarContent = new LinkedHashMap<>();
+        jarContent.put("META-INF/MANIFEST.MF", "Manifest-Version: 
1.0\nMulti-Release: true\n\n".getBytes());
+        Path classesPath = classes.toPath();
+        Files.walk(classesPath)
+             .filter(p -> Files.isRegularFile(p))
+             .forEach(p -> {
+                  try {
+                      jarContent.put(classesPath.relativize(p).toString(), 
TestFileUtils.readFileBin(p.toFile()));
+                  } catch (IOException ex) {
+                      throw new IllegalStateException(ex);
+                  }
+             });
+        File jar = new File(getWorkDir(), "multi-release.jar");
+        try (OutputStream out = new FileOutputStream(jar)) {
+            TestFileUtils.writeZipFile(out, jarContent);
+        }
+        JarClassLoader jcl = new JarClassLoader(Arrays.asList(jar), new 
ProxyClassLoader[0]);

Review Comment:
   have to agree here. Some empty lines which keep logical parts separate, like 
paragraphs in text, do a lot for readability.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists

Reply via email to