Require JDK 9 for compilation #28071

jasontedor · 2018-01-04T04:46:31Z

This commit modifies the build to require JDK 9 for compilation. Henceforth, we will compile with a JDK 9 compiler targeting JDK 8 as the class file format. Optionally, RUNTIME_JAVA_HOME can be set as the runtime JDK used for running tests. To enable this change, we separate the meaning of the compiler Java home versus the runtime Java home. If the runtime Java home is not set (via RUNTIME_JAVA_HOME) then we fallback to using JAVA_HOME as the runtime Java home. This enables:

developers only have to set one Java home (JAVA_HOME)
developers can set an optional Java home (RUNTIME_JAVA_HOME) to test on the
minimum supported runtime
we can test compiling with JDK 9 running on JDK 8 and compiling with JDK 9 running on JDK 9 in CI

Relates #28051

This commit modifies the build to require JDK 9 for compilation. Henceforth, we will compile with a JDK 9 compiler targeting JDK 8 as the class file format. Optionally, JAVA_8_HOME can be set as the runtime JDK used for running tests. To enable this change, we separate the meaning of the compiler Java home versus the runtime Java home. If the runtime Java home is not set (via JAVA_8_HOME) then we fallback to using JAVA_HOME as the runtime Java home. This enables: - developers only have to set one Java home (JAVA_HOME) - developers can set an optional Java home (JAVA_8_HOME) to test on the minimum supported runtime - we can test compiling with JDK 9 running on JDK 8 and compiling with JDK 9 running on JDK 9 in CI

colings86 · 2018-01-04T11:21:29Z

I tested this PR with Eclipse IDE and can confirm that it causes no problems for my dev environment

ywelsch

I've left one suggestion. I've not tested the change but looks good to me.

ywelsch · 2018-01-04T15:51:43Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

