RFR: 8335912: Add an operation mode to the jar command when extracting to not overwriting existing files [v4]

Jaikiran Pai jpai at openjdk.org
Sat Sep 28 14:35:39 UTC 2024


On Fri, 27 Sep 2024 01:41:33 GMT, Henry Jen <henryjen at openjdk.org> wrote:

>> This PR support a -k, --keep options like tar that allows jar to avoid override existing files.
>
> Henry Jen has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Address review feedbacks

test/jdk/tools/jar/ExtractFilesTest.java line 236:

> 234:         PrintStream err = new PrintStream(baes);
> 235:         PrintStream saveErr = System.err;
> 236:         System.setErr(err);

Given that we are updating the runtime level state, I think we should use `/othervm` in this test's definition to avoid any interference with other parts of the runtime.

test/jdk/tools/jar/ExtractFilesTest.java line 238:

> 236:         System.setErr(err);
> 237:         int rc = JAR_TOOL.run(out, err, cmdline.split(" +"));
> 238:         System.setErr(saveErr);

Doing a

try {
int rc = JAR_TOOL.run(out, err, cmdline.split(" +"));
}finally {
System.setErr(saveErr);
}

would be safer.

test/jdk/tools/jar/ExtractFilesTest.java line 241:

> 239:         if (rc != 0) {
> 240:             String s = baes.toString();
> 241:             if (s.startsWith("java.util.zip.ZipException: duplicate entry: ")) {

This method runs any arbitrary `jar` "command". Is there any significance why we are checking for a "duplicate entry" in the exception message? Maybe we could just remove this entire if block and just throw a `new IOException(s)`? As far as I can see in this test, we don't do any exception type checks on the thrown exception from this method.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1779499876
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1779500135
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1779501074


More information about the core-libs-dev mailing list