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

Calvin Cheung calvin.cheung at oracle.com
Fri Jun 12 04:24:03 UTC 2020


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