RFR: 8011397: JTREG needs to copy additional WhiteBox class file to JTwork/scratch/sun/hotspot

[Igor Ignatyev]

Andrey,

I don't like this idea, since it completely changes the tests. 
'run/othervm/bootclasspath' adds all paths from CP to BCP, so the tests 
whose main idea was testing WB methods themselves (sanity, 
compiler/whitebox, ...) don't check that it's possible to use WB when 
the application isn't in BCP.

Igor

On 06/09/2014 06:59 PM, Andrey Zakharov wrote:
> Hi, everybody
> I have tested my changes on major platforms and found one bug, filed:
> https://bugs.openjdk.java.net/browse/JDK-8046231
> Also, i did another try to make ClassFileInstaller to copy all inner
> classes within parent, but this fails for WhiteBox due its static
> "registerNatives" dependency.
>
> Please, review suggested changes:
>   - replace ClassFileInstaller and run/othervm with
> "run/othervm/bootclasspath".
>         bootclasspath parameter for othervm adds-Xbootclasspath/a:
> option with ${test.src} and ${test.classes}according to
> http://hg.openjdk.java.net/code-tools/jtreg/file/31003a1c46d9/src/share/classes/com/sun/javatest/regtest/MainAction.java.
> Is this suitable for our needs - give to test compiled WhiteBox?
>   - replace explicit -Xbootclasspath option values (".") in
> ProcessBuilder invocations to ${test.classes} where WhiteBox has been
> compiled.
>
> Webrev: http://cr.openjdk.java.net/~fzhinkin/azakharov/8011397/webrev.00/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8011397
> Thanks.
>
>
> On 23.05.2014 15:40, Andrey Zakharov wrote:
>>
>> On 22.05.2014 12:47, Igor Ignatyev wrote:
>>> Andrey,
>>>
>>> 1. You changed dozen of tests, have you tested your changes?
>> Locally, aurora on the way.
>>>
>>> 2. Your changes of year in copyright is wrong. it has to be
>>> $first_year, [$last_year, ], see Mark's email[1] for details.
>>>
>>> [1] http://mail.openjdk.java.net/pipermail/jdk7-dev/2010-May/001321.html
>> Thanks, fixed. will be uploaded soon.
>>
>>
>>>
>>> Igor
>>>
>>> On 05/21/2014 07:37 PM, Andrey Zakharov wrote:
>>>>
>>>> On 13.05.2014 14:43, Andrey Zakharov wrote:
>>>>> Hi
>>>>> So here is trivial patch -
>>>>> removing  ClassFileInstaller sun.hotspot.WhiteBox and adding
>>>>> main/othervm/bootclasspath
>>>>> where this needed
>>>>>
>>>>> Also, some tests are modified as
>>>>> -        "-Xbootclasspath/a:.",
>>>>> +        "-Xbootclasspath/a:" + System.getProperty("test.classes"),
>>>>>
>>>>> Thanks.
>>>> webrev: http://cr.openjdk.java.net/~jwilhelm/8011397/webrev.02/
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8011397
>>>> Thanks.
>>>>
>>>>>
>>>>> On 09.05.2014 12:13, Mikael Gerdin wrote:
>>>>>> On Thursday 08 May 2014 19.28.13 Igor Ignatyev wrote:
>>>>>>> // cc'ing hotspot-dev instaed of compiler, runtime and gc lists.
>>>>>>>
>>>>>>> On 05/08/2014 07:09 PM, Filipp Zhinkin wrote:
>>>>>>>> Andrey,
>>>>>>>>
>>>>>>>> I've CC'ed compiler and runtime mailing list, because you're
>>>>>>>> changes
>>>>>>>> affect test for other components as too.
>>>>>>>>
>>>>>>>> I don't like your solution (but I'm not a reviewer, so treat my
>>>>>>>> words
>>>>>>>> just as suggestion),
>>>>>>>> because we'll have to write more meta information for each test
>>>>>>>> and it
>>>>>>>> is very easy to
>>>>>>>> forget to install WhiteBoxPermission if you don't test your test
>>>>>>>> with
>>>>>>>> some security manager.
>>>>>>>>
>>>>>>>>   From my point of view, it will be better to extend
>>>>>>>> ClassFileInstaller
>>>>>>>>
>>>>>>>> so it will copy not only
>>>>>>>> a class whose name was passed as an arguments, but also all inner
>>>>>>>> classes of that class.
>>>>>>>> And if someone want copy only specified class without inner
>>>>>>>> classes,
>>>>>>>> then some option
>>>>>>>> could be added to ClassFileInstaller to force such behaviour.
>>>>>> Perhaps this is a good time to get rid of ClassFileInstaller
>>>>>> altogether?
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8009117
>>>>>>
>>>>>> The reason for its existence is that the WhiteBox class needs to be
>>>>>> on the
>>>>>> boot class path.
>>>>>> If we can live with having all the test's classes on the boot class
>>>>>> path then
>>>>>> we could use the /bootclasspath option in jtreg as stated in the RFE.
>>>>>>
>>>>>> /Mikael
>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Filipp.
>>>>>>>>
>>>>>>>> On 05/08/2014 04:47 PM, Andrey Zakharov wrote:
>>>>>>>>> Hi!
>>>>>>>>> Suggesting patch with fixes for
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8011397
>>>>>>>>>
>>>>>>>>> webrev:
>>>>>>>>> https://bugs.openjdk.java.net/secure/attachment/20275/8011397.tgz
>>>>>>>>> patch:
>>>>>>>>> https://bugs.openjdk.java.net/secure/attachment/20274/8011397.WhiteBoxPer
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> mission
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks.
>>>>>
>>>>
>>
>