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

Ioi Lam ioi.lam at oracle.com
Fri Jun 12 05:25:05 UTC 2020


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