Update developer guide reference to download JDK 14 #1452
Conversation
Signed-off-by: Andrew Ross <[email protected]>
|
✅ Gradle Wrapper Validation success 6cf9112 |
|
Can one of the admins verify this patch? |
|
✅ DCO Check Passed 6cf9112 |
|
✅ Gradle Precommit success 6cf9112 |
DEVELOPER_GUIDE.md
Outdated
| @@ -65,7 +65,7 @@ Download Java 11 from [here](https://adoptium.net/releases.html?variant=openjdk1 | |||
|
|
|||
| #### JDK 8 and 17 | |||
|
|
|||
| To run the full suite of tests, download and install [JDK 8](https://adoptium.net/releases.html?variant=openjdk8) and [17](https://adoptium.net/releases.html?variant=openjdk17) and set `JAVA8_HOME`, `JAVA11_HOME`, and `JAVA14_HOME`. They are required by the [backwards compatibility test](./TESTING.md#testing-backwards-compatibility). | |||
| To run the full suite of tests, download and install [JDK 8](https://adoptium.net/releases.html?variant=openjdk8) and [17](https://adoptium.net/releases.html?variant=openjdk17) and set `JAVA8_HOME`, `JAVA11_HOME`, and `JAVA17_HOME`. They are required by the [backwards compatibility test](./TESTING.md#testing-backwards-compatibility). | |||
There was a problem hiding this comment.
Hi @andrross, I think the environment variable JAVA14_HOME is used to build the artifact from 1.0 branch, for fully compatibility with Elasticsearch 7.10.2. (https://github.com/opensearch-project/OpenSearch/blob/1.0/DEVELOPER_GUIDE.md#jdk-14)
There was a problem hiding this comment.
On the main branch it seems that JAVA17_HOME is required and JAVA14_HOME is unused, so my update here is correct. Am I wrong about that?
There was a problem hiding this comment.
According to the doc: https://github.com/opensearch-project/OpenSearch/blob/1.1.0/TESTING.md#testing-backwards-compatibility
When running ./gradlew check, minimal bwc checks are also run against compatible versions that are not yet released.
Even run ./gradlew check on main branch, it will build artifact from 1.x, 1.1, 1.0 branches to run BWC tests 😁, I can double check this by running ./gradlew check after unset JAVA14_HOME.
There was a problem hiding this comment.
This is the console output:
> Task :distribution:bwc:bugfix:checkoutBwcBranch
Performing checkout of opensearch-project/1.0...
Checkout hash for :distribution:bwc:bugfix is a70cc74cbfe0690cf3b082147dccaef880425934
> Task :distribution:bwc:bugfix:buildBwcLinuxTar FAILED
...
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':distribution:bwc:bugfix:buildBwcLinuxTar'.
> JAVA14_HOME required
😐 It's a bit cumbersome to keep the same build requirement for version 1.0 with Elasticsearch.
There was a problem hiding this comment.
@dblock Should we change anything here? It is indeed true that JAVA14_HOME is required to run ./gradlew check (I even tried setting that env variable to a Java 17 JDK but there's a check that explicitly fails in that case). It seems not great to require contributors to install an end-of-life JDK.
There was a problem hiding this comment.
I'm suggesting that we change the older branches to allow them to build against Java 11 or 17 instead of requiring Java 14. I admit that I don't fully understand the implications of backporting such a change, though the benefit is that contributors would not be required to install an end-of-life and unsupported JDK.
There was a problem hiding this comment.
Changing what we have released feels wrong to me. Running bcw tests means using the JDK used for that release, and that's unfortunately 14.
There was a problem hiding this comment.
I've updated the document to specify a location to download JDK 14. Please let me know if there's a better place to link to. I've also removed reference to JDK17 and setting JAVA17_HOME because in my testing it is not required in order to get ./gradlew check to pass (I may be missing something, but the only time I see JDK 17 being used is if I explicitly specify -Druntime.java=17)
andrross
changed the title
|
✅ Gradle Wrapper Validation success 862b366 |
|
✅ Gradle Precommit success 862b366 |
Description
Update developer guide reference to download JDK 14
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.