Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Steve Drach]

I’ve updated the webrev to incorporate the changes suggested by Oleg Barbashov. 
 This changeset has a couple of minor changes to the tests and I’ve changed the 
Validator class to be an instance of Consumer.

http://cr.openjdk.java.net/~sdrach/8158295/webrev.03/index.html 



> On Jul 18, 2016, at 6:33 PM, Steve Drach  wrote:
> 
>>> Please review the following:
>>> 
>>> webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 
>>> 
>>> issue: https://bugs.openjdk.java.net/browse/JDK-8158295 
>>> 
>>> 
>>> This changeset adds a multi-release jar validator to jar tool.  After the 
>>> jar tool builds a multi-release jar, the potential resultant jar file is 
>>> passed to the validator to assure that the jar file meets the minimal 
>>> standards of a multi-release jar, in particular that versioned classes have 
>>> the same api as base classes they override.  There are other checks, for 
>>> example warning if two classes are identical.  If the jar file is invalid, 
>>> it is not kept, so —create will not produce a jar file and —update will 
>>> keep the input jar file.
> 
> I’ve updated the webrev to address the issues raised — 
> http://cr.openjdk.java.net/~sdrach/8158295/webrev.02/index.html 
> 
> 
>> 
>> jar/Main.java
>> 
>> 284 // This code would be repeated in the create and update 
>> sections.
>> 285 // In order not to do that, it's here as a consumer.
>> 286 Consumer validateAndClose = tmpfile -> {
>> Why does it need to be Consumer rather than just a method?
> 
> I changed it to a method.
> 
>> 
>> Then i think you don’t need to rethow, but you can still keep the try block 
>> and use finally for File.deleteIfExist since it will not complain for the 
>> case where you move.
> 
> Done.
> 
>> 
>> 
>> 558 int i1 = s1.indexOf('/', n);
>> 559 int i2 = s1.indexOf('/', n);
>> 560 if (i1 == -1) throw new NumberFormatException(s1);
>> 561 if (i2 == -2) throw new NumberFormatException(s2);
>> 
>> I think you are trying to reject entries directly under the versioned area 
>> so it’s not about numbers (same in the validator, so there is some 
>> redundancy?).
> 
> A couple things here.  The most obvious is it should be “if (i2 == -1)”.  I 
> replaced NumberFormatException with a new InvalidJarException.  I added a 
> comment that
> this code is used to sort the version numbers as string representations of 
> numbers, therefore “9” goes before “10”, not usual for string sorts.
> 
>> 
>> 
>> 588 AtomicBoolean validHolder = new AtomicBoolean();
>> 589 validHolder.set(valid);
>> 
>> Pass valid to the constructor
> 
> Done.
> 
>> 
>> 
>> Validator/FingerPrint.java
>> 
>> It would be useful to have some comments on what is checked in terms of API 
>> compatibility. e.g. describe what FingerPrint collects and how Validator 
>> uses it.
> 
> Added some commentary to FingerPrint.
> 
>> 
>> 
>> FingerPrint.java
>> 
>> 205 private final Map fields = new HashMap<>();
>> 206 private final Map methods = new HashMap<>();
>> 
>> Not sure you need to use a Map, why not use a Set?
> 
> Not sure why I did it with Maps instead of Sets, but I wanted to keep the 
> method and field names and maybe that made sense at one time.  It doesn’t 
> now, so Sets they are.
> 
>> 
>> Change Method to be literally a representation of a single method rather 
>> than a consolidation that is lossy for the set of exceptions.
> 
> Done.
> 
> 

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Steve Drach]

