From f58ebe58ee52799fb42bb5a7099dc4cf582b833a Mon Sep 17 00:00:00 2001 From: Rene Groeschke Date: Mon, 5 Oct 2020 15:52:15 +0200 Subject: [PATCH] Use services for archive and file operations in tasks (#62968) (#63201) Referencing a project instance during task execution is discouraged by Gradle and should be avoided. E.g. It is incompatible with Gradles incubating configuration cache. Instead there are services available to handle archive and filesystem operations in task actions. Brings us one step closer to #57918 --- .../org/elasticsearch/gradle/AntTask.groovy | 7 ++ .../precommit/LicenseHeadersTask.groovy | 2 +- .../gradle/precommit/PrecommitTasks.groovy | 2 +- .../gradle/test/AntFixture.groovy | 9 ++- .../gradle/ElasticsearchTestBasePlugin.java | 9 ++- .../gradle/FileSystemOperationsAware.java | 30 +++++++ .../org/elasticsearch/gradle/LoggedExec.java | 16 +++- .../gradle/test/rest/CopyRestApiTask.java | 32 +++++--- .../gradle/test/rest/CopyRestTestsTask.java | 34 +++++--- .../testclusters/ElasticsearchCluster.java | 43 +++++++++- .../testclusters/ElasticsearchNode.java | 30 +++++-- .../StandaloneRestIntegTestTask.java | 8 +- .../testclusters/TestClustersPlugin.java | 23 +++++- .../testfixtures/TestFixturesPlugin.java | 9 ++- .../elasticsearch/gradle/util/FileUtils.java | 78 +++++++++++++++++++ .../gradle/util/GradleUtils.java | 11 ++- distribution/packages/build.gradle | 2 +- plugins/discovery-azure-classic/build.gradle | 2 +- qa/full-cluster-restart/build.gradle | 2 +- qa/mixed-cluster/build.gradle | 2 +- qa/repository-multi-version/build.gradle | 2 +- qa/rolling-upgrade/build.gradle | 2 +- x-pack/qa/full-cluster-restart/build.gradle | 2 +- 23 files changed, 305 insertions(+), 52 deletions(-) create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/FileSystemOperationsAware.java create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/util/FileUtils.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/AntTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/AntTask.groovy index 0393e7632bb..73df8ddbffa 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/AntTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/AntTask.groovy @@ -25,9 +25,11 @@ import org.apache.tools.ant.DefaultLogger import org.apache.tools.ant.Project import org.gradle.api.DefaultTask import org.gradle.api.GradleException +import org.gradle.api.file.FileSystemOperations import org.gradle.api.tasks.Input import org.gradle.api.tasks.TaskAction +import javax.inject.Inject import java.nio.charset.Charset /** @@ -43,6 +45,11 @@ public abstract class AntTask extends DefaultTask { */ public final ByteArrayOutputStream outputBuffer = new ByteArrayOutputStream() + @Inject + protected FileSystemOperations getFileSystemOperations() { + throw new UnsupportedOperationException(); + } + @TaskAction final void executeTask() { AntBuilder ant = new AntBuilder() diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LicenseHeadersTask.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LicenseHeadersTask.groovy index 81894695190..e24bb9b1499 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LicenseHeadersTask.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/LicenseHeadersTask.groovy @@ -35,7 +35,7 @@ import java.nio.file.Files *

* This is a port of the apache lucene check */ -public class LicenseHeadersTask extends AntTask { +class LicenseHeadersTask extends AntTask { @OutputFile File reportFile = new File(project.buildDir, 'reports/licenseHeaders/rat.log') diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy index 98863224daa..2747b238377 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -28,7 +28,7 @@ class PrecommitTasks { /** Adds a precommit task, which depends on non-test verification tasks. */ - public static void create(Project project, boolean includeDependencyLicenses) { + static void create(Project project, boolean includeDependencyLicenses) { project.pluginManager.apply(CheckstylePrecommitPlugin) project.pluginManager.apply(ForbiddenApisPrecommitPlugin) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/AntFixture.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/AntFixture.groovy index 6650463a5c5..3d8258eca9d 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/AntFixture.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/AntFixture.groovy @@ -96,7 +96,10 @@ class AntFixture extends AntTask implements Fixture { @Override protected void runAnt(AntBuilder ant) { - project.delete(baseDir) // reset everything + // reset everything + getFileSystemOperations().delete { + it.delete(baseDir) + } cwd.mkdirs() final String realExecutable final List realArgs = new ArrayList<>() @@ -242,7 +245,9 @@ class AntFixture extends AntTask implements Fixture { args('-9', pid) } doLast { - project.delete(fixture.pidFile) + getFileSystemOperations().delete { + it.delete(fixture.pidFile) + } } } diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchTestBasePlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchTestBasePlugin.java index 7fe401b1137..3eb10187bb3 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchTestBasePlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchTestBasePlugin.java @@ -38,6 +38,7 @@ import org.gradle.api.tasks.testing.Test; import java.io.File; import java.util.Map; +import static org.elasticsearch.gradle.util.FileUtils.mkdirs; import static org.elasticsearch.gradle.util.GradleUtils.maybeConfigure; /** @@ -84,10 +85,10 @@ public class ElasticsearchTestBasePlugin implements Plugin { test.doFirst(new Action<>() { @Override public void execute(Task t) { - project.mkdir(testOutputDir); - project.mkdir(heapdumpDir); - project.mkdir(test.getWorkingDir()); - project.mkdir(test.getWorkingDir().toPath().resolve("temp")); + mkdirs(testOutputDir); + mkdirs(heapdumpDir); + mkdirs(test.getWorkingDir()); + mkdirs(test.getWorkingDir().toPath().resolve("temp").toFile()); // TODO remove once jvm.options are added to test system properties if (BuildParams.getRuntimeJavaVersion() == JavaVersion.VERSION_1_8) { diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/FileSystemOperationsAware.java b/buildSrc/src/main/java/org/elasticsearch/gradle/FileSystemOperationsAware.java new file mode 100644 index 00000000000..b082b86e6c2 --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/FileSystemOperationsAware.java @@ -0,0 +1,30 @@ +/* + * 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; + +import org.gradle.api.tasks.WorkResult; + +/** + * An interface for tasks that support basic file operations. + * Methods will be added as needed. + */ +public interface FileSystemOperationsAware { + WorkResult delete(Object... objects); +} diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/LoggedExec.java b/buildSrc/src/main/java/org/elasticsearch/gradle/LoggedExec.java index 222640f4e87..0c17084220f 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/LoggedExec.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/LoggedExec.java @@ -22,15 +22,18 @@ import org.gradle.api.Action; import org.gradle.api.GradleException; import org.gradle.api.Project; import org.gradle.api.Task; +import org.gradle.api.file.FileSystemOperations; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.tasks.Exec; +import org.gradle.api.tasks.WorkResult; import org.gradle.process.BaseExecSpec; import org.gradle.process.ExecOperations; import org.gradle.process.ExecResult; import org.gradle.process.ExecSpec; import org.gradle.process.JavaExecSpec; +import javax.inject.Inject; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; @@ -47,13 +50,15 @@ import java.util.regex.Pattern; * A wrapper around gradle's Exec task to capture output and log on error. */ @SuppressWarnings("unchecked") -public class LoggedExec extends Exec { +public class LoggedExec extends Exec implements FileSystemOperationsAware { private static final Logger LOGGER = Logging.getLogger(LoggedExec.class); private Consumer outputLogger; + private FileSystemOperations fileSystemOperations; - public LoggedExec() { - + @Inject + public LoggedExec(FileSystemOperations fileSystemOperations) { + this.fileSystemOperations = fileSystemOperations; if (getLogger().isInfoEnabled() == false) { setIgnoreExitValue(true); setSpoolOutput(false); @@ -154,4 +159,9 @@ public class LoggedExec extends Exec { throw e; } } + + @Override + public WorkResult delete(Object... objects) { + return fileSystemOperations.delete(d -> d.delete(objects)); + } } diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestApiTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestApiTask.java index d9a4b45293b..08d46f97dda 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestApiTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestApiTask.java @@ -24,7 +24,9 @@ import org.elasticsearch.gradle.util.GradleUtils; import org.gradle.api.DefaultTask; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.file.ArchiveOperations; import org.gradle.api.file.ConfigurableFileCollection; +import org.gradle.api.file.FileSystemOperations; import org.gradle.api.file.FileTree; import org.gradle.api.plugins.JavaPluginConvention; import org.gradle.api.provider.ListProperty; @@ -47,6 +49,8 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import static org.elasticsearch.gradle.util.GradleUtils.getProjectPathFromTask; + /** * Copies the files needed for the Rest YAML specs to the current projects test resources output directory. * This is intended to be be used from {@link RestResourcesPlugin} since the plugin wires up the needed @@ -76,6 +80,16 @@ public class CopyRestApiTask extends DefaultTask { throw new UnsupportedOperationException(); } + @Inject + protected FileSystemOperations getFileSystemOperations() { + throw new UnsupportedOperationException(); + } + + @Inject + protected ArchiveOperations getArchiveOperations() { + throw new UnsupportedOperationException(); + } + @Input public ListProperty getIncludeCore() { return includeCore; @@ -137,11 +151,11 @@ public class CopyRestApiTask extends DefaultTask { @TaskAction void copy() { - Project project = getProject(); // always copy the core specs if the task executes + String projectPath = getProjectPathFromTask(getPath()); if (BuildParams.isInternal()) { - getLogger().debug("Rest specs for project [{}] will be copied to the test resources.", project.getPath()); - project.copy(c -> { + getLogger().debug("Rest specs for project [{}] will be copied to the test resources.", projectPath); + getFileSystemOperations().copy(c -> { c.from(coreConfig.getAsFileTree()); c.into(getOutputDir()); c.include(corePatternSet.getIncludes()); @@ -149,11 +163,11 @@ public class CopyRestApiTask extends DefaultTask { } else { getLogger().debug( "Rest specs for project [{}] will be copied to the test resources from the published jar (version: [{}]).", - project.getPath(), + projectPath, VersionProperties.getElasticsearch() ); - project.copy(c -> { - c.from(project.zipTree(coreConfig.getSingleFile())); + getFileSystemOperations().copy(c -> { + c.from(getArchiveOperations().zipTree(coreConfig.getSingleFile())); // this ends up as the same dir as outputDir c.into(Objects.requireNonNull(getSourceSet().orElseThrow().getOutput().getResourcesDir())); if (includeCore.get().isEmpty()) { @@ -167,8 +181,8 @@ public class CopyRestApiTask extends DefaultTask { } // only copy x-pack specs if explicitly instructed if (includeXpack.get().isEmpty() == false) { - getLogger().debug("X-pack rest specs for project [{}] will be copied to the test resources.", project.getPath()); - project.copy(c -> { + getLogger().debug("X-pack rest specs for project [{}] will be copied to the test resources.", projectPath); + getFileSystemOperations().copy(c -> { c.from(xpackConfig.getSingleFile()); c.into(getOutputDir()); c.include(xpackPatternSet.getIncludes()); @@ -177,7 +191,7 @@ public class CopyRestApiTask extends DefaultTask { // TODO: once https://github.com/elastic/elasticsearch/pull/62968 lands ensure that this uses `getFileSystemOperations()` // copy any additional config if (additionalConfig != null) { - project.copy(c -> { + getFileSystemOperations().copy(c -> { c.from(additionalConfig.getAsFileTree()); c.into(getOutputDir()); }); diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestTestsTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestTestsTask.java index 99dbad7f025..fb7cf7c6d2b 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestTestsTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/CopyRestTestsTask.java @@ -24,7 +24,9 @@ import org.elasticsearch.gradle.util.GradleUtils; import org.gradle.api.DefaultTask; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.file.ArchiveOperations; import org.gradle.api.file.ConfigurableFileCollection; +import org.gradle.api.file.FileSystemOperations; import org.gradle.api.file.FileTree; import org.gradle.api.plugins.JavaPluginConvention; import org.gradle.api.provider.ListProperty; @@ -44,6 +46,8 @@ import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; +import static org.elasticsearch.gradle.util.GradleUtils.getProjectPathFromTask; + /** * Copies the Rest YAML test to the current projects test resources output directory. * This is intended to be be used from {@link RestResourcesPlugin} since the plugin wires up the needed @@ -73,6 +77,16 @@ public class CopyRestTestsTask extends DefaultTask { throw new UnsupportedOperationException(); } + @Inject + protected FileSystemOperations getFileSystemOperations() { + throw new UnsupportedOperationException(); + } + + @Inject + protected ArchiveOperations getArchiveOperations() { + throw new UnsupportedOperationException(); + } + @Input public ListProperty getIncludeCore() { return includeCore; @@ -127,25 +141,24 @@ public class CopyRestTestsTask extends DefaultTask { @TaskAction void copy() { - Project project = getProject(); + String projectPath = getProjectPathFromTask(getPath()); // only copy core tests if explicitly instructed if (includeCore.get().isEmpty() == false) { if (BuildParams.isInternal()) { - getLogger().debug("Rest tests for project [{}] will be copied to the test resources.", project.getPath()); - project.copy(c -> { + getLogger().debug("Rest tests for project [{}] will be copied to the test resources.", projectPath); + getFileSystemOperations().copy(c -> { c.from(coreConfig.getAsFileTree()); c.into(getOutputDir()); c.include(corePatternSet.getIncludes()); }); - } else { getLogger().debug( "Rest tests for project [{}] will be copied to the test resources from the published jar (version: [{}]).", - project.getPath(), + projectPath, VersionProperties.getElasticsearch() ); - project.copy(c -> { - c.from(project.zipTree(coreConfig.getSingleFile())); + getFileSystemOperations().copy(c -> { + c.from(getArchiveOperations().zipTree(coreConfig.getSingleFile())); // this ends up as the same dir as outputDir c.into(Objects.requireNonNull(getSourceSet().orElseThrow().getOutput().getResourcesDir())); c.include( @@ -156,17 +169,16 @@ public class CopyRestTestsTask extends DefaultTask { } // only copy x-pack tests if explicitly instructed if (includeXpack.get().isEmpty() == false) { - getLogger().debug("X-pack rest tests for project [{}] will be copied to the test resources.", project.getPath()); - project.copy(c -> { + getLogger().debug("X-pack rest tests for project [{}] will be copied to the test resources.", projectPath); + getFileSystemOperations().copy(c -> { c.from(xpackConfig.getAsFileTree()); c.into(getOutputDir()); c.include(xpackPatternSet.getIncludes()); }); } - // TODO: once https://github.com/elastic/elasticsearch/pull/62968 lands ensure that this uses `getFileSystemOperations()` // copy any additional config if (additionalConfig != null) { - project.copy(c -> { + getFileSystemOperations().copy(c -> { c.from(additionalConfig.getAsFileTree()); c.into(getOutputDir()); }); diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java index 2315ce47cb7..772f4042f35 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchCluster.java @@ -26,6 +26,8 @@ import org.elasticsearch.gradle.http.WaitForHttpResource; import org.gradle.api.Named; import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.Project; +import org.gradle.api.file.ArchiveOperations; +import org.gradle.api.file.FileSystemOperations; import org.gradle.api.file.RegularFile; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; @@ -65,17 +67,41 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named { private final LinkedHashMap> waitConditions = new LinkedHashMap<>(); private final Project project; private final ReaperService reaper; + private final FileSystemOperations fileSystemOperations; + private final ArchiveOperations archiveOperations; private int nodeIndex = 0; - public ElasticsearchCluster(String clusterName, Project project, ReaperService reaper, File workingDirBase, Jdk bwcJdk) { + public ElasticsearchCluster( + String clusterName, + Project project, + ReaperService reaper, + File workingDirBase, + FileSystemOperations fileSystemOperations, + ArchiveOperations archiveOperations, + Jdk bwcJdk + ) { this.path = project.getPath(); this.clusterName = clusterName; this.project = project; this.reaper = reaper; + this.fileSystemOperations = fileSystemOperations; + this.archiveOperations = archiveOperations; this.workingDirBase = workingDirBase; this.nodes = project.container(ElasticsearchNode.class); this.bwcJdk = bwcJdk; - this.nodes.add(new ElasticsearchNode(clusterName + "-0", project, reaper, workingDirBase, bwcJdk)); + + this.nodes.add( + new ElasticsearchNode( + path, + clusterName + "-0", + project, + reaper, + fileSystemOperations, + archiveOperations, + workingDirBase, + bwcJdk + ) + ); // configure the cluster name eagerly so nodes know about it this.nodes.all((node) -> node.defaultConfig.put("cluster.name", safeName(clusterName))); @@ -96,7 +122,18 @@ public class ElasticsearchCluster implements TestClusterConfiguration, Named { } for (int i = nodes.size(); i < numberOfNodes; i++) { - this.nodes.add(new ElasticsearchNode(clusterName + "-" + i, project, reaper, workingDirBase, bwcJdk)); + this.nodes.add( + new ElasticsearchNode( + path, + clusterName + "-" + i, + project, + reaper, + fileSystemOperations, + archiveOperations, + workingDirBase, + bwcJdk + ) + ); } } diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java index fd16263cdf6..a4dd4b6c046 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/ElasticsearchNode.java @@ -39,6 +39,8 @@ import org.gradle.api.Named; import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.file.ArchiveOperations; +import org.gradle.api.file.FileSystemOperations; import org.gradle.api.file.FileTree; import org.gradle.api.file.RegularFile; import org.gradle.api.logging.Logger; @@ -123,6 +125,9 @@ public class ElasticsearchNode implements TestClusterConfiguration { private final Project project; private final ReaperService reaper; private final Jdk bwcJdk; + private final FileSystemOperations fileSystemOperations; + private final ArchiveOperations archiveOperations; + private final AtomicBoolean configurationFrozen = new AtomicBoolean(false); private final Path workingDir; @@ -163,11 +168,22 @@ public class ElasticsearchNode implements TestClusterConfiguration { private Path confPathData; private String keystorePassword = ""; - ElasticsearchNode(String name, Project project, ReaperService reaper, File workingDirBase, Jdk bwcJdk) { - this.path = project.getPath(); + ElasticsearchNode( + String path, + String name, + Project project, + ReaperService reaper, + FileSystemOperations fileSystemOperations, + ArchiveOperations archiveOperations, + File workingDirBase, + Jdk bwcJdk + ) { + this.path = path; this.name = name; this.project = project; this.reaper = reaper; + this.fileSystemOperations = fileSystemOperations; + this.archiveOperations = archiveOperations; this.bwcJdk = bwcJdk; workingDir = workingDirBase.toPath().resolve(safeName(name)).toAbsolutePath(); confPathRepo = workingDir.resolve("repo"); @@ -436,7 +452,7 @@ public class ElasticsearchNode implements TestClusterConfiguration { logToProcessStdout("Configuring working directory: " + workingDir); // make sure we always start fresh if (Files.exists(workingDir)) { - project.delete(workingDir); + fileSystemOperations.delete(d -> d.delete(workingDir)); } isWorkingDirConfigured = true; } @@ -624,9 +640,9 @@ public class ElasticsearchNode implements TestClusterConfiguration { .resolve(module.get().getName().replace(".zip", "").replace("-" + getVersion(), "").replace("-SNAPSHOT", "")); // only install modules that are not already bundled with the integ-test distribution if (Files.exists(destination) == false) { - project.copy(spec -> { + fileSystemOperations.copy(spec -> { if (module.get().getName().toLowerCase().endsWith(".zip")) { - spec.from(project.zipTree(module)); + spec.from(archiveOperations.zipTree(module)); } else if (module.get().isDirectory()) { spec.from(module); } else { @@ -1039,7 +1055,7 @@ public class ElasticsearchNode implements TestClusterConfiguration { private void createWorkingDir() throws IOException { // Start configuration from scratch in case of a restart - project.delete(configFile.getParent()); + fileSystemOperations.delete(d -> d.delete(configFile.getParent())); Files.createDirectories(configFile.getParent()); Files.createDirectories(confPathRepo); Files.createDirectories(confPathData); @@ -1284,7 +1300,7 @@ public class ElasticsearchNode implements TestClusterConfiguration { .filter(File::exists) // TODO: We may be able to simplify this with Gradle 5.6 // https://docs.gradle.org/nightly/release-notes.html#improved-handling-of-zip-archives-on-classpaths - .map(zipFile -> project.zipTree(zipFile).matching(filter)) + .map(zipFile -> archiveOperations.zipTree(zipFile).matching(filter)) .flatMap(tree -> tree.getFiles().stream()) .sorted(Comparator.comparing(File::getName)) .collect(Collectors.toList()); diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/StandaloneRestIntegTestTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/StandaloneRestIntegTestTask.java index dd2a081024f..f1bd38d2033 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/StandaloneRestIntegTestTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/StandaloneRestIntegTestTask.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.gradle.testclusters; +import org.elasticsearch.gradle.FileSystemOperationsAware; import org.elasticsearch.gradle.test.Fixture; import org.elasticsearch.gradle.util.GradleUtils; import org.gradle.api.Task; @@ -26,6 +27,7 @@ import org.gradle.api.services.internal.BuildServiceRegistryInternal; import org.gradle.api.tasks.CacheableTask; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.Nested; +import org.gradle.api.tasks.WorkResult; import org.gradle.api.tasks.testing.Test; import org.gradle.internal.resources.ResourceLock; import org.gradle.internal.resources.SharedResource; @@ -44,7 +46,7 @@ import static org.elasticsearch.gradle.testclusters.TestClustersPlugin.THROTTLE_ * {@link Nested} inputs. */ @CacheableTask -public class StandaloneRestIntegTestTask extends Test implements TestClustersAware { +public class StandaloneRestIntegTestTask extends Test implements TestClustersAware, FileSystemOperationsAware { private Collection clusters = new HashSet<>(); @@ -113,4 +115,8 @@ public class StandaloneRestIntegTestTask extends Test implements TestClustersAwa } } } + + public WorkResult delete(Object... objects) { + return getFileSystemOperations().delete(d -> d.delete(objects)); + } } 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 de027fa8ef6..df7b03f1ee5 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testclusters/TestClustersPlugin.java @@ -35,12 +35,15 @@ import org.gradle.api.Project; import org.gradle.api.Task; import org.gradle.api.execution.TaskActionListener; import org.gradle.api.execution.TaskExecutionListener; +import org.gradle.api.file.ArchiveOperations; +import org.gradle.api.file.FileSystemOperations; import org.gradle.api.invocation.Gradle; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.provider.Provider; import org.gradle.api.tasks.TaskState; +import javax.inject.Inject; import java.io.File; import static org.elasticsearch.gradle.util.GradleUtils.noop; @@ -56,6 +59,16 @@ public class TestClustersPlugin implements Plugin { private static final String LEGACY_JAVA_VERSION = "8u242+b08"; private static final Logger logger = Logging.getLogger(TestClustersPlugin.class); + @Inject + protected FileSystemOperations getFileSystemOperations() { + throw new UnsupportedOperationException(); + } + + @Inject + protected ArchiveOperations getArchiveOperations() { + throw new UnsupportedOperationException(); + } + @Override public void apply(Project project) { project.getPluginManager().apply(JdkDownloadPlugin.class); @@ -107,7 +120,15 @@ public class TestClustersPlugin implements Plugin { // Create an extensions that allows describing clusters NamedDomainObjectContainer container = project.container( ElasticsearchCluster.class, - name -> new ElasticsearchCluster(name, project, reaper, new File(project.getBuildDir(), "testclusters"), bwcJdk) + name -> new ElasticsearchCluster( + name, + project, + reaper, + new File(project.getBuildDir(), "testclusters"), + getFileSystemOperations(), + getArchiveOperations(), + bwcJdk + ) ); project.getExtensions().add(EXTENSION_NAME, container); return container; diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java index 88e744043b7..25a145098da 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java @@ -35,6 +35,7 @@ import org.gradle.api.DefaultTask; import org.gradle.api.Plugin; import org.gradle.api.Project; import org.gradle.api.Task; +import org.gradle.api.file.FileSystemOperations; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.plugins.BasePlugin; @@ -44,6 +45,7 @@ import org.gradle.api.tasks.TaskContainer; import org.gradle.api.tasks.TaskProvider; import org.gradle.api.tasks.testing.Test; +import javax.inject.Inject; import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; @@ -57,6 +59,11 @@ public class TestFixturesPlugin implements Plugin { private static final String DOCKER_COMPOSE_THROTTLE = "dockerComposeThrottle"; static final String DOCKER_COMPOSE_YML = "docker-compose.yml"; + @Inject + protected FileSystemOperations getFileSystemOperations() { + throw new UnsupportedOperationException(); + } + @Override public void apply(Project project) { project.getRootProject().getPluginManager().apply(DockerSupportPlugin.class); @@ -122,7 +129,7 @@ public class TestFixturesPlugin implements Plugin { t.mustRunAfter(preProcessFixture); }); tasks.named("composePull").configure(t -> t.mustRunAfter(preProcessFixture)); - tasks.named("composeDown").configure(t -> t.doLast(t2 -> project.delete(testfixturesDir))); + tasks.named("composeDown").configure(t -> t.doLast(t2 -> getFileSystemOperations().delete(d -> d.delete(testfixturesDir)))); } else { project.afterEvaluate(spec -> { if (extension.fixtures.isEmpty()) { diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/util/FileUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/util/FileUtils.java new file mode 100644 index 00000000000..f8e8b5a39c5 --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/util/FileUtils.java @@ -0,0 +1,78 @@ +/* + * 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.gradle.api.UncheckedIOException; + +import java.io.File; +import java.util.Collections; +import java.util.LinkedList; +import java.util.List; + +public final class FileUtils { + + /** + * Like {@link java.io.File#mkdirs()}, except throws an informative error if a dir cannot be created. + * + * @param dir The dir to create, including any non existent parent dirs. + */ + public static void mkdirs(File dir) { + dir = dir.getAbsoluteFile(); + if (dir.isDirectory()) { + return; + } + + if (dir.exists() && !dir.isDirectory()) { + throw new UncheckedIOException(String.format("Cannot create directory '%s' as it already exists, but is not a directory", dir)); + } + + List toCreate = new LinkedList(); + File parent = dir.getParentFile(); + while (!parent.exists()) { + toCreate.add(parent); + parent = parent.getParentFile(); + } + Collections.reverse(toCreate); + for (File parentDirToCreate : toCreate) { + if (parentDirToCreate.isDirectory()) { + continue; + } + File parentDirToCreateParent = parentDirToCreate.getParentFile(); + if (!parentDirToCreateParent.isDirectory()) { + throw new UncheckedIOException( + String.format( + "Cannot create parent directory '%s' when creating directory '%s' as '%s' is not a directory", + parentDirToCreate, + dir, + parentDirToCreateParent + ) + ); + } + if (!parentDirToCreate.mkdir() && !parentDirToCreate.isDirectory()) { + throw new UncheckedIOException( + String.format("Failed to create parent directory '%s' when creating directory '%s'", parentDirToCreate, dir) + ); + } + } + if (!dir.mkdir() && !dir.isDirectory()) { + throw new UncheckedIOException(String.format("Failed to create directory '%s'", dir)); + } + } +} diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java b/buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java index 79452a83e35..48619a5ed42 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java @@ -112,7 +112,7 @@ public abstract class GradleUtils { /** * Add a source set and task of the same name that runs tests. - * + *

* IDEs are also configured if setup, and the test task is added to check. The new test source * set extends from the normal test source set to allow sharing of utilities. * @@ -218,4 +218,13 @@ public abstract class GradleUtils { depConfig.put("configuration", projectConfig); return project.getDependencies().project(depConfig); } + + /** + * To calculate the project path from a task path without relying on Task#getProject() which is discouraged during + * task execution time. + */ + public static String getProjectPathFromTask(String taskPath) { + int lastDelimiterIndex = taskPath.lastIndexOf(":"); + return lastDelimiterIndex == 0 ? ":" : taskPath.substring(0, lastDelimiterIndex); + } } diff --git a/distribution/packages/build.gradle b/distribution/packages/build.gradle index aa76e569f3c..a759e47cb14 100644 --- a/distribution/packages/build.gradle +++ b/distribution/packages/build.gradle @@ -459,7 +459,7 @@ subprojects { tasks.register('checkExtraction', LoggedExec) { dependsOn buildDist doFirst { - project.delete(extractionDir) + delete(extractionDir) extractionDir.mkdirs() } } diff --git a/plugins/discovery-azure-classic/build.gradle b/plugins/discovery-azure-classic/build.gradle index cfcb48841c4..842d46e7be6 100644 --- a/plugins/discovery-azure-classic/build.gradle +++ b/plugins/discovery-azure-classic/build.gradle @@ -74,7 +74,7 @@ File keystore = new File(project.buildDir, 'keystore/test-node.jks') // generate the keystore TaskProvider createKey = tasks.register("createKey", LoggedExec) { doFirst { - project.delete(keystore.parentFile) + delete(keystore.parentFile) keystore.parentFile.mkdirs() } outputs.file(keystore).withPropertyName('keystoreFile') diff --git a/qa/full-cluster-restart/build.gradle b/qa/full-cluster-restart/build.gradle index 899ace6616a..1b099bf624f 100644 --- a/qa/full-cluster-restart/build.gradle +++ b/qa/full-cluster-restart/build.gradle @@ -44,7 +44,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.indexCompatible) { useCluster testClusters."${baseName}" mustRunAfter(precommit) doFirst { - project.delete("${buildDir}/cluster/shared/repo/${baseName}") + delete("${buildDir}/cluster/shared/repo/${baseName}") } systemProperty 'tests.is_old_cluster', 'true' diff --git a/qa/mixed-cluster/build.gradle b/qa/mixed-cluster/build.gradle index 0589105b2ec..f8a73182f28 100644 --- a/qa/mixed-cluster/build.gradle +++ b/qa/mixed-cluster/build.gradle @@ -56,7 +56,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) { useCluster testClusters."${baseName}" mustRunAfter(precommit) doFirst { - project.delete("${buildDir}/cluster/shared/repo/${baseName}") + delete("${buildDir}/cluster/shared/repo/${baseName}") // Getting the endpoints causes a wait for the cluster println "Test cluster endpoints are: ${-> testClusters."${baseName}".allHttpSocketURI.join(",")}" println "Upgrading one node to create a mixed cluster" diff --git a/qa/repository-multi-version/build.gradle b/qa/repository-multi-version/build.gradle index 2c24f8e81c9..a60aca8db76 100644 --- a/qa/repository-multi-version/build.gradle +++ b/qa/repository-multi-version/build.gradle @@ -51,7 +51,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.indexCompatible) { useCluster testClusters."${oldClusterName}" mustRunAfter(precommit) doFirst { - project.delete("${buildDir}/cluster/shared/repo/${baseName}") + delete("${buildDir}/cluster/shared/repo/${baseName}") } systemProperty 'tests.rest.suite', 'step1' } diff --git a/qa/rolling-upgrade/build.gradle b/qa/rolling-upgrade/build.gradle index 9e094f1641d..5575d1dccae 100644 --- a/qa/rolling-upgrade/build.gradle +++ b/qa/rolling-upgrade/build.gradle @@ -59,7 +59,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.wireCompatible) { useCluster testClusters."${baseName}" mustRunAfter(precommit) doFirst { - project.delete("${buildDir}/cluster/shared/repo/${baseName}") + delete("${buildDir}/cluster/shared/repo/${baseName}") } systemProperty 'tests.upgrade_from_version', bwcVersionStr systemProperty 'tests.rest.suite', 'old_cluster' diff --git a/x-pack/qa/full-cluster-restart/build.gradle b/x-pack/qa/full-cluster-restart/build.gradle index e567a605e26..89b4697dd54 100644 --- a/x-pack/qa/full-cluster-restart/build.gradle +++ b/x-pack/qa/full-cluster-restart/build.gradle @@ -75,7 +75,7 @@ for (Version bwcVersion : BuildParams.bwcVersions.indexCompatible) { useCluster testClusters."${baseName}" dependsOn copyTestNodeKeyMaterial doFirst { - project.delete("${buildDir}/cluster/shared/repo/${baseName}") + delete("${buildDir}/cluster/shared/repo/${baseName}") } systemProperty 'tests.is_old_cluster', 'true' exclude 'org/elasticsearch/upgrades/FullClusterRestartIT.class'