[icedtea-web] RFC: Patch to check the manifest file for main class

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2012-02-27 20:01]:
> On 02/27/2012 05:42 PM, Deepak Bhole wrote:
> > Hi,
> > 
> > This commit introduced a check for main in HEAD:
> > http://icedtea.classpath.org/hg/icedtea-web/rev/bd59947fa857
> > 
> > However it causes an error if the main method is listed in the manifest
> > rather than JNLP (e.g.: http://pardemo02.ilog.fr/mapviewer/mapviewer.jnlp)
> > 
> 
> Thanks for catching this.
> 
> > 
> > diff -r 4d7bddf0d4de netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> > --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Mon Feb 27 21:58:41 2012 +0100
> > +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Mon Feb 27 17:40:45 2012 -0500
> > @@ -593,6 +593,22 @@
> >              mainClass = ad.getMainClass();
> >          } else
> >              return;
> > +
> > +        // The main class may be specified in the manifest
> > +        if (mainClass == null) {
> > +            JARDesc mainJarDesc = file.getResources().getMainJAR();
> > +            File f = CacheUtil.getCacheFile(mainJarDesc.getLocation(), null);
> 
> A couple of lines below,
> 
> tracker.getCacheFile(jars.get(i).getLocation());
> 
> is used instead of CacheUtil.getCacheFile. The javadoc of
> CacheUtil.getCacheFile notes that "This method returns the file location
> only and does not download the resource". Is there any reason to use
> CacheUtil over ResourceTracker?
>

Ah, I see. No I just copied/pasted the code from Launcher.java which does
this check. I will fix it.
 
> > +            if( f != null) {
> > +                try {
> > +                    JarFile mainJar = new JarFile(f);
> > +                    mainClass = mainJar.getManifest().
> > +                            getMainAttributes().getValue("Main-Class");
> > +                } catch (IOException ioe) {
> > +                    return;
> > +                }
> > +            }
> > +        }
> > +
> 
> Just throwing this one out there: if there is no jar marked main, should
> we try and iterate over all jars to find a jar with a Main-Class in the
> manifest file?
> 

I thought about that, but that would be ambiguous. e.g. if there are 2
jars with a Main-Class, which one do we pick then? So I figured it would
be better to only look in main jar and if it is not supplied, the app
will fail to launch.

New patch with getCacheFile attached.

Thanks,
Deepak
-------------- next part --------------
diff -r 07717ba83e5f netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
--- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Mon Feb 27 22:04:05 2012 +0100
+++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java	Tue Feb 28 09:53:43 2012 -0500
@@ -593,6 +593,20 @@
             mainClass = ad.getMainClass();
         } else
             return;
+
+        // The main class may be specified in the manifest
+        JARDesc mainJarDesc = file.getResources().getMainJAR();
+        File f = tracker.getCacheFile(mainJarDesc.getLocation());
+        if (mainClass == null && f != null) {
+            try {
+                JarFile mainJar = new JarFile(f);
+                mainClass = mainJar.getManifest().
+                            getMainAttributes().getValue("Main-Class");
+            } catch (IOException ioe) {
+                return;
+            }
+        }
+
         String desiredJarEntryName = mainClass + ".class";
 
         for (int i = 0; i < jars.size(); i++) {