On Tue, 24 Sep 2024 17:01:54 GMT, Henry Jen <henry...@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 with a new target base due to a merge
> or a rebase. The pull request now contains one commit:
>
> 8335912: Add an operation mode to the jar command when extracting to not
> overwriting existing files
Thank you for tackling Henry.
A few initial comments. Also please make sure to update the copyright on the
files being updated to 2024
src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 169:
> 167: extracted: {0}
> 168: out.kept=\
> 169: \ \ skipped: {0}
We might want to add a bit more wording to indicate it is being skipped due to
the file already existing on disk
src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 316:
> 314: main.help.opt.extract.keep-old-files=\
> 315: \ -k, --keep-old-files Do not overwrite existing files.\n\
> 316: \ In particular, if a file appears more than
> once in an\n\
In addition, we need to update the help to clarify -x. --extract behavior that
it will overwrite by default.
Here is the verbiage from tar to give you a start
` -x Extract to disk from the archive. If a file with the same name
appears more than
once in the archive, each copy will be extracted, with later
copies overwriting
(replacing) earlier copies. The long option form is --extract.`
test/jdk/tools/jar/ExtractFilesTest.java line 32:
> 30: * @build jdk.test.lib.Platform
> 31: * jdk.test.lib.util.FileUtils
> 32: * @run testng ExtractFilesTest
Please convert the test to use junit as we are moving away from testng
test/jdk/tools/jar/ExtractFilesTest.java line 94:
> 92:
> 93: @Test
> 94: public void testExtract() throws IOException {
Please consider adding comments introducing the methods used by the test
-------------
PR Review: https://git.openjdk.org/jdk/pull/21141#pullrequestreview-2326184792
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1773915982
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1773915040
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1773928582
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1773920945
