RFR(s) 8245060: remove copying of s.h.WB$WhiteBoxPermission in cds/appcds tests

[Yumin Qi]

Looks good to me.


Thanks

Yumin

On 6/11/20 9:24 PM, Calvin Cheung wrote:
> Hi David and Yumin,
>
> Here's an updated webrev:
>
>     http://cr.openjdk.java.net/~ccheung/jdk16/8245060/webrev.01/
>
> It contains only changes to the @run and copyright header.
>
> thanks,
>
> Calvin
>
> On 6/11/20 6:40 PM, David Holmes wrote:
>> On 12/06/2020 11:08 am, Calvin Cheung wrote:
>>> Hi David,
>>>
>>> Thanks for your review.
>>>
>>> On 6/11/20 5:04 PM, David Holmes wrote:
>>>> Hi Calvin,
>>>>
>>>> On 12/06/2020 8:43 am, Calvin Cheung wrote:
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8245060
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~ccheung/jdk16/8245060/webrev.00/
>>>>>
>>>>> Most changes involve removing the 
>>>>> sun.hotspot.WhiteBox$WhiteBoxPermission from the following:
>>>>>
>>>>> @run driver ClassFileInstaller sun.hotspot.WhiteBox 
>>>>> sun.hotspot.WhiteBox$WhiteBoxPermission
>>>>
>>>> That bit seems fine.
>>>>
>>>> Your copyright updates are wrong:
>>>>
>>>> - * Copyright (c) 2019, Oracle and/or its affiliates. All rights 
>>>> reserved.
>>>> + * Copyright (c) 2020, Oracle and/or its affiliates. All rights 
>>>> reserved.
>>>>
>>>> They should be
>>>>
>>>> - * Copyright (c) 2019, Oracle and/or its affiliates. All rights 
>>>> reserved.
>>>> + * Copyright (c) 2019, 2020, Oracle and/or its affiliates. All 
>>>> rights reserved.
>>> I did it too quick. I'll correct them.
>>>>
>>>>> Two additional cleanups:
>>>>> - remove the unneeded SkippedException related code from the tests 
>>>>> under dynamicArchive/methodHandles;
>>>>
>>>> This is completly unrelated cleanup.
>>> Can I leave those in? Or do you prefer to have those changed in a 
>>> separate bug?
>>
>> I'd prefer to see it separated out.
>>
>>>>
>>>>> - remove the JarBuilder.addInnerClasses method since its purpose 
>>>>> is to add the sun.hotspot.WhiteBox$WhiteBoxPermission inner class.
>>>>
>>>> I'm not clear how this relates either. Don't we still need to list 
>>>> all classes to be added to the jar file? If this code is unneeded 
>>>> then it must have been unneeded for quite some time now.
>>>
>>> IIUC, the function only adds sun/hotspot/WhiteBox$WhiteBoxPermission 
>>> to the list of classes if the list contains sun/hotspot/WhiteBox. 
>>> Although all appcds tests passed without the function, I will revert 
>>> the JarBuilder.java change to keep it consistent with 
>>> ClassFileInstaller and we may need it in the future.
>>
>> I think there may be something to look at here, but that would be a 
>> separate issue.
>>
>> Thanks,
>> David
>>
>>> thanks,
>>>
>>> Calvin
>>>
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> Passed tier1, 2 tests.
>>>>
>>>>
>>>>
>>>>> thanks,
>>>>>
>>>>> Calvin
>>>>>