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

Calvin Cheung calvin.cheung at oracle.com
Fri Jun 12 16:51:00 UTC 2020


Thanks - David, Yumin, Ioi.

Changeset pushed.

thanks,

Calvin

On 6/11/20 10:25 PM, Ioi Lam wrote:
> Looks good to me, too:
>
> $ wget 
> http://cr.openjdk.java.net/~ccheung/jdk16/8245060/webrev.01/open.patch
> $ cat open.patch  | grep '^[+-] ' | grep -v Copyright | less
>
> Thanks
> - Ioi
>
> On 6/11/20 10:11 PM, Yumin Qi wrote:
>> 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
>>>>>>>
>


More information about the hotspot-runtime-dev mailing list