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

[Sean Chou]

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


-- 
Best Regards,
Sean Chou