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

[Paul Sandoz]

> On 7 Jul 2016, at 22:32, Steve Drach <steve.drach at oracle.com> wrote:
> 
> Hi,
> 
> Please review the following:
> 
> webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ <http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/>
> issue: https://bugs.openjdk.java.net/browse/JDK-8158295 <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<File> 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<String,Field> fields = new HashMap<>();
 206         private final Map<String,Method> 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.