Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-22 Thread Kumar Srinivasan
Alex, I am satisfied. Thanks Kumar Kumar, see my notes inline. On 1/21/14 20:11, Kumar Srinivasan wrote: 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 f

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-22 Thread Alexander Zuev
Kumar, see my notes inline. On 1/21/14 20:11, Kumar Srinivasan wrote: 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. I've t

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-21 Thread Kumar Srinivasan
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

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-16 Thread Alexander Zuev
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

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Kumar Srinivasan
Hi Alex, Kumar, thanks for your findings. See my comments inline. On 1/15/14 2:10, Kumar Srinivasan wrote: Hi Alex, zip.cpp: (nit) I would keep the hex values to be in upper case just like the others for consistency, not a big deal. Fixed. typo: + // Zi

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Alexander Zuev
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 variab

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Xueming Shen
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

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Alexander Zuev
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

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Alexander Zuev
Kumar, thanks for your findings. See my comments inline. On 1/15/14 2:10, Kumar Srinivasan wrote: Hi Alex, zip.cpp: (nit) I would keep the hex values to be in upper case just like the others for consistency, not a big deal. Fixed. typo: + // Zip64 END sugn

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Alexander Zuev
On 1/15/14 18:34, Alexander Zuev wrote: Sherman et all, self-correction regarding the flags, i misread the specification so flags are: always support UTF-8 file encoding (bit 11) and using EOS marker for the compressed files(bit 4). Damn my fast fingers - not bit 4, bit 3 and it tells we are s

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Alexander Zuev
Sherman et all, self-correction regarding the flags, i misread the specification so flags are: always support UTF-8 file encoding (bit 11) and using EOS marker for the compressed files(bit 4). /Alex On 1/15/14 18:26, Alexander Zuev wrote: Hi Sherman, Thanks for comments, here are some ans

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-15 Thread Alexander Zuev
Hi Sherman, Thanks for comments, here are some answers: the answer to (1)-(3) is that i do whatever the JarOutputStream/ZipOutputStream of the current JDK does and for a reason - there was a request from couple of customers to make native unpack200 results binary identical to the results t

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-14 Thread Xueming Shen
Hi Alex, (1) what's the bits are you setting into the general flags fields as below? header[4] = ( store ) ? SWAP_BYTES(0x0800) : 0x0808; (2) why do you want to use extra data descriptor to set crc/size/csize? while I understand that Jar/ZipOutputStream does that, but that is because they

Re: JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-14 Thread Kumar Srinivasan
Hi Alex, zip.cpp: (nit) I would keep the hex values to be in upper case just like the others for consistency, not a big deal. typo: + // Zip64 END sugnature PackTestZip64.java: shouldn't we test a jar with 64K+ entries ? Kumar On 1/14/2014 10:04 AM, Alexande

JDK9 RFR of JDK-8029646: [pack200] should support the new zip64 format.

2014-01-14 Thread Alexander Zuev
Please review my fix for JDK-8029646: [pack200] should support the new zip64 format. The fix can be found at http://cr.openjdk.java.net/~kizune/8029646/webrev.00/ Bug description is: https://bugs.openjdk.java.net/browse/JDK-8029646 /Alex