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

[Sean Chou]

That's right, thanks.


On Wed, Nov 14, 2012 at 1:34 PM, Jonathan Lu <luchsh at linux.vnet.ibm.com>wrote:

> Hi Sean,
>
> Patch pushed @ http://hg.openjdk.java.net/**jdk8/tl/jdk/rev/83765e82cacb<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/>
>>>> <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/>
>>>>>> <ht**tp://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/>
>>>>>> <h**ttp://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/<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