+        final String maybeJavaHome
+        if (java8Home == null) {
+            // if JAVA_8_HOME is not set fallback to JAVA_HOME
+            maybeJavaHome = System.getenv('JAVA_HOME')


maybe simpler to let the method findCompilerJavaHome stay named findJavaHome and just call maybeJavaHome = findJavaHome() here? This means we don't have to duplicate all the logic around idea.active etc. Yet another alternative would be to pass a fallback parameter to findRuntimeJavaHome.

* master: Set the elasticsearch-nio codebase for tests (elastic#28067) Bump compat version for local depdendent test to 6.2.0 Pass `java.locale.providers=COMPAT` to Java 9 onwards (elastic#28080) Allow shrinking of indices from a previous major (elastic#28076) Remove deprecated exceptions (elastic#28059) Add Writeable.Reader support to TransportResponseHandler (elastic#28010) Plugins: Add plugin extension capabilities (elastic#27881)

* master: test: replaced try-catch statements with expectThrows(...) Add getWarmer and getTranslog method to NodeIndicesStats (elastic#28092) fix doc mistake Added ASN support for Ingest GeoIP plugin. Fix global aggregation that requires breadth first and scores (elastic#27942) Introduce Gradle wrapper Ignore GIT_COMMIT when calculating commit hash Re-enable bwc tests after elastic#27881 was backported

rjernst · 2018-01-05T21:53:50Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

@@ -182,6 +180,34 @@ class BuildPlugin implements Plugin<Project> {
        return javaHome
    }

+    private static String findRuntimeJavaHome(Project project) {
+        String java8Home = System.getenv('JAVA_8_HOME')


I think we should use a more explicit name here, like ES_TEST_JAVA_HOME for a couple reasons:

I personally have env vars for each version of java, so I can quickly switch to that java version with an alias. I always have this env set, so it would mean for me I would always be testing against java 8.

This doesn't have to be java 8 specific. The same feature of our build can be used to test against a different jvm like azul or ibm, while still building with the official java 9 jdk we use for releases.

I discussed this with @rjernst and after some consideration I agree. We agreed to name the variable RUNTIME_JAVA_HOME though.

* master: Remove Gradle cheatsheet Fix reproduction info to point to Gradle wrapper Update platforms tests to use Gradle wrapper Update testing docs to reflect Gradle wrapper Painless: Modify Loader to Load Classes Directly from Definition (elastic#28088) Update contributing docs to use the Gradle wrapper Create nio-transport plugin for NioTransport (elastic#27949)

* master: Use Gradle wrapper when building BWC Painless: Add a simple cache for whitelist methods and fields. (elastic#28142) Fix upgrading indices which use a custom similarity plugin. (elastic#26985) Fix Licenses values for CDDL and Custom URL (elastic#27999) Cleanup TcpChannelFactory and remove classes (elastic#28102) Fix expected plugins test for transport-nio [Docs] Fix Date Math example descriptions (elastic#28125) Fail rollover if duplicated alias found in template (elastic#28110) Avoid concurrent snapshot finalizations when deleting an INIT snapshot (elastic#28078) Deprecate `isShardsAcked()` in favour of `isShardsAcknowledged()` (elastic#27819) [TEST] Wait for replicas to be allocated before shrinking Use the underlying connection version for CCS connections (elastic#28093) test: do not use asn fields Test: Add assumeFalse for test that cannot pass on windows Clarify reproduce info on Windows Remove out-of-date projectile file

* master: Fix Gradle wrapper usage on Windows when building BWC (elastic#28146) [Docs] Fix some typos in comments (elastic#28098)

* master: Set watermarks in single-node test cases Add the ability to bundle multiple plugins into a meta plugin (elastic#28022) Declare empty package dirs as output dirs Consistent updates of IndexShardSnapshotStatus (elastic#28130)

* master: (30 commits) Fix lock accounting in releasable lock Add ability to associate an ID with tasks (elastic#27764) [DOCS] Removed differencies between text and code (elastic#27993) text fixes (elastic#28136) Update getting-started.asciidoc (elastic#28145) [Docs] Spelling fix in painless-getting-started.asciidoc (elastic#28187) Fixed the cat.health REST test to accept 4ms, not just 4.0ms (elastic#28186) Do not keep 5.x commits once having 6.x commits (elastic#28188) Rename core module to server (elastic#28180) upgraded jna from 4.4.0-1 to 4.5.1 (elastic#28183) [TEST] Do not call RandomizedTest.scaledRandomIntBetween from multiple threads Primary send safe commit in file-based recovery (elastic#28038) [Docs] Correct response json in rank-eval.asciidoc Add scroll parameter to _reindex API (elastic#28041) Include all sentences smaller than fragment_size in the unified highlighter (elastic#28132) Modifies the JavaAPI docs related to AggregationBuilder [Docs] Improvements in script-fields.asciidoc (elastic#28174) [Docs] Remove Kerberos/SPNEGO Shield plugin (elastic#28019) Ignore null value for range field (elastic#27845) (elastic#28116) Fix environment variable substitutions in list setting (elastic#28106) ...

* master: TEST: init unassigned gcp in testAcquireIndexCommit Replica start peer recovery with safe commit (elastic#28181) Truncate tlog cli should assign global checkpoint (elastic#28192)

* master: (21 commits) [GEO] Add WKT Support to GeoBoundingBoxQueryBuilder Painless: Add whitelist extensions (elastic#28161) Fix daitch_mokotoff phonetic filter to use the dedicated Lucene filter (elastic#28225) Avoid doing redundant work when checking for self references. (elastic#26927) Fix casts in HotThreads. (elastic#27578) Ignore the `-snapshot` suffix when comparing the Lucene version in the build and the docs. (elastic#27927) Allow update of `eager_global_ordinals` on `_parent`. (elastic#28014) Fix NPE on composite aggregation with sub-aggregations that need scores (elastic#28129) `MockTcpTransport` to connect asynchronously (elastic#28203) Fix synonym phrase query expansion for cross_fields parsing (elastic#28045) Introduce elasticsearch-core jar (elastic#28191) elastic#28218: Update the Lucene version for 6.2.0 after backport upgrade to lucene 7.2.1 (elastic#28218) [Docs] Fix an error in painless-types.asciidoc (elastic#28221) Adds metadata to rewritten aggregations (elastic#28185) Update version of TaskInfo header serialization after backport TEST: Tightens file-based condition in peer-recovery Correct backport replica rollback to 6.2 (elastic#28181) Backport replica rollback to 6.2 (elastic#28181) Rename deleteLocalTranslog to createNewTranslog ...

rjernst · 2018-01-15T23:42:15Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

                javaHome = Jvm.current().javaHome
            } else {
-                throw new GradleException('JAVA_HOME must be set to build Elasticsearch')


Why was this exception changed to an assertion? Is it moved somewhere else I'm missing?

I am glad that you caught this. This was inadvertent, it arose in a refactoring I did in 95475f9 which refactored two basically identical methods from an earlier version of this pull request (findCompilerJavaHome and findRuntimeJavaHome) into one (findJavaHome). Before the refactoring, the second method did not need such a line because by the time it had executed the first one would have already thrown that exception (hence the assert false). When I refactored, I must have based by work on findRuntimeJavaHome inadvertently losing this exception. I have pushed: bcee920.

rjernst

LGTM

rjernst · 2018-01-16T01:58:25Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

-                    assert minimumJava == JavaVersion.VERSION_1_8
-                    options.compilerArgs << '--release' << '8'
-                }
+                options.compilerArgs << '--release' << project.targetCompatibility.majorVersion


Can you add a comment to this line linking the relevant gradle issue for adding builtin support for this: gradle/gradle#2510

I pushed 9434717.

* master: [Docs] Fix base directory to include for put_mapping.asciidoc Open engine should keep only starting commit (elastic#28228) [Docs] Fix Java Api index administration usage (elastic#28133) Fix eclipse build. (elastic#28236) Never return null from Strings.tokenizeToStringArray (elastic#28224) Fallback to TransportMasterNodeAction for cluster health retries (elastic#28195) [Docs] Changes to ingest.asciidoc (elastic#28212) TEST: Update logging for testAckedIndexing

* master: Revert "[Docs] Fix Java Api index administration usage (elastic#28133)" Revert "[Docs] Fix base directory to include for put_mapping.asciidoc" Added multi get api to the high level rest client. [Docs] Clarify numeric datatype ranges (elastic#28240)

This commit modifies the build to require JDK 9 for compilation. Henceforth, we will compile with a JDK 9 compiler targeting JDK 8 as the class file format. Optionally, RUNTIME_JAVA_HOME can be set as the runtime JDK used for running tests. To enable this change, we separate the meaning of the compiler Java home versus the runtime Java home. If the runtime Java home is not set (via RUNTIME_JAVA_HOME) then we fallback to using JAVA_HOME as the runtime Java home. This enables: - developers only have to set one Java home (JAVA_HOME) - developers can set an optional Java home (RUNTIME_JAVA_HOME) to test on the minimum supported runtime - we can test compiling with JDK 9 running on JDK 8 and compiling with JDK 9 running on JDK 9 in CI Relates #28071

* master: (35 commits) Move the multi-get response tests to server Require JDK 9 for compilation (elastic#28071) Revert "[Docs] Fix Java Api index administration usage (elastic#28133)" Revert "[Docs] Fix base directory to include for put_mapping.asciidoc" Added multi get api to the high level rest client. [Docs] Clarify numeric datatype ranges (elastic#28240) [Docs] Fix base directory to include for put_mapping.asciidoc Open engine should keep only starting commit (elastic#28228) [Docs] Fix Java Api index administration usage (elastic#28133) Fix eclipse build. (elastic#28236) Never return null from Strings.tokenizeToStringArray (elastic#28224) Fallback to TransportMasterNodeAction for cluster health retries (elastic#28195) [Docs] Changes to ingest.asciidoc (elastic#28212) TEST: Update logging for testAckedIndexing [GEO] Add WKT Support to GeoBoundingBoxQueryBuilder Painless: Add whitelist extensions (elastic#28161) Fix daitch_mokotoff phonetic filter to use the dedicated Lucene filter (elastic#28225) Avoid doing redundant work when checking for self references. (elastic#26927) Fix casts in HotThreads. (elastic#27578) Ignore the `-snapshot` suffix when comparing the Lucene version in the build and the docs. (elastic#27927) ...

* master: Fix third-party audit tasks on JDK 8 Remove duplicated javadoc `fieldType` param Handle 5.6.6 and 6.1.2 release Introduce multi-release JAR Move the multi-get response tests to server Require JDK 9 for compilation (#28071) Revert "[Docs] Fix Java Api index administration usage (#28133)" Revert "[Docs] Fix base directory to include for put_mapping.asciidoc" Added multi get api to the high level rest client. [Docs] Clarify numeric datatype ranges (#28240) [Docs] Fix base directory to include for put_mapping.asciidoc Open engine should keep only starting commit (#28228)

* 6.x: Fix third-party audit tasks on JDK 8 Remove duplicated javadoc `fieldType` param Handle 5.6.6 and 6.1.2 release Introduce multi-release JAR Move the multi-get response tests to server Require JDK 9 for compilation (#28071) Revert "[Docs] Fix Java Api index administration usage (#28133)" Revert "[Docs] Fix base directory to include for put_mapping.asciidoc" Added multi get api to the high level rest client. Open engine should keep only starting commit (#28228) [Docs] Clarify numeric datatype ranges (#28240) Add migration guide for 6.1 [Docs] Fix base directory to include for put_mapping.asciidoc [Docs] Fix Java Api index administration usage (#28133) Add 6.1.2 release notes

jasontedor added :Delivery/Build Build or test infrastructure >enhancement v6.2.0 v7.0.0 labels Jan 4, 2018

jasontedor requested review from nik9000, rjernst, hub-cap and ywelsch January 4, 2018 04:46

Update docs

b92fdf3

Fix reindex from remote tests

fab460e

jasontedor force-pushed the compile-with-jdk-9 branch from 7aabf0a to fab460e Compare January 4, 2018 16:21

ywelsch approved these changes Jan 4, 2018

View reviewed changes

jasontedor added 2 commits January 4, 2018 12:01

danielmitterdorfer mentioned this pull request Jan 5, 2018

Require JDK 9 for source builds elastic/rally#387

Closed

jasontedor closed this Jan 5, 2018

jasontedor deleted the compile-with-jdk-9 branch January 5, 2018 15:14

jasontedor restored the compile-with-jdk-9 branch January 5, 2018 15:14

jasontedor reopened this Jan 5, 2018

rjernst reviewed Jan 5, 2018

View reviewed changes

jasontedor added 2 commits January 5, 2018 19:24

Runtime Java home

95475f9

jasontedor mentioned this pull request Jan 8, 2018

Use Gradle wrapper when building BWC #28138

Merged

jasontedor added 5 commits January 8, 2018 21:50

Use JDK 8 if available when building legacy

8d33402

Merge branch 'master' into compile-with-jdk-9

6ca2834

* master: Fix Gradle wrapper usage on Windows when building BWC (elastic#28146) [Docs] Fix some typos in comments (elastic#28098)

Merge branch 'master' into compile-with-jdk-9

a79c2c1

* master: Set watermarks in single-node test cases Add the ability to bundle multiple plugins into a meta plugin (elastic#28022) Declare empty package dirs as output dirs Consistent updates of IndexShardSnapshotStatus (elastic#28130)

jasontedor added 2 commits January 13, 2018 13:42

Merge branch 'master' into compile-with-jdk-9

f3a76a7

* master: TEST: init unassigned gcp in testAcquireIndexCommit Replica start peer recovery with safe commit (elastic#28181) Truncate tlog cli should assign global checkpoint (elastic#28192)

rjernst reviewed Jan 15, 2018

View reviewed changes

Refactor Java home

bcee920

rjernst approved these changes Jan 16, 2018

View reviewed changes

jasontedor added 4 commits January 15, 2018 21:03

In case 6.0 is compiled

55d2b1b

Add comment on --release flag

9434717

jasontedor merged commit 0a79555 into elastic:master Jan 16, 2018

jasontedor deleted the compile-with-jdk-9 branch January 16, 2018 20:11

DaveCTurner mentioned this pull request Jan 17, 2018

add toString implementation for UpdateRequest. #27997

Merged

jasontedor mentioned this pull request Jan 17, 2018

Build is buggy for java 9. #18039

Closed

jakelandis mentioned this pull request Jan 17, 2018

[META] Elasticsearch now requires Java 9 for compilation elastic/logstash#8976

Closed

colings86 removed the >enhancement label Jan 22, 2018

jaymode mentioned this pull request Feb 9, 2018

updating to 6.2.1 has issues elastic/shield-custom-realm-example#30

Closed

DaveCTurner mentioned this pull request Mar 9, 2018

Use RUNTIME_JAVA_HOME for benchmarks #28961

Closed

jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Require JDK 9 for compilation #28071

Require JDK 9 for compilation #28071

jasontedor commented Jan 4, 2018 •

edited

Loading

colings86 commented Jan 4, 2018 •

edited

Loading

ywelsch left a comment

ywelsch Jan 4, 2018

rjernst Jan 5, 2018

jasontedor Jan 5, 2018

rjernst Jan 15, 2018

jasontedor Jan 16, 2018 •

edited

Loading

rjernst left a comment

rjernst Jan 16, 2018

jasontedor Jan 16, 2018

Require JDK 9 for compilation #28071

Require JDK 9 for compilation #28071

Conversation

jasontedor commented Jan 4, 2018 • edited Loading

colings86 commented Jan 4, 2018 • edited Loading

ywelsch left a comment

Choose a reason for hiding this comment

ywelsch Jan 4, 2018

Choose a reason for hiding this comment

rjernst Jan 5, 2018

Choose a reason for hiding this comment

jasontedor Jan 5, 2018

Choose a reason for hiding this comment

rjernst Jan 15, 2018

Choose a reason for hiding this comment

jasontedor Jan 16, 2018 • edited Loading

Choose a reason for hiding this comment

rjernst left a comment

Choose a reason for hiding this comment

rjernst Jan 16, 2018

Choose a reason for hiding this comment

jasontedor Jan 16, 2018

Choose a reason for hiding this comment

jasontedor commented Jan 4, 2018 •

edited

Loading

colings86 commented Jan 4, 2018 •

edited

Loading

jasontedor Jan 16, 2018 •

edited

Loading