From 9e0c77a358f3d20c4fb95b443b36e850e3490c1f Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Thu, 14 Feb 2019 15:39:42 -0800 Subject: [PATCH 1/2] Improve testcluster distribution artifact handling This commit moves validation logic for ensuring our testclusters configuration doesn't contain unexpected artifacts into the plugin itself. This change allows us to remove the custom copy task implementation altogether. Additionally, the error message has been improved to display component ids in addition to the artifacts to make it easier to figure out what actual dependency is at fault. --- .../SyncTestClustersConfiguration.java | 77 ------------------- .../testclusters/TestClustersPlugin.java | 40 ++++++---- 2 files changed, 26 insertions(+), 91 deletions(-) delete mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/SyncTestClustersConfiguration.java diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/SyncTestClustersConfiguration.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/SyncTestClustersConfiguration.java deleted file mode 100644 index d1a86a38c66ff..0000000000000 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/SyncTestClustersConfiguration.java +++ /dev/null @@ -1,77 +0,0 @@ -/* - * 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.testclusters; - -import org.gradle.api.DefaultTask; -import org.gradle.api.Project; -import org.gradle.api.file.FileCollection; -import org.gradle.api.tasks.InputFiles; -import org.gradle.api.tasks.OutputDirectory; -import org.gradle.api.tasks.TaskAction; - -import java.io.File; -import java.util.Set; -import java.util.stream.Collectors; - -public class SyncTestClustersConfiguration extends DefaultTask { - - @InputFiles - public FileCollection getDependencies() { - Set nonZip = getProject().getConfigurations() - .getByName(TestClustersPlugin.HELPER_CONFIGURATION_NAME) - .getFiles() - .stream() - .filter(file -> file.getName().endsWith(".zip") == false) - .collect(Collectors.toSet()); - if(nonZip.isEmpty() == false) { - throw new IllegalStateException("Expected only zip files in configuration : " + - TestClustersPlugin.HELPER_CONFIGURATION_NAME + " but found " + - nonZip - ); - } - return getProject().files( - getProject().getConfigurations() - .getByName(TestClustersPlugin.HELPER_CONFIGURATION_NAME) - .getFiles() - ); - } - - @OutputDirectory - public File getOutputDir() { - return getTestClustersConfigurationExtractDir(getProject()); - } - - @TaskAction - public void doExtract() { - File outputDir = getOutputDir(); - getProject().delete(outputDir); - outputDir.mkdirs(); - getDependencies().forEach(dep -> - getProject().copy(spec -> { - spec.from(getProject().zipTree(dep)); - spec.into(new File(outputDir, "zip")); - }) - ); - } - - static File getTestClustersConfigurationExtractDir(Project project) { - return new File(TestClustersPlugin.getTestClustersBuildDir(project), "extract"); - } - -} diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java index 3abc9a6a6177e..3512c2d6b875d 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java @@ -25,24 +25,19 @@ import org.gradle.api.Project; import org.gradle.api.Task; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.component.ComponentArtifactIdentifier; import org.gradle.api.execution.TaskActionListener; import org.gradle.api.execution.TaskExecutionListener; +import org.gradle.api.file.FileTree; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.plugins.ExtraPropertiesExtension; +import org.gradle.api.tasks.Sync; import org.gradle.api.tasks.TaskState; import java.io.File; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.TimeUnit; +import java.util.*; +import java.util.concurrent.*; import java.util.stream.Collectors; public class TestClustersPlugin implements Plugin { @@ -87,6 +82,20 @@ public void apply(Project project) { "Internal helper configuration used by cluster configuration to download " + "ES distributions and plugins." ); + helperConfiguration.getIncoming().afterResolve(resolvableDependencies -> { + Set nonZipComponents = resolvableDependencies.getArtifacts() + .getArtifacts() + .stream() + .filter(artifact -> artifact.getFile().getName().endsWith(".zip") == false) + .map(artifact -> artifact.getId()) + .collect(Collectors.toSet()); + + if(nonZipComponents.isEmpty() == false) { + throw new IllegalStateException("Dependencies with non-zip artifacts found in configuration '" + + TestClustersPlugin.HELPER_CONFIGURATION_NAME + "': " + nonZipComponents + ); + } + }); // When running in the Daemon it's possible for this to hold references to past usedClusters.clear(); @@ -98,7 +107,10 @@ public void apply(Project project) { // the clusters will look for artifacts there based on the naming conventions. // Tasks that use a cluster will add this as a dependency automatically so it's guaranteed to run early in // the build. - rootProject.getTasks().create(SYNC_ARTIFACTS_TASK_NAME, SyncTestClustersConfiguration.class); + rootProject.getTasks().create(SYNC_ARTIFACTS_TASK_NAME, Sync.class, sync -> { + sync.from((Callable>) () -> helperConfiguration.getFiles().stream().map(project::zipTree).collect(Collectors.toList())); + sync.into(new File(getTestClustersConfigurationExtractDir(project), "zip")); + }); // When we know what tasks will run, we claim the clusters of those task to differentiate between clusters // that are defined in the build script and the ones that will actually be used in this invocation of gradle @@ -129,7 +141,7 @@ private NamedDomainObjectContainer createTestClustersContaine project.getPath(), name, GradleServicesAdapter.getInstance(project), - SyncTestClustersConfiguration.getTestClustersConfigurationExtractDir(project), + getTestClustersConfigurationExtractDir(project), new File(project.getBuildDir(), "testclusters") ) ); @@ -249,8 +261,8 @@ public void beforeExecute(Task task) {} ); } - static File getTestClustersBuildDir(Project project) { - return new File(project.getRootProject().getBuildDir(), "testclusters"); + static File getTestClustersConfigurationExtractDir(Project project) { + return new File(project.getRootProject().getBuildDir(), "testclusters/extract"); } /** From 670bfd31b0a537981edf9aeda7f2f355dbc3a242 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Thu, 14 Feb 2019 15:53:00 -0800 Subject: [PATCH 2/2] Fix checkstyle issues --- .../testclusters/TestClustersPlugin.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java index 3512c2d6b875d..56ff501a3885c 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java @@ -36,8 +36,17 @@ import org.gradle.api.tasks.TaskState; import java.io.File; -import java.util.*; -import java.util.concurrent.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; public class TestClustersPlugin implements Plugin { @@ -108,7 +117,12 @@ public void apply(Project project) { // Tasks that use a cluster will add this as a dependency automatically so it's guaranteed to run early in // the build. rootProject.getTasks().create(SYNC_ARTIFACTS_TASK_NAME, Sync.class, sync -> { - sync.from((Callable>) () -> helperConfiguration.getFiles().stream().map(project::zipTree).collect(Collectors.toList())); + sync.from((Callable>) () -> + helperConfiguration.getFiles() + .stream() + .map(project::zipTree) + .collect(Collectors.toList()) + ); sync.into(new File(getTestClustersConfigurationExtractDir(project), "zip")); });