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

[David Holmes]

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
>>>