> On Jul 28, 2016, at 11:59 AM, Oleg G. Barbashov  
> wrote:
> 
> 27.07.2016 15:34, Oleg G. Barbashov пишет:
>> 
>> 07.07.2016 23:32, Steve Drach пишет:
>>> Hi,
>>> 
>>> Please review the following:
>>> 
>>> webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 
>>> 
>>> issue: https://bugs.openjdk.java.net/browse/JDK-8158295 
>>> 
>>> 
>>> This changeset adds a multi-release jar validator to jar tool. After the 
>>> jar tool builds a multi-release jar, the potential resultant jar file is 
>>> passed to the validator to assure that the jar file meets the minimal 
>>> standards of a multi-release jar, in particular that versioned classes have 
>>> the same api as base classes they override.  There are other checks, for 
>>> example warning if two classes are identical.  If the jar file is invalid, 
>>> it is not kept, so —create will not produce a jar file and —update will 
>>> keep the input jar file.
>>> 
>>> Thanks
>>> Steve
>> 
>> 574 private boolean validate(String fname) {
>> 575 boolean valid = true;
>> 576 Validator validator = new Validator(this);
>> 577
>> 578 try (JarFile jf = new JarFile(fname)) {
>> 579 AtomicBoolean validHolder = new AtomicBoolean(valid);
>> 580 jf.stream()
>> 581 .filter(e -> !e.isDirectory())
>> 582 .filter(e -> !e.getName().equals(MANIFEST_NAME))
>> 583 .filter(e -> !e.getName().endsWith(MODULE_INFO))
>> 584 .sorted(entryComparator)
>> 585 .forEachOrdered(je -> {
>> 586 boolean b = validator.accept(je, jf);
>> 587 if (validHolder.get()) validHolder.set(b);
>> 588 });
>> 589 valid = validHolder.get();
>> 590 } catch (IOException e) {
>> 591 error(formatMsg2("error.validator.jarfile.exception", fname, 
>> e.getMessage()));
>> 592 valid = false;
>> 593 } catch (InvalidJarException e) {
>> 594 error(formatMsg("error.validator.bad.entry.name", 
>> e.getMessage()));
>> 595 valid = false;
>> 596 }
>> 597 return valid;
>> 598 }
>> 
>> (IMHO) Using of AtomicBoolean and forEachOrdered() for stateful validator 
>> here looks forced. It may be avoided by using regular iterator with "for" 
>> loop:
>> 
>>Stream sorted = jf.stream()
>>.filter(e -> !e.isDirectory())
>>.filter(e -> !e.getName().equals(MANIFEST_NAME))
>>.filter(e -> !e.getName().endsWith(MODULE_INFO))
>>.sorted(entryComparator);
>>for (Iterator iter = sorted.iterator(); 
>> iter.hasNext();) {
>>if (!validator.accept(iter.next(), jf)) {
>>valid = false;
>>}
>>}
>> 
>> or even better by storing "valid" state inside the Validator after turning 
>> it to regular Consumer:
>> 
>>try (JarFile jf = new JarFile(fname)) {
>>Validator validator = new Validator(this, jf);
>>jf.stream()
>>.filter(e -> !e.isDirectory())
>>.filter(e -> !e.getName().equals(MANIFEST_NAME))
>>.filter(e -> !e.getName().endsWith(MODULE_INFO))
>>.sorted(entryComparator)
>>.forEachOrdered(validator);
>>return validator.isValidated();
>>} catch (IOException e) {
>>error(formatMsg2("error.validator.jarfile.exception", fname, 
>> e.getMessage()));
>>return false;
>>} catch (NumberFormatException e) {
>>error(formatMsg("error.validator.bad.entry.name", 
>> e.getMessage()));
>>return false;
>>}
>> 
>> Moreover what is the reason to have an external loop over the jar's entries? 
>> Even if we will use several different filter sets in future it would be 
>> better to use it as optional parameter for Validator.
>> 
> I mean that the jar file entries iteration code ("580 
> jf.stream().filter(e ")...588 }); ) 
> processing should be a part of "Validator”.

Yes, I know what you meant.  I think it’s fine where it is, but I do like your 
suggestion to make Validator a Consumer and I’ll be releasing an updated webrev 
soon, with that change and the changes you recommended for the tests.  Thanks 
for the ideas!

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Oleg G. Barbashov]

27.07.2016 15:34, Oleg G. Barbashov пишет:


07.07.2016 23:32, Steve Drach пишет:

Hi,

Please review the following:

webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 

issue: https://bugs.openjdk.java.net/browse/JDK-8158295 



This changeset adds a multi-release jar validator to jar tool. After 
the jar tool builds a multi-release jar, the potential resultant jar 
file is passed to the validator to assure that the jar file meets the 
minimal standards of a multi-release jar, in particular that 
versioned classes have the same api as base classes they override.  
There are other checks, for example warning if two classes are 
identical.  If the jar file is invalid, it is not kept, so —create 
will not produce a jar file and —update will keep the input jar file.


Thanks
Steve


574 private boolean validate(String fname) {
 575 boolean valid = true;
 576 Validator validator = new Validator(this);
 577
 578 try (JarFile jf = new JarFile(fname)) {
 579 AtomicBoolean validHolder = new AtomicBoolean(valid);
 580 jf.stream()
 581 .filter(e -> !e.isDirectory())
 582 .filter(e -> !e.getName().equals(MANIFEST_NAME))
 583 .filter(e -> !e.getName().endsWith(MODULE_INFO))
 584 .sorted(entryComparator)
 585 .forEachOrdered(je -> {
 586 boolean b = validator.accept(je, jf);
 587 if (validHolder.get()) validHolder.set(b);
 588 });
 589 valid = validHolder.get();
 590 } catch (IOException e) {
 591 error(formatMsg2("error.validator.jarfile.exception", fname, 
e.getMessage()));

 592 valid = false;
 593 } catch (InvalidJarException e) {
 594 error(formatMsg("error.validator.bad.entry.name", 
e.getMessage()));

 595 valid = false;
 596 }
 597 return valid;
 598 }

(IMHO) Using of AtomicBoolean and forEachOrdered() for stateful 
validator here looks forced. It may be avoided by using regular 
iterator with "for" loop:


Stream sorted = jf.stream()
.filter(e -> !e.isDirectory())
.filter(e -> !e.getName().equals(MANIFEST_NAME))
.filter(e -> !e.getName().endsWith(MODULE_INFO))
.sorted(entryComparator);
for (Iterator iter = sorted.iterator(); 
iter.hasNext();) {

if (!validator.accept(iter.next(), jf)) {
valid = false;
}
}

or even better by storing "valid" state inside the Validator after 
turning it to regular Consumer:


try (JarFile jf = new JarFile(fname)) {
Validator validator = new Validator(this, jf);
jf.stream()
.filter(e -> !e.isDirectory())
.filter(e -> !e.getName().equals(MANIFEST_NAME))
.filter(e -> !e.getName().endsWith(MODULE_INFO))
.sorted(entryComparator)
.forEachOrdered(validator);
return validator.isValidated();
} catch (IOException e) {
error(formatMsg2("error.validator.jarfile.exception", 
fname, e.getMessage()));

return false;
} catch (NumberFormatException e) {
error(formatMsg("error.validator.bad.entry.name", 
e.getMessage()));

return false;
}

Moreover what is the reason to have an external loop over the jar's 
entries? Even if we will use several different filter sets in future 
it would be better to use it as optional parameter for Validator.


I mean that the jar file entries iteration code ("580 
jf.stream().filter(e ")...588 }); ) 
processing should be a part of "Validator".

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Oleg G. Barbashov]

07.07.2016 23:32, Steve Drach пишет:

Hi,

Please review the following:

webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 

issue: https://bugs.openjdk.java.net/browse/JDK-8158295 


This changeset adds a multi-release jar validator to jar tool.  After the jar 
tool builds a multi-release jar, the potential resultant jar file is passed to 
the validator to assure that the jar file meets the minimal standards of a 
multi-release jar, in particular that versioned classes have the same api as 
base classes they override.  There are other checks, for example warning if two 
classes are identical.  If the jar file is invalid, it is not kept, so —create 
will not produce a jar file and —update will keep the input jar file.

Thanks
Steve


574 private boolean validate(String fname) {
 575 boolean valid = true;
 576 Validator validator = new Validator(this);
 577
 578 try (JarFile jf = new JarFile(fname)) {
 579 AtomicBoolean validHolder = new AtomicBoolean(valid);
 580 jf.stream()
 581 .filter(e -> !e.isDirectory())
 582 .filter(e -> !e.getName().equals(MANIFEST_NAME))
 583 .filter(e -> !e.getName().endsWith(MODULE_INFO))
 584 .sorted(entryComparator)
 585 .forEachOrdered(je -> {
 586 boolean b = validator.accept(je, jf);
 587 if (validHolder.get()) validHolder.set(b);
 588 });
 589 valid = validHolder.get();
 590 } catch (IOException e) {
 591 error(formatMsg2("error.validator.jarfile.exception", fname, 
e.getMessage()));
 592 valid = false;
 593 } catch (InvalidJarException e) {
 594 error(formatMsg("error.validator.bad.entry.name", 
e.getMessage()));
 595 valid = false;
 596 }
 597 return valid;
 598 }

(IMHO) Using of AtomicBoolean and forEachOrdered() for stateful validator here looks 
forced. It may be avoided by using regular iterator with "for" loop:

Stream sorted = jf.stream()
.filter(e -> !e.isDirectory())
.filter(e -> !e.getName().equals(MANIFEST_NAME))
.filter(e -> !e.getName().endsWith(MODULE_INFO))
.sorted(entryComparator);
for (Iterator iter = sorted.iterator(); iter.hasNext();) {
if (!validator.accept(iter.next(), jf)) {
valid = false;
}
}

or even better by storing "valid" state inside the Validator after turning it to 
regular Consumer:

try (JarFile jf = new JarFile(fname)) {
Validator validator = new Validator(this, jf);
jf.stream()
.filter(e -> !e.isDirectory())
.filter(e -> !e.getName().equals(MANIFEST_NAME))
.filter(e -> !e.getName().endsWith(MODULE_INFO))
.sorted(entryComparator)
.forEachOrdered(validator);
return validator.isValidated();
} catch (IOException e) {
error(formatMsg2("error.validator.jarfile.exception", fname, 
e.getMessage()));
return false;
} catch (NumberFormatException e) {
error(formatMsg("error.validator.bad.entry.name", e.getMessage()));
return false;
}

Moreover what is the reason to have an external loop over the jar's entries? 
Even if we will use several different filter sets in future it would be better 
to use it as optional parameter for Validator.

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Steve Drach]

>>> I would also suggest to pass VM options to jar util in 
>>> test/tools/jar/multiRelease/Basic.java:
>>> 
>>> 540 commands.add(jar);
>>> +541 commands.addAll(Utils.getForwardVmOptions());
>> Done.
>> 
>>> Also it seems that there is no need to compile classes using separate 
>>> process:
>>> 
>>> 526 String javac = JDKToolFinder.getJDKTool("javac");
>>> 
>>> It is possible to use jdk/test/lib/testlibrary/CompilerUtils.java instead.
>> Yes, it does seem possible, and I’ll remember that in the future, but what I 
>> have works, so I think I’ll stick with it.
> 
> In this case it is also needed to pass the jtreg's javac options 
> ("test.compiler.opts”).

OK

> 
> - Oleg
>> 
>>> Thanks,
>>> Oleg
>>> 
>>> 07.07.2016 23:32, Steve Drach пишет:
 Hi,
 
 Please review the following:
 
 webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 
 
 issue: https://bugs.openjdk.java.net/browse/JDK-8158295 
 
 
 This changeset adds a multi-release jar validator to jar tool.  After the 
 jar tool builds a multi-release jar, the potential resultant jar file is 
 passed to the validator to assure that the jar file meets the minimal 
 standards of a multi-release jar, in particular that versioned classes 
 have the same api as base classes they override.  There are other checks, 
 for example warning if two classes are identical.  If the jar file is 
 invalid, it is not kept, so —create will not produce a jar file and 
 —update will keep the input jar file.
 
 Thanks
 Steve
>>> 
> 

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Oleg G. Barbashov]

22.07.2016 23:35, Steve Drach пишет:

I would also suggest to pass VM options to jar util in 
test/tools/jar/multiRelease/Basic.java:

540 commands.add(jar);
+541 commands.addAll(Utils.getForwardVmOptions());

Done.


Also it seems that there is no need to compile classes using separate process:

526 String javac = JDKToolFinder.getJDKTool("javac");

It is possible to use jdk/test/lib/testlibrary/CompilerUtils.java instead.

Yes, it does seem possible, and I’ll remember that in the future, but what I 
have works, so I think I’ll stick with it.


In this case it is also needed to pass the jtreg's javac options 
("test.compiler.opts").


- Oleg



Thanks,
Oleg

07.07.2016 23:32, Steve Drach пишет:

Hi,

Please review the following:

webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 

issue: https://bugs.openjdk.java.net/browse/JDK-8158295 


This changeset adds a multi-release jar validator to jar tool.  After the jar 
tool builds a multi-release jar, the potential resultant jar file is passed to 
the validator to assure that the jar file meets the minimal standards of a 
multi-release jar, in particular that versioned classes have the same api as 
base classes they override.  There are other checks, for example warning if two 
classes are identical.  If the jar file is invalid, it is not kept, so —create 
will not produce a jar file and —update will keep the input jar file.

Thanks
Steve

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Steve Drach]

