[icedtea-web] RFC: find jars based on Class-Path entry in manifest

[Deepak Bhole]

* Omair Majid <omajid at redhat.com> [2011-04-20 15:18]:
> On 04/20/2011 03:15 PM, Deepak Bhole wrote:
> >* Omair Majid<omajid at redhat.com>  [2011-04-20 15:09]:
> >>On 04/20/2011 02:45 PM, Deepak Bhole wrote:
> >>>* Omair Majid<omajid at redhat.com>   [2011-04-15 12:28]:
> >>>>Updated patch attached.
> >>>>
> >>>
> >>>I'll let Andrew reply to his concerns. I have only one concern (below):
> >>>
> >>>>-
> >>>>-                            try {
> >>>>-                                u = tracker.getCacheURL(remoteURL);
> >>>>+                                addNewJar(desc);
> >>>>                              } catch (Exception e) {
> >>>>                                  throw new ClassNotFoundException(name);
> >>>>                              }
> >>>>-
> >>>
> >>>We are catching any exception and re-throwing a CNFE. However exceptions
> >>>could occur due to something like a file mentioned in the jarindex not
> >>>existing on the server. This may not necessarily be fatal to the app
> >>>(the class may exist in the next jar specified in the index). The CNFE
> >>>will however make it fatal..
> >>>
> >>
> >>That's what I thought when I was going over this code - but that's
> >>how the old code was written. You can see that the new section that
> >>handles classpaths does not throw CNFE - it just logs the exception
> >>and continues.
> >>
> >>FWIW, addNewJar() and getCacheURL() do not throw any checked
> >>exceptions. I believe this is more of a safety net than anything
> >>else. Do you want me to update this code to just log the exception?
> >>
> >
> >Yes, I think that will be better. If the current code is throwing
> >exceptions in such instances, it is incorrect IMO.
> >
> 
> Sure, I will post an updated patch in a bit.
> 
> >>On a slightly different note, this code needs to be moved to the
> >>findResource(s) methods. As it is, searching for anything other than
> >>classes will not use Class-Path jars or the JarIndexes.
> >>
> >
> >Good point. Would you be doing that? If so, do you want to do it in this
> >patch or a new one?
> >
> 
> I can take care of it, but I would like to do it in a separate
> patch. I dont think this is important enough to block 1.1 branching.
> 

Fair enough. Please post an updated patch with just the exception change
then and I will approve it.

Thanks,
Deepak

> Cheers,
> Omair