Request for review: 7201156 : jar tool fails to convert file separation characters for list and extract

[Jonathan Lu]

Hi Sean,

Patch pushed @ http://hg.openjdk.java.net/jdk8/tl/jdk/rev/83765e82cacb
Could you please verify?

Thanks & regards
Jonathan

On 11/14/2012 10:23 AM, Sean Chou wrote:
> Thanks Alan and Chris.
>
>
> On Tue, Nov 13, 2012 at 7:21 PM, Chris Hegarty <chris.hegarty at oracle.com>wrote:
>
>> Sean,
>>
>> Looks good to me. Thanks for updating the test.
>>
>> -Chris.
>>
>>
>> On 11/13/2012 03:17 AM, Sean Chou wrote:
>>
>>> Hi Alan,
>>>
>>> Here is the updated webrev:
>>> http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.03/<http://cr.openjdk.java.net/~zhouyx/7201156/webrev.03/> .
>>>
>>>
>>> On Mon, Nov 12, 2012 at 6:00 PM, Alan Bateman <Alan.Bateman at oracle.com>**
>>> wrote:
>>>
>>>   On 12/11/2012 09:08, Sean Chou wrote:
>>>>   Hi ,
>>>>> I didn't realize that rt.jar would miss before. The testcase is updated
>>>>> to create a temp file as well as test the extract option. However,
>>>>> extract
>>>>> testing will create a directory if it passes, I just deleted it in
>>>>> testcase.  Please take a look.
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~****zhouyx/7201156/webrev.02/<http://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/>
>>>>> <http**://cr.openjdk.java.net/~**zhouyx/7201156/webrev.02/<http://cr.openjdk.java.net/~zhouyx/7201156/webrev.02/>
>>>>>> <
>>>>> http://cr.openjdk.java.net/%****7Ezhouyx/7201156/webrev.02/<ht**
>>>>> tp://cr.openjdk.java.net/%**7Ezhouyx/7201156/webrev.02/<http://cr.openjdk.java.net/%7Ezhouyx/7201156/webrev.02/>
>>>>>    .
>>>>>
>>>>>    Thanks for removing the dependency on rt.jar.
>>>>>
>>>> A couple of comments on the updated test:
>>>>
>>>> - in main then it still has "pathRtJar" so I assume you just forgot to
>>>> rename that.
>>>>
>>>>   I forgot the name had its meaning.
>>>
>>>> - it would be good to change createJarFile to use try-with-resources so
>>>> that an error doesn't cause it to terminate with an open file.
>>>>
>>>>   Changed.
>>>
>>>> - testJarExact, I assume you intended to name this testJarExtract.
>>>>
>>>>   Yes!
>>>
>>>> - L114-117, the "./" is not needed. It would be okay to leave those files
>>>> there if you want, jtreg will clean them up too.
>>>>
>>> Thanks for the hint. I removed these lines, so the testcase looks tidy.
>>>
>>>
>>>> -Alan.
>>>>
>>>>
>>>>
>>>>
>>>
>