JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.
Kumar Srinivasan
kumar.x.srinivasan at oracle.com
Tue Jan 21 16:11:19 UTC 2014
Alex,
Thanks for adding the test, few comments:
PackTestZip64.java:
1. compareTwoFile, I would read the entire file into
ByteArrayInputStream, compare
the total first ie. fast fail, but what you have is also ok.
2. generateLargeJar: I would replace lines 126,127 and 128 using
for-each loop,
by using Collections.list<Enumeration> e); will make it more compact.
Though you are usign Try-With-Resources on the input jar, how about
3. Though the test might not take too long on your system, I am
concerned it may
take too long on our Jprt system.
Otherwise looks good.
Kumar
On 1/16/2014 8:21 AM, Alexander Zuev wrote:
> Sherman, Kumar,
>
> i have fixed the glitches you have found and changed the test so it
> creates a new jar
> based on the golden.jar content (to have a reasonable set of various
> data to start with),
> then adding to it 65536 empty files to test how we cope with such a
> huge jars.
>
> The testing time is less than a 30 seconds on my machine (not the
> top-of-the-line) so i
> decided not to bother with conditional testing, lets test our behavior
> every time in full.
>
> The updated webrev can be found at
> http://cr.openjdk.java.net/~kizune/8029646/webrev.02
>
> /Alex
>
> On 1/15/14 21:34, Xueming Shen wrote:
>> On 1/15/14 7:01 AM, Alexander Zuev wrote:
>>> Hello,
>>>
>>> the new webrev with all the typos and comments fixed can be found
>>> at http://cr.openjdk.java.net/~kizune/8029646/webrev.01/
>>>
>>> /Alex
>>
>> (1) jarmagic can be just a static constant somewhere or a stack
>> variable. not big deal though.
>> (2) the test only "tests" EOF for s. there is possibility that the
>> newly created has more extra
>> bytes at the end...though in theory this should not happen, it
>> might be better just add an
>> extra line to check the sizes of two first first?
>> (3) the rest of the change looks good, but I agreed with Kumar that
>> you may need to add a
>> regression test for a jar file with > 64k entries. otherwise
>> the code for zip64 end is not
>> being tested. the code looks fine, but I would trust a
>> regression test more than my eyes:-)
>>
>> Thanks!
>> -Sherman
>
More information about the core-libs-dev
mailing list