RFR(M/T) : 8243935 : s.h.WhiteBoxPermission must be ClassFileInstall-ed together w/ s.h.WhiteBox

[David Holmes]

On 28/04/2020 5:34 pm, Stefan Karlsson wrote:
> Hi,
> 
> I have another idea. Can't we simply remove WhiteBoxPermission and 
> incorporate that functionality in the WhiteBox class? We would then be 
> able to remove all these WhiteBoxPermission lines.

Interesting though a little hackish. :)

But why is this an issue anyway we already hacked ClassfileInstaller to 
ensure it always copies the permission class!!!

  102     // Add commonly used inner classes that are often omitted by 
mistake. Currently
  103     // we support only sun.hotspot.WhiteBox$WhiteBoxPermission. 
See JDK-8199290
  104     private static String[] addInnerClasses(String[] classes, int 
startIdx) {
  105         boolean seenWB = false;
  106         boolean seenWBInner = false;
  107         final String wb = "sun.hotspot.WhiteBox";
  108         final String wbInner = 
"sun.hotspot.WhiteBox$WhiteBoxPermission";

David
-----

> I've tested this and it seems to work:
> https://cr.openjdk.java.net/~stefank/prototype/removeWhiteBoxPermission/webrev.01/ 
> 
> 
> Summary of changes:    695 lines changed: 0 ins; 657 del; 38 mod; 60752 
> unchg
> 
> The interesting code:
> https://cr.openjdk.java.net/~stefank/prototype/removeWhiteBoxPermission/webrev.01/test/lib/sun/hotspot/WhiteBox.java.udiff.html 
> 
> 
> (There are some parts of the patch that was butchered by the sed-script 
> - I'll fix this if we agree that this is a good thing to do)
> 
> If you think this is something we should do, I'll create an RFE, 
> complete the patch, run more tests, and send it out for review.
> 
> Thanks,
> StefanK
> 
> On 2020-04-28 08:08, David Holmes wrote:
>> On 28/04/2020 3:37 pm, Igor Ignatyev wrote:
>>>
>>>
>>>> On Apr 27, 2020, at 10:03 PM, David Holmes <david.holmes at oracle.com> 
>>>> wrote:
>>>>
>>>> Hi Igor,
>>>>
>>>> On 28/04/2020 12:32 pm, Igor Ignatyev wrote:
>>>>> http://cr.openjdk.java.net/~iignatyev//8243935/webrev.00/
>>>>>> 338 lines changed: 107 ins; 0 del; 231 mod
>>>>> Hi all,
>>>>> could you please review the trivial yet tedious patch which adds 
>>>>> 'sun.hotspot.WhiteBox$WhiteBoxPermission' to all 
>>>>> 'ClassFileInstaller sun.hotspot.WhiteBox' actions?
>>>>> the patch also
>>>>>   - replaces '@run main ClassFileInstaller' w/ '@run driver 
>>>>> ClassFileInstaller'
>>>>>   - unifies the way ClassFileInstaller sun.hotspot.WhiteBox are 
>>>>> aligned
>>>>
>>>> That alignment change is unnecessary and makes it very hard to 
>>>> review this patch.
>>>>
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8243935
>>>>> webrevs:
>>>>>   - http://cr.openjdk.java.net/~iignatyev//8243935/webrev.00/ -- 
>>>>> regular webrev
>>>>>   - 
>>>>> http://cr.openjdk.java.net/~iignatyev//8243935/webrev.00.including_ws 
>>>>> -- shows "whitespace only" changes
>>>>
>>>> I'd like to see the patch that leaves out the whitespace-only 
>>>> changes - is that possible?
>>> sure, I've used `hg diff -w` (--ignore-all-space    ignore white 
>>> space when comparing lines) to remove all whitespace changes, here is 
>>> webrev -- http://cr.openjdk.java.net/~iignatyev//8243935/webrev.00.w/
>>
>> Thanks that was somewhat better. Change seems okay.
>>
>> David
>>
>>> -- Igor
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> Thanks,
>>>>> -- Igor
>>>
>