> I would also suggest to pass VM options to jar util in 
> test/tools/jar/multiRelease/Basic.java:
> 
> 540 commands.add(jar);
> +541 commands.addAll(Utils.getForwardVmOptions());

Done.

> 
> Also it seems that there is no need to compile classes using separate process:
> 
> 526 String javac = JDKToolFinder.getJDKTool("javac");
> 
> It is possible to use jdk/test/lib/testlibrary/CompilerUtils.java instead.

Yes, it does seem possible, and I’ll remember that in the future, but what I 
have works, so I think I’ll stick with it.

> 
> Thanks,
> Oleg
> 
> 07.07.2016 23:32, Steve Drach пишет:
>> Hi,
>> 
>> Please review the following:
>> 
>> webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 
>> 
>> issue: https://bugs.openjdk.java.net/browse/JDK-8158295 
>> 
>> 
>> This changeset adds a multi-release jar validator to jar tool.  After the 
>> jar tool builds a multi-release jar, the potential resultant jar file is 
>> passed to the validator to assure that the jar file meets the minimal 
>> standards of a multi-release jar, in particular that versioned classes have 
>> the same api as base classes they override.  There are other checks, for 
>> example warning if two classes are identical.  If the jar file is invalid, 
>> it is not kept, so —create will not produce a jar file and —update will keep 
>> the input jar file.
>> 
>> Thanks
>> Steve
> 
> 

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Oleg G. Barbashov]

