On Wed, 14 Feb 2024 11:40:16 GMT, Alan Bateman <[email protected]> wrote:
>> Please review this PR that makes the launcher helper keep a reference to the
>> executable JAR file active after extracting the name of the main class and
>> returning it as Class instance. Now, when loading classes from the JAR file,
>> it hasn't to be re-opened.
>
> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 593:
>
>> 591: }
>> 592:
>> 593: private static String getMainClassFromJar(String jarname, JarFile
>> jarFile) {
>
> Can you check if the "jarname" parameter can be dropped, the error handling
> in this method should be able to use jarFile.getName().
`String jarname` is filled by the caller with the value of `String what`, and
therefore contains the entire path to the executable JAR file. Using only
`jarFile.getName()` will prevent any parent directories from being noted in
error messages; which might affect some tests
> src/java.base/share/classes/sun/launcher/LauncherHelper.java line 823:
>
>> 821: // get the class name
>> 822: String cn;
>> 823: // store the jar file
>
> "store the jar file" is a confusing comment to add. I think what you want to
> say is that the JarFile will put the underlying file in the JarFile/ZipFile
> cache and this will avoid needing to re-parse the manifest when the JAR file
> is opened on the class path, triggered by Class.forName.
Updating to use the precise description.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17843#discussion_r1489688919
PR Review Comment: https://git.openjdk.org/jdk/pull/17843#discussion_r1489689891
