Java version reproduction #32715
Merged
Java version reproduction #32715
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Provide the ability to control compiler and hava versions just by passing a property. The actual java home comes from the `JAVA<major>_HOME` env vars that we allready require. This works better with the Gradle daemon as well. Output is also changed a bit. for `-Druntime.java=8 -Dcompiler.java=9`: ``` ======================================= Elasticsearch Build Hamster says Hello! Gradle Version : 4.9 OS Info : Linux 4.17.8-1-ARCH (amd64) Compiler JDK Version : 11 (Oracle Corporation 11-ea [OpenJDK 64-Bit Server VM 11-ea+22]) Runtime JDK Version : 11 (Oracle Corporation 11-ea [OpenJDK 64-Bit Server VM 11-ea+22]) Gradle JDK Version : 10 (Oracle Corporation 10.0.1 [OpenJDK 64-Bit Server VM 10.0.1+10]) Compiler java.home : /home/alpar/opt/jdk-11-ea22/ Runtime java.home : /home/alpar/opt/jdk-11-ea22/ Gradle java.home : /usr/lib/jvm/java-10-openjdk Random Testing Seed : EA858533191E8DFB ======================================= ``` Without configuration: ``` ======================================= Elasticsearch Build Hamster says Hello! ======================================= Gradle Version : 4.9 OS Info : Linux 4.17.8-1-ARCH (amd64) JDK Version : 10 (Oracle Corporation 10.0.1 [OpenJDK 64-Bit Server VM 10.0.1+10]) JAVA_HOME : /usr/lib/jvm/java-10-openjdk Random Testing Seed : 4BD5B2A839C8FCA1 ======================================= ```
Here's how a reproduction line will look like (test made to fail): ``` ./gradlew :modules:lang-painless:test -Dtests.seed=2DA2379065A4EEAB -Dtests.class=org.elasticsearch.painless.AdditionTests -Dtests.method="testInt" -Dtests.security.manager=true -Dtests.locale=es-PE -Dtests.timezone=WET -Dcompiler.java=10 -Druntime.java=10 ```
So this works for FIPS as well by passing something like `-Druntime.java=8FIPS` to look for `JAVA8FIPS_HOME`
|
Pinging @elastic/es-core-infra |
rjernst
approved these changes
Aug 8, 2018
There was a problem hiding this comment.
I left some small suggestions. I've gone back and forth on whether it is better to pass back in via sys props or env vars. I personally use bash aliases right now to switch between these. But the issues with the gradle daemon, plus the better ability to copy/paste a repro line without the ./gradlew portion (I use an alias for that as well, giving its relative location), has convinced me this is better. My only remaining hesitation is about leaving support for env vars. Do you have thoughts on why we need to support both?
| println " JAVA_HOME (runtime) : ${runtimeJavaHome}" | ||
| println " Compiler JDK Version : ${getPaddedMajorVersion(compilerJavaVersionEnum)} (${compilerJavaVersionDetails})" | ||
| println " Runtime JDK Version : ${getPaddedMajorVersion(runtimeJavaVersionEnum)} (${runtimeJavaVersionDetails})" | ||
| println " Gradle JDK Version : ${getPaddedMajorVersion(JavaVersion.toVersion(gradleJavaVersion))} (${gradleJavaVersionDetails})" |
There was a problem hiding this comment.
I think the previous grouping was better. Having both gradle java version and java home next to each other is visually easier to look at, because you would often want to look at the version and home path together.
|
|
||
| // enforce Java version | ||
| if (compilerJavaVersionEnum < minimumCompilerVersion) { | ||
| final String message = | ||
| "the environment variable JAVA_HOME must be set to a JDK installation directory for Java ${minimumCompilerVersion}" + | ||
| "the Compiler java.home must be set to a JDK installation directory for Java ${minimumCompilerVersion}" + |
There was a problem hiding this comment.
nit: I don't think we need to capitalize "Compiler" here, or "Runtime" below
| @@ -271,7 +290,10 @@ class BuildPlugin implements Plugin<Project> { | |||
| } | |||
|
|
|||
| private static String findRuntimeJavaHome(final String compilerJavaHome) { | |||
| assert compilerJavaHome != null | |||
| def runtimeJavaProperty = System.getProperty("runtime.java") | |||
There was a problem hiding this comment.
nit: def -> String
ditto everywhere else def is used
| @@ -150,6 +150,10 @@ public ReproduceErrorMessageBuilder appendESProperties() { | |||
| appendOpt("tests.locale", Locale.getDefault().toLanguageTag()); | |||
| appendOpt("tests.timezone", TimeZone.getDefault().getID()); | |||
| appendOpt("tests.distribution", System.getProperty("tests.distribution")); | |||
| appendOpt("compiler.java", System.getProperty("compiler.java")); | |||
| appendOpt("runtime.java", System.getProperty("runtime.java")); | |||
| appendOpt("javax.net.ssl.keyStorePassword", System.getProperty("javax.net.ssl.keyStorePassword")); | |||
There was a problem hiding this comment.
I don't think we should append these unless we are in a fips jvm.
There was a problem hiding this comment.
We only append if the props are set, and they are set only for fips.
|
I'm ok to remove the env vars any time after this is merged. We'll need to update CI and Rally at a minimum. |
alpar-t
added a commit
that referenced
this pull request
Aug 10, 2018
Enhance reproduction line with info about jdks Provide the ability to control compiler and hava versions just by passing a property. The actual java home comes from the `JAVA<major>_HOME` env vars that we allready require. This works better with the Gradle daemon as well. Output is also changed a bit. for `-Druntime.java=8 -Dcompiler.java=9`: ``` ======================================= Elasticsearch Build Hamster says Hello! Gradle Version : 4.9 OS Info : Linux 4.17.8-1-ARCH (amd64) Compiler JDK Version : 11 (Oracle Corporation 11-ea [OpenJDK 64-Bit Server VM 11-ea+22]) Runtime JDK Version : 11 (Oracle Corporation 11-ea [OpenJDK 64-Bit Server VM 11-ea+22]) Gradle JDK Version : 10 (Oracle Corporation 10.0.1 [OpenJDK 64-Bit Server VM 10.0.1+10]) Compiler java.home : /home/alpar/opt/jdk-11-ea22/ Runtime java.home : /home/alpar/opt/jdk-11-ea22/ Gradle java.home : /usr/lib/jvm/java-10-openjdk Random Testing Seed : EA858533191E8DFB ======================================= ``` Without configuration: ``` ======================================= Elasticsearch Build Hamster says Hello! ======================================= Gradle Version : 4.9 OS Info : Linux 4.17.8-1-ARCH (amd64) JDK Version : 10 (Oracle Corporation 10.0.1 [OpenJDK 64-Bit Server VM 10.0.1+10]) JAVA_HOME : /usr/lib/jvm/java-10-openjdk Random Testing Seed : 4BD5B2A839C8FCA1 ======================================= ``` Here's how a reproduction line will look like (test made to fail): ``` ./gradlew :modules:lang-painless:test -Dtests.seed=2DA2379065A4EEAB -Dtests.class=org.elasticsearch.painless.AdditionTests -Dtests.method="testInt" -Dtests.security.manager=true -Dtests.locale=es-PE -Dtests.timezone=WET -Dcompiler.java=10 -Druntime.java=10 ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Provide the ability to control compiler and hava versions just by
passing a property. The actual java home comes from the
JAVA<major>_HOMEenv vars that we allready require.This works better with the Gradle daemon as well.
Output is also changed a bit.
for
-Druntime.java=8 -Dcompiler.java=9:Without configuration:
Here's how a reproduction line will look like (test made to fail):
This is extended so it works for FIPS as well, as long as one has the
JAVA8FIPS_HOMEetc env vars set. so with this change, one doesn't need to edit the reproduction line to reproduce fips failures.Closes #31898