Thanks Martin!
webrev has been updated as suggested (the old webrev has been renamed to
webrev00)
http://cr.openjdk.java.net/~sherman/8186464/webrev
The only thing is not updated is the centot, we are using it as "int"
anyway at other place.
Let's keep it for couple more years till the 32-bit is totally gone.
-Sherman
On 9/5/17, 5:51 PM, Martin Buchholz wrote:
Thanks for doing this.
I'm also hoping to contribute some testing effort, but time ... time ...
This Looks Good To Me, but as always I have comments.
---
I wouldn't create an echo process. Just create a zip subprocess and
write the bytes from java to its stdin.
181 ProcessBuilder echo = new ProcessBuilder("echo", "hello");
---
You should check whether there's a usable zip program, e.g. I once
wrote code like this:
if (! new File("/usr/bin/perl").canExecute()
182 ProcessBuilder zip = new ProcessBuilder("zip", path.toString().toString(),
"-");
oh wait, now I see
196 } catch (IOException x) {
197 // ignore, probably from process.start() for "Cannot run
program"
well ... I would still add some checks for /usr/bin/zip and elide the
catch.
---
"""Always specify the charset""" (here "ASCII" is fine).
163 if (!"hello".equals(new String(zf.getInputStream(new
ZipEntry("hello"))
164 .readAllBytes())))
---
Can we say something like
// We must always check for a zip64 EOCD record; it is always
permitted to be present
1125 // try if we have a zip64 end;
---
Maybe
// end64 candidate found
1139 // end64 found,
---
It's 2017. Time to just make centot a long?
1152 end.centot = (int)centot64; // assume total<
2g
---
Here's a typo to fix:
entires
On Fri, Sep 1, 2017 at 4:17 PM, Xueming Shen <xueming.s...@oracle.com
<mailto:xueming.s...@oracle.com>> wrote:
On 8/22/17, 5:54 PM, Martin Buchholz wrote:
On Tue, Aug 22, 2017 at 3:27 PM, Xueming Shen
<xueming.s...@oracle.com <mailto:xueming.s...@oracle.com>> wrote:
How about to add an option to our zipfs to force the ZIP64
end record when
enabled. Harmless if not specified.
I agree that adding more options for testability is good. Since
our users are likely to need such things as well, I'd also like
to see such features become public, analogous to zip's -fz- flag.
OTOH, interoperability testing is very valuable, so despite the
troubles involved writing tests that involve both jdk and zipinfo
code, I think we should do it.
Martin,
I have added a test case that tests "echo hello | zip infozip.zip
-" via pb.startPipeline()
in ReadZip.java, which should add some coverage for the
"interoperability testing".
Would you please help take a look?
http://cr.openjdk.java.net/~sherman/8186464/webrev/
<http://cr.openjdk.java.net/%7Esherman/8186464/webrev/>
Thanks,
Sherman