Hi Steve,

I would also suggest to pass VM options to jar util in 
test/tools/jar/multiRelease/Basic.java:


 540 commands.add(jar);
+541 commands.addAll(Utils.getForwardVmOptions());

Also it seems that there is no need to compile classes using separate 
process:


526 String javac = JDKToolFinder.getJDKTool("javac");

It is possible to use jdk/test/lib/testlibrary/CompilerUtils.java instead.

Thanks,
Oleg

07.07.2016 23:32, Steve Drach пишет:

Hi,

Please review the following:

webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 

issue: https://bugs.openjdk.java.net/browse/JDK-8158295 


This changeset adds a multi-release jar validator to jar tool.  After the jar 
tool builds a multi-release jar, the potential resultant jar file is passed to 
the validator to assure that the jar file meets the minimal standards of a 
multi-release jar, in particular that versioned classes have the same api as 
base classes they override.  There are other checks, for example warning if two 
classes are identical.  If the jar file is invalid, it is not kept, so —create 
will not produce a jar file and —update will keep the input jar file.

Thanks
Steve

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Steve Drach]

>> Please review the following:
>> 
>> webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 
>> 
>> issue: https://bugs.openjdk.java.net/browse/JDK-8158295 
>> 
>> 
>> This changeset adds a multi-release jar validator to jar tool.  After the 
>> jar tool builds a multi-release jar, the potential resultant jar file is 
>> passed to the validator to assure that the jar file meets the minimal 
>> standards of a multi-release jar, in particular that versioned classes have 
>> the same api as base classes they override.  There are other checks, for 
>> example warning if two classes are identical.  If the jar file is invalid, 
>> it is not kept, so —create will not produce a jar file and —update will keep 
>> the input jar file.

