Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use services for archive and file operations in tasks (7.x backport) #63201

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import java.nio.file.Files
* <p>
* 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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Object> realArgs = new ArrayList<>()
Expand Down Expand Up @@ -242,7 +245,9 @@ class AntFixture extends AntTask implements Fixture {
args('-9', pid)
}
doLast {
project.delete(fixture.pidFile)
getFileSystemOperations().delete {
it.delete(fixture.pidFile)
}
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.io.File;
import java.util.Map;

import static org.elasticsearch.gradle.util.FileUtils.mkdirs;
import static org.elasticsearch.gradle.util.GradleUtils.maybeConfigure;

/**
Expand Down Expand Up @@ -84,10 +85,10 @@ public void apply(Project project) {
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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
16 changes: 13 additions & 3 deletions buildSrc/src/main/java/org/elasticsearch/gradle/LoggedExec.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,18 @@
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;
Expand All @@ -47,13 +50,15 @@
* 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<Logger> outputLogger;
private FileSystemOperations fileSystemOperations;

public LoggedExec() {

@Inject
public LoggedExec(FileSystemOperations fileSystemOperations) {
this.fileSystemOperations = fileSystemOperations;
if (getLogger().isInfoEnabled() == false) {
setIgnoreExitValue(true);
setSpoolOutput(false);
Expand Down Expand Up @@ -154,4 +159,9 @@ private static <T extends BaseExecSpec> ExecResult genericExec(Function<Action<T
throw e;
}
}

@Override
public WorkResult delete(Object... objects) {
return fileSystemOperations.delete(d -> d.delete(objects));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
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;
Expand All @@ -47,6 +49,8 @@
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
Expand Down Expand Up @@ -76,6 +80,16 @@ protected Factory<PatternSet> getPatternSetFactory() {
throw new UnsupportedOperationException();
}

@Inject
protected FileSystemOperations getFileSystemOperations() {
throw new UnsupportedOperationException();
}

@Inject
protected ArchiveOperations getArchiveOperations() {
throw new UnsupportedOperationException();
}

@Input
public ListProperty<String> getIncludeCore() {
return includeCore;
Expand Down Expand Up @@ -137,23 +151,23 @@ public File getOutputDir() {

@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());
});
} 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()) {
Expand All @@ -167,8 +181,8 @@ void copy() {
}
// 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());
Expand All @@ -177,7 +191,7 @@ void copy() {
// 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());
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
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;
Expand All @@ -44,6 +46,8 @@
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
Expand Down Expand Up @@ -73,6 +77,16 @@ protected Factory<PatternSet> getPatternSetFactory() {
throw new UnsupportedOperationException();
}

@Inject
protected FileSystemOperations getFileSystemOperations() {
throw new UnsupportedOperationException();
}

@Inject
protected ArchiveOperations getArchiveOperations() {
throw new UnsupportedOperationException();
}

@Input
public ListProperty<String> getIncludeCore() {
return includeCore;
Expand Down Expand Up @@ -127,25 +141,24 @@ public File getOutputDir() {

@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(
Expand All @@ -156,17 +169,16 @@ void copy() {
}
// 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());
});
Expand Down
Loading