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