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