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.
> 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> wrote: > > Hello, > > Could you review the updated fix: > > 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> 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 >>>> Webrev: 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.
