Hi Steve, do you have checked that the your patch doesn't load a bunch of supplementary classes at start-up ? because JarFile is used at start-up, it triggers the load of Runtime.Version and Runtime.Version has a dependency on a dozen of classes in java.util.regex.
cheers, Rémi ----- Mail original ----- > De: "Steve Drach" <steve.dr...@oracle.com> > À: "Joseph D. Darcy" <joe.da...@oracle.com> > Cc: "Core-Libs-Dev" <core-libs-dev@openjdk.java.net> > Envoyé: Jeudi 16 Juin 2016 23:44:14 > Objet: [SPAM-RENATER]Re: RFR: 8150680 JarFile.Release enum needs > reconsideration with respect to it's values > > This webrev uses methods instead of fields to return the base and runtime > values used internally by JarFile. I’ve also optimized it a bit. > > http://cr.openjdk.java.net/~sdrach/8150680/webrev.02/ > <http://cr.openjdk.java.net/~sdrach/8150680/webrev.02/> > > > On Jun 15, 2016, at 4:31 PM, Joseph D. Darcy <joe.da...@oracle.com> wrote: > > > > Steve, > > > > In JarFile, please use methods not fields to return the new information. > > The information in question is not constant across versions. Using methods > > instead of fields avoid over-committing on a particular implementation, > > etc. > > > > Cheers, > > > > -Joe > > > > On 6/15/2016 3:49 PM, Steve Drach wrote: > >> I’ve updated the webrev to address the issue of the constructor accepting > >> values like Version.parse(“7.1”) > >> > >> http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/ > >> <http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/> > >> > >>> On Jun 15, 2016, at 8:56 AM, Steve Drach <steve.dr...@oracle.com> wrote: > >>> > >>>>> Please review the following changeset: > >>>>> > >>>>> webrev: http://cr.openjdk.java.net/~sdrach/8150680/webrev.00/index.html > >>>>> <http://cr.openjdk.java.net/~sdrach/8150680/webrev.00/index.html> > >>>>> issue: https://bugs.openjdk.java.net/browse/JDK-8150680 > >>>>> <https://bugs.openjdk.java.net/browse/JDK-8150680> > >>>>> > >>>>> The issue calls for reconsidering the JarFile.Release enum. A comment > >>>>> in the bug report suggests replacing JarFile.Release with > >>>>> Runtime.Version, and that’s what I did. Specifically I removed the > >>>>> enum, changed the constructor to accept a Runtime.Version object > >>>>> instead of a JarFile.Release object, updated all places in the JDK > >>>>> that invoked the constructor and updated all tests. > >>>>> > >>>> Moving to Runtime.Version seems right but doesn't the javadoc for the > >>>> constructor need to be updated to make it clear how it behavior when > >>>> invoking with something like Version.parse("7.1") ? If I read the code > >>>> correctly then this will be accepted and getVersion() will return 7.1. > >>> Yes, it needs to be updated and it needs to be fixed. Thanks for finding > >>> that. > >>> > >>>> Fields or methods is another discussion point for the base and runtime > >>>> versions. > >>> My thinking is, in this case fields and methods are equivalent, the > >>> method not giving any more flexibility than a field. For example the > >>> method JarFile.baseVersion will just return the value contained in the > >>> private final static field BASE_VERSION. Or the public final static > >>> field BASE_VERSION can be directly accessed. I see no advantage of a > >>> method here. But I’m willing to be enlightened. > >>> > >>>> -Alan. > > > >
