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

Lance Andersen lancea at openjdk.org
Tue Sep 24 19:24:36 UTC 2024


On Tue, 24 Sep 2024 17:01:54 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 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


More information about the core-libs-dev mailing list