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