I’ve updated the webrev to address the issues raised — 
http://cr.openjdk.java.net/~sdrach/8158295/webrev.02/index.html 


> 
> jar/Main.java
> 
> 284 // This code would be repeated in the create and update 
> sections.
> 285 // In order not to do that, it's here as a consumer.
> 286 Consumer validateAndClose = tmpfile -> {
> Why does it need to be Consumer rather than just a method?

I changed it to a method.

> 
> Then i think you don’t need to rethow, but you can still keep the try block 
> and use finally for File.deleteIfExist since it will not complain for the 
> case where you move.

Done.

> 
> 
> 558 int i1 = s1.indexOf('/', n);
> 559 int i2 = s1.indexOf('/', n);
> 560 if (i1 == -1) throw new NumberFormatException(s1);
> 561 if (i2 == -2) throw new NumberFormatException(s2);
> 
> I think you are trying to reject entries directly under the versioned area so 
> it’s not about numbers (same in the validator, so there is some redundancy?).

A couple things here.  The most obvious is it should be “if (i2 == -1)”.  I 
replaced NumberFormatException with a new InvalidJarException.  I added a 
comment that
this code is used to sort the version numbers as string representations of 
numbers, therefore “9” goes before “10”, not usual for string sorts.

> 
> 
> 588 AtomicBoolean validHolder = new AtomicBoolean();
> 589 validHolder.set(valid);
> 
> Pass valid to the constructor

Done.

> 
> 
> Validator/FingerPrint.java
> 
> It would be useful to have some comments on what is checked in terms of API 
> compatibility. e.g. describe what FingerPrint collects and how Validator uses 
> it.

Added some commentary to FingerPrint.

> 
> 
> FingerPrint.java
> 
> 205 private final Map fields = new HashMap<>();
> 206 private final Map methods = new HashMap<>();
> 
> Not sure you need to use a Map, why not use a Set?

Not sure why I did it with Maps instead of Sets, but I wanted to keep the 
method and field names and maybe that made sense at one time.  It doesn’t now, 
so Sets they are.

> 
> Change Method to be literally a representation of a single method rather than 
> a consolidation that is lossy for the set of exceptions.

Done.

Re: RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Paul Sandoz]

> On 7 Jul 2016, at 22:32, Steve Drach  wrote:
> 
> Hi,
> 
> Please review the following:
> 
> webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8158295 
> 
> 
> This changeset adds a multi-release jar validator to jar tool.  After the jar 
> tool builds a multi-release jar, the potential resultant jar file is passed 
> to the validator to assure that the jar file meets the minimal standards of a 
> multi-release jar, in particular that versioned classes have the same api as 
> base classes they override.  There are other checks, for example warning if 
> two classes are identical.  If the jar file is invalid, it is not kept, so 
> —create will not produce a jar file and —update will keep the input jar file.
> 

jar/Main.java

 284 // This code would be repeated in the create and update 
sections.
 285 // In order not to do that, it's here as a consumer.
 286 Consumer validateAndClose = tmpfile -> {
Why does it need to be Consumer rather than just a method?

Then i think you don’t need to rethow, but you can still keep the try block and 
use finally for File.deleteIfExist since it will not complain for the case 
where you move.


 558 int i1 = s1.indexOf('/', n);
 559 int i2 = s1.indexOf('/', n);
 560 if (i1 == -1) throw new NumberFormatException(s1);
 561 if (i2 == -2) throw new NumberFormatException(s2);

I think you are trying to reject entries directly under the versioned area so 
it’s not about numbers (same in the validator, so there is some redundancy?).


 588 AtomicBoolean validHolder = new AtomicBoolean();
 589 validHolder.set(valid);

Pass valid to the constructor


Validator/FingerPrint.java

It would be useful to have some comments on what is checked in terms of API 
compatibility. e.g. describe what FingerPrint collects and how Validator uses 
it.


FingerPrint.java

 205 private final Map fields = new HashMap<>();
 206 private final Map methods = new HashMap<>();

Not sure you need to use a Map, why not use a Set?

Change Method to be literally a representation of a single method rather than a 
consolidation that is lossy for the set of exceptions.

Paul.

RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

[Steve Drach]

Hi,

Please review the following:

webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ 

issue: https://bugs.openjdk.java.net/browse/JDK-8158295 


This changeset adds a multi-release jar validator to jar tool.  After the jar 
tool builds a multi-release jar, the potential resultant jar file is passed to 
the validator to assure that the jar file meets the minimal standards of a 
multi-release jar, in particular that versioned classes have the same api as 
base classes they override.  There are other checks, for example warning if two 
classes are identical.  If the jar file is invalid, it is not kept, so —create 
will not produce a jar file and —update will keep the input jar file.

Thanks
Steve