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> 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> > 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/ > > Thanks, > Sherman >