From 7163f4e767f8efb3ff4e4e9a55e766d1e2c16e72 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 22 Apr 2020 16:34:39 -0700 Subject: [PATCH 1/3] Simplify java home verification At one time, all uses of java home were found through the getJavaHome utility method on BuildPlugin. However, that was changed many refactorings ago, but the complex support for registering a java home version needed that fails at configuration time still exists. The only remaining use of grabbing java home is within bwc tests, and must be at runtime since that is when we have the checkout and know what version is needed. This commit consolidates the java home finding method into a utility unassociated with BuildPlugin. --- .../elasticsearch/gradle/BuildPlugin.groovy | 54 ------------------- .../elasticsearch/gradle/util/JavaUtil.java | 46 ++++++++++++++++ distribution/bwc/build.gradle | 2 +- modules/reindex/build.gradle | 2 - 4 files changed, 47 insertions(+), 57 deletions(-) create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index 323ffaf5338c8..93110777d0753 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -182,60 +182,6 @@ class BuildPlugin implements Plugin { } } - /** Add a check before gradle execution phase which ensures java home for the given java version is set. */ - static void requireJavaHome(Task task, int version) { - // use root project for global accounting - Project rootProject = task.project.rootProject - ExtraPropertiesExtension extraProperties = rootProject.extensions.extraProperties - - // hacky way (but the only way) to find if the task graph has already been populated - boolean taskGraphReady - try { - rootProject.gradle.taskGraph.getAllTasks() - taskGraphReady = true - } catch (IllegalStateException) { - taskGraphReady = false - } - - if (taskGraphReady) { - // check directly if the version is present since we are already executing - if (BuildParams.javaVersions.find { it.version == version } == null) { - throw new GradleException("JAVA${version}_HOME required to run task:\n${task}") - } - } else { - // setup list of java versions we will check at the end of configuration time - if (extraProperties.has('requiredJavaVersions') == false) { - extraProperties.set('requiredJavaVersions', [:]) - rootProject.gradle.taskGraph.whenReady { TaskExecutionGraph taskGraph -> - List messages = [] - Map> requiredJavaVersions = (Map>) extraProperties.get('requiredJavaVersions') - for (Map.Entry> entry : requiredJavaVersions) { - if (BuildParams.javaVersions.any { it.version == entry.key }) { - continue - } - List tasks = entry.value.findAll { taskGraph.hasTask(it) }.collect { " ${it.path}".toString() } - if (tasks.isEmpty() == false) { - messages.add("JAVA${entry.key}_HOME required to run tasks:\n${tasks.join('\n')}".toString()) - } - } - if (messages.isEmpty() == false) { - throw new GradleException(messages.join('\n')) - } - } - } - Map> requiredJavaVersions = (Map>) extraProperties.get('requiredJavaVersions') - requiredJavaVersions.putIfAbsent(version, []) - requiredJavaVersions.get(version).add(task) - } - } - - /** A convenience method for getting java home for a version of java and requiring that version for the given task to execute */ - static String getJavaHome(final Task task, final int version) { - requireJavaHome(task, version) - JavaHome java = BuildParams.javaVersions.find { it.version == version } - return java == null ? null : java.javaHome.get().absolutePath - } - /** * Makes dependencies non-transitive. * diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java b/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java new file mode 100644 index 0000000000000..23e1430c0a433 --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java @@ -0,0 +1,46 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.gradle.util; + +import org.elasticsearch.gradle.info.BuildParams; +import org.elasticsearch.gradle.info.JavaHome; +import org.gradle.api.GradleException; +import org.gradle.api.Project; +import org.gradle.api.Task; +import org.gradle.api.execution.TaskExecutionGraph; +import org.gradle.api.plugins.ExtraPropertiesExtension; + +import java.util.List; +import java.util.Map; +import java.util.Optional; + +public class JavaUtil { + + /** A convenience method for getting java home for a version of java and requiring that version for the given task to execute */ + static String getJavaHome(final Task task, final int version) { + List javaHomes = BuildParams.getJavaVersions(); + Optional java = javaHomes.stream().filter(j -> j.getVersion() == version).findFirst(); + // check directly if the version is present since we are already executing + if (java.isEmpty()) { + throw new GradleException("JAVA" + version + "_HOME required to run task " + task.getPath()); + } + return java.get().getJavaHome().get().getAbsolutePath(); + } +} diff --git a/distribution/bwc/build.gradle b/distribution/bwc/build.gradle index dd2145b53a339..b313ed68c4f2f 100644 --- a/distribution/bwc/build.gradle +++ b/distribution/bwc/build.gradle @@ -27,7 +27,7 @@ import org.gradle.util.GradleVersion import java.nio.charset.StandardCharsets -import static org.elasticsearch.gradle.BuildPlugin.getJavaHome +import static org.elasticsearch.gradle.util.JavaUtil.getJavaHome /** * We want to be able to do BWC tests for unreleased versions without relying on and waiting for snapshots. diff --git a/modules/reindex/build.gradle b/modules/reindex/build.gradle index 741688f5d224f..df28321729033 100644 --- a/modules/reindex/build.gradle +++ b/modules/reindex/build.gradle @@ -22,8 +22,6 @@ import org.elasticsearch.gradle.Architecture import org.elasticsearch.gradle.OS import org.elasticsearch.gradle.info.BuildParams -import static org.elasticsearch.gradle.BuildPlugin.getJavaHome - apply plugin: 'elasticsearch.test-with-dependencies' apply plugin: 'elasticsearch.jdk-download' From f8d516f86649d8c359b779dbe3c03249dd88f511 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 22 Apr 2020 16:48:12 -0700 Subject: [PATCH 2/3] fix checkstyle --- .../src/main/java/org/elasticsearch/gradle/util/JavaUtil.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java b/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java index 23e1430c0a433..89e5f369b2a49 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java @@ -22,13 +22,9 @@ import org.elasticsearch.gradle.info.BuildParams; import org.elasticsearch.gradle.info.JavaHome; import org.gradle.api.GradleException; -import org.gradle.api.Project; import org.gradle.api.Task; -import org.gradle.api.execution.TaskExecutionGraph; -import org.gradle.api.plugins.ExtraPropertiesExtension; import java.util.List; -import java.util.Map; import java.util.Optional; public class JavaUtil { From 8413b5d3b804db1fd6fa687cb212a8e6655f3d06 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 22 Apr 2020 20:30:41 -0700 Subject: [PATCH 3/3] address feedback --- .../java/org/elasticsearch/gradle/util/JavaUtil.java | 9 ++------- distribution/bwc/build.gradle | 4 ++-- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java b/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java index 89e5f369b2a49..4157bd5bb9b56 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/util/JavaUtil.java @@ -22,7 +22,6 @@ import org.elasticsearch.gradle.info.BuildParams; import org.elasticsearch.gradle.info.JavaHome; import org.gradle.api.GradleException; -import org.gradle.api.Task; import java.util.List; import java.util.Optional; @@ -30,13 +29,9 @@ public class JavaUtil { /** A convenience method for getting java home for a version of java and requiring that version for the given task to execute */ - static String getJavaHome(final Task task, final int version) { + static String getJavaHome(final int version) { List javaHomes = BuildParams.getJavaVersions(); Optional java = javaHomes.stream().filter(j -> j.getVersion() == version).findFirst(); - // check directly if the version is present since we are already executing - if (java.isEmpty()) { - throw new GradleException("JAVA" + version + "_HOME required to run task " + task.getPath()); - } - return java.get().getJavaHome().get().getAbsolutePath(); + return java.orElseThrow(() -> new GradleException("JAVA" + version + "_HOME required")).getJavaHome().get().getAbsolutePath(); } } diff --git a/distribution/bwc/build.gradle b/distribution/bwc/build.gradle index b313ed68c4f2f..eb5d9bfa87ccd 100644 --- a/distribution/bwc/build.gradle +++ b/distribution/bwc/build.gradle @@ -166,7 +166,7 @@ bwcVersions.forPreviousUnreleased { BwcVersions.UnreleasedVersionInfo unreleased List lines = file("${checkoutDir}/.ci/java-versions.properties").readLines() environment( 'JAVA_HOME', - getJavaHome(it, Integer.parseInt( + getJavaHome(Integer.parseInt( lines .findAll({ it.startsWith("ES_BUILD_JAVA=") }) .collect({ it.replace("ES_BUILD_JAVA=java", "").trim() }) @@ -176,7 +176,7 @@ bwcVersions.forPreviousUnreleased { BwcVersions.UnreleasedVersionInfo unreleased ) environment( 'RUNTIME_JAVA_HOME', - getJavaHome(it, Integer.parseInt( + getJavaHome(Integer.parseInt( lines .findAll({ it.startsWith("ES_RUNTIME_JAVA=java") }) .collect({ it.replace("ES_RUNTIME_JAVA=java", "").trim() })