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

[Henry Jen]

On Tue, 24 Sep 2024 19:16:17 GMT, Lance Andersen <lancea at openjdk.org> wrote:

>> 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
>
> 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

I follow existing pattern with short status update. Open to suggestions.

> 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.`

Updated.

> 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

Done. Also add a test case for multiple manifest files in the same archive as we mentioned such use case in the help text.

> 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

Updated.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1777910329
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1777909703
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1777911075
PR Review Comment: https://git.openjdk.org/jdk/pull/21141#discussion_r1777910602