Re: RFR: 8165723: JarFile::isMultiRelease() method returns false when it should return true

[Paul Sandoz]

> On 12 Sep 2016, at 08:10, Claes Redestad  wrote:
> 
> On 2016-09-12 16:24, Alan Bateman wrote:
>> This looks okay.
> 
> Thanks for the review!
> 
>> For the MultiReleaseJarAPI test then you probably should use 
>> jdk.testlibrary.RandomFactory so that the seed is recorded in the output for 
>> when the test fails.
> 
> Done.
> 
>> Also in CreateMultiReleaseTestJars then it might be cleaner to have a 
>> separate method to add extra stuff to the JAR file - I think that would make 
>> the usages a bit easier to read.
> 
> I took a second look, realized 
> CreateMultiReleaseTestJars::buildShortMultiReleaseJar is no longer being used 
> by any test, and simplified things a bit accordingly:
> 
> http://cr.openjdk.java.net/~redestad/8165723/webrev.02/
> 

+1 (post review i see it is already committed).

Our optimisation to compress the “optoSft” data was premature, i guess we had 
an error in the initial data.

Paul.

Re: RFR: 8165723: JarFile::isMultiRelease() method returns false when it should return true

[Claes Redestad]

On 2016-09-12 16:24, Alan Bateman wrote:
This looks okay. 


Thanks for the review!

For the MultiReleaseJarAPI test then you probably should use 
jdk.testlibrary.RandomFactory so that the seed is recorded in the 
output for when the test fails. 


Done.

Also in CreateMultiReleaseTestJars then it might be cleaner to have a 
separate method to add extra stuff to the JAR file - I think that 
would make the usages a bit easier to read.


I took a second look, realized 
CreateMultiReleaseTestJars::buildShortMultiReleaseJar is no longer being 
used by any test, and simplified things a bit accordingly:


http://cr.openjdk.java.net/~redestad/8165723/webrev.02/

Thanks!

/Claes

Re: RFR: 8165723: JarFile::isMultiRelease() method returns false when it should return true

[Alan Bateman]

On 10/09/2016 01:58, Claes Redestad wrote:


Hi,

JDK-8152733 introduced a corner-case where isMultiRelease returns
false when it should return true due to an erroneously hand-crafted
Boyer-Moore search.

Webrev: http://cr.openjdk.java.net/~redestad/8165723/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8165723

Testing: created a test reproducer using randomly constructed
manifests and verified the supplied jar is properly detected as being
multi-release.
This looks okay. For the MultiReleaseJarAPI test then you probably 
should use jdk.testlibrary.RandomFactory so that the seed is recorded in 
the output for when the test fails. Also in CreateMultiReleaseTestJars 
then it might be cleaner to have a separate method to add extra stuff to 
the JAR file - I think that would make the usages a bit easier to read.


-Alan