thanks! LGTM.
-- Igor
> On Mar 18, 2020, at 10:35 AM, Alexander Scherbatiy
> <alexander.scherba...@bell-sw.com> wrote:
>
> On 18.03.2020 20:02, Igor Ignatyev wrote:
>
>> +import static jdk.test.lib.Utils.TEST_CLASS_PATH;
>> I'm not a huge fun of 'import static', yet don't insist on removing it
>> either.
>>
>> + System.out.println(" libjvm : " + jvmLibDir.toString());
>> jvmLibDir doesn't point to libjvm, so you need either update message prefix
>> or use the actual value which will be used as path to libjvm. I personally
>> prefer the latter.
>> btw, you don't need to explicitly call toString in string concatenation.
>>
> Here is the updated fix where the static import is removed and libjvm path
> is used:
>
> http://cr.openjdk.java.net/~alexsch/8240604/webrev.03/
> <http://cr.openjdk.java.net/~alexsch/8240604/webrev.03/>
>
> Thanks,
>
> Alexander.
>
>
>
>> -- Igor
>>
>>> On Mar 18, 2020, at 9:54 AM, Alexander Scherbatiy
>>> <alexander.scherba...@bell-sw.com
>>> <mailto:alexander.scherba...@bell-sw.com>> wrote:
>>>
>>> On 18.03.2020 19:00, Igor Ignatyev wrote:
>>>
>>>> Hi Alexander,
>>>>
>>>>> I also included TEST_NATIVE_PATH to the Utils lib.
>>>> for the sake of clarity and ease of backporting, I'd prefer to have it
>>>> added by a separate bug and commit.
>>>
>>> Here is the updated fix where TEST_NATIVE_PATH is not added to the Utils
>>> lib.
>>>
>>> http://cr.openjdk.java.net/~alexsch/8240604/webrev.02/
>>> <http://cr.openjdk.java.net/~alexsch/8240604/webrev.02/>
>>>
>>>
>>> Thanks,
>>>
>>> Alexander.
>>>
>>>>> Could I just use "hg remove binary-fie" and run webrev to add the removed
>>>>> binary files into webrev?
>>>> IIRC correctly, webrev will just say 'a binary file got removed', in any
>>>> case I'll take it as a 'yes, I'm going to remove these files as part of
>>>> 8240604', so thumbs up.
>>>>
>>>> -- Igor
>>>>
>>>>> On Mar 18, 2020, at 4:57 AM, Alexander Scherbatiy
>>>>> <alexander.scherba...@bell-sw.com
>>>>> <mailto:alexander.scherba...@bell-sw.com>> wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the updated fix:
>>>>>
>>>>> http://cr.openjdk.java.net/~alexsch/8240604/webrev.01
>>>>> <http://cr.openjdk.java.net/~alexsch/8240604/webrev.01>
>>>>>
>>>>> Utils.TEST_CLASS_PATH, Platform.jvmLibDir(), and /native flag are added
>>>>> to the CustomLauncherTest.java test. I also included TEST_NATIVE_PATH to
>>>>> the Utils lib.
>>>>>
>>>>> I have not found a history about CustomLauncherTest.sh script in
>>>>> launcher.c so I just updated the comment as "A minature launcher for use
>>>>> by CustomLauncherTest.java test" in the exelauncher.c file.
>>>>>
>>>>>
>>>>> The comment that I had about removing the linux-* and solaris-* binary
>>>>> files I wrote because it is not clear for what is the right way to
>>>>> include removed binary files into webrev.
>>>>>
>>>>> Could I just use "hg remove binary-fie" and run webrev to add the removed
>>>>> binary files into webrev?
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Alexander.
>>>>>
>>>>> On 17.03.2020 20:11, Igor Ignatyev wrote:
>>>>>> Hi Alexander,
>>>>>>
>>>>>> overall looks good to me, I have a few comments though:
>>>>>> - you can use Utils.TEST_CLASSPATH instead of
>>>>>> CustomLauncherTest.TEST_CLASSPATH
>>>>>> - CustomLauncherTest::findLibjvm can be simplified by use
>>>>>> Platform::jvmLibDir
>>>>>> - exelauncher.c has a comment which refers to the test as
>>>>>> CustomLauncherTest.sh, could you please update the comment?
>>>>>> - you have to add /native flag to @run action, otherwise jtreg won't
>>>>>> exclude this test from runs w/ test.nativepath being unset
>>>>>>
>>>>>> I also have a question regarding your statement that
>>>>>>>> The changes for obsolete binary files <...> are not included into the
>>>>>>>> webrev. They needs to be removed manually.
>>>>>> you are planning to remove these files as part of this patch, right?
>>>>>>
>>>>>> Thanks,
>>>>>> -- Igor
>>>>>>
>>>>>>
>>>>>>> On Mar 5, 2020, at 6:27 AM, Daniel Fuchs <daniel.fu...@oracle.com
>>>>>>> <mailto:daniel.fu...@oracle.com>> wrote:
>>>>>>>
>>>>>>> Hi Alexander,
>>>>>>>
>>>>>>> Fixes to JMX & management agent are reviewed on the
>>>>>>> seviceability-dev (added in to:) these days.
>>>>>>>
>>>>>>> best regards,
>>>>>>>
>>>>>>> -- daniel
>>>>>>>
>>>>>>> On 05/03/2020 13:17, Alexander Scherbatiy wrote:
>>>>>>>> Hello,
>>>>>>>> Could you review a small enhancement where the test CustomLauncherTest
>>>>>>>> is updated to build binary launcher file from launcher.c file.
>>>>>>>> The file launcher.c is renamed to exelauncher.c to follow the name
>>>>>>>> convention for executable test files building by jdk make system.
>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8240604
>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8240604>
>>>>>>>> Webrev: http://cr.openjdk.java.net/~alexsch/8240604/webrev.00
>>>>>>>> <http://cr.openjdk.java.net/~alexsch/8240604/webrev.00>
>>>>>>>> The changes for obsolete binary files from
>>>>>>>> sun/management/jmxremote/bootstrap/linux-* and solaris-* are not
>>>>>>>> included into the webrev. They needs to be removed manually.
>>>>>>>> The test is passed on Ubuntu 18.04 x86-64, Solaris Sparc v9 11.2, and
>>>>>>>> Solaris x64 11.4 systems.
>>>>>>>> The test is excluded from Windows and Mac Os X systems.
>>>>>>>> Thanks,
>>>>>>>> Alexander.
>>
