-
-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ignore JVMJ9VM090I message in java -version output #113
Conversation
Tested : internal/Grinder/14822 Note: the original issue which causes |
a170d70
to
aee6990
Compare
@[email protected] - please review. |
while (javaVersionOutput.startsWith("JVMJ9VM082E") || javaVersionOutput.startsWith("JIT:")) { | ||
// Eat superfluous first lines (e.g. JIT: env var TR_OPTIONS etc, or JVMJ9VM082E Unable to switch to IFA processor, | ||
// or JVMJ9VM090I Slow response to network query) | ||
while (javaVersionOutput.startsWith("JVMJ9VM082E") || javaVersionOutput.startsWith("JIT:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be more robust to skip anything that starts with "JVMJ9VM" rather than these specific messages. Not sure if there are other possibilities besides these two, and additional ones could be added in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see! Agreed. Updated PR.
aee6990
to
e92405e
Compare
Does it make sense to use |
The way this piece of STF code was implemented is suggestive of the fact that the people involved in coding it had decided to validate if java -version output starts with 'java version'. I have no knowledge of why they decided to do that. If we want to change this check's criteria, we can change it to whatever we want. We just need to come to an agreement. That's all. FYI @lumpfish |
@llxia : As discussed at the AQAvit call, could we deliver this PR so that it unblocks build from any reoccurrence of the issue reported in eclipse-openj9/openj9#12470? As the next step, I'll create a second issue to remove the version check altogether and update the logic of figuring out various Java versions in JavaVersion.java to depend on Java system properties instead of parsing |
Issue for second step mentioned above: #114 |
We are still technically in a code freeze for release week. |
That was a slip from my side! |
This PR is to Ignore JVMJ9VM090I messages in java -version output.
it should fix the related issue reported in to eclipse-openj9/openj9#12470. Meaning, next time we encounter network issues which cause
java -version
to outputJVMJ9VM090I
as below, STF would ignore it and continue.Signed-off-by: [email protected] [email protected]