Remove buildResources from BuildPlugin (#57482)

The shared buildResources task is a catch all for resources needing to
be copied from the build-tools jar at runtime. Utilizing this for all
resources causes any tasks using resources from this to be triggered on
any changes to any of those files. This commit creates separate export
tasks per usage, and removes the buildResources task.
This commit is contained in:
Ryan Ernst 2020-06-01 15:10:52 -07:00 committed by Ryan Ernst
parent 6592c3856d
commit 88ea1f97b8
No known key found for this signature in database
GPG Key ID: 5F7EA39E15F54DCE
6 changed files with 55 additions and 76 deletions

View File

@ -32,38 +32,22 @@ public class ExportElasticsearchBuildResourcesTaskIT extends GradleIntegrationTe
BuildResult result = getGradleRunner(PROJECT_NAME).withArguments("buildResources", "-s", "-i").build();
assertTaskSuccessful(result, ":buildResources");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml");
// using task avoidance api means the task configuration of the sample task is never triggered
assertBuildFileDoesNotExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml");
result = getGradleRunner(PROJECT_NAME).withArguments("buildResources", "-s", "-i").build();
assertTaskUpToDate(result, ":buildResources");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml");
// using task avoidance api means the task configuration of the sample task is never triggered
assertBuildFileDoesNotExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml");
}
public void testImplicitTaskDependencyCopy() {
BuildResult result = getGradleRunner(PROJECT_NAME).withArguments("clean", "sampleCopyAll", "-s", "-i").build();
assertTaskSuccessful(result, ":buildResources");
assertTaskSuccessful(result, ":sampleCopyAll");
assertBuildFileExists(result, PROJECT_NAME, "sampleCopyAll/checkstyle.xml");
// using task avoidance api means the task configuration of the sample task is never triggered
// which means buildResource is not configured to copy this file
assertBuildFileDoesNotExists(result, PROJECT_NAME, "sampleCopyAll/checkstyle_suppressions.xml");
}
public void testImplicitTaskDependencyInputFileOfOther() {
BuildResult result = getGradleRunner(PROJECT_NAME).withArguments("clean", "sample", "-s", "-i").build();
assertTaskSuccessful(result, ":sample");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml");
}
public void testOutputAsInput() {
BuildResult result = getGradleRunner(PROJECT_NAME).withArguments("clean", "sampleCopy", "-s", "-i").build();
assertTaskSuccessful(result, ":sampleCopy");
assertBuildFileExists(result, PROJECT_NAME, "sampleCopy/checkstyle.xml");
assertBuildFileExists(result, PROJECT_NAME, "sampleCopy/checkstyle_suppressions.xml");
}
public void testIncorrectUsage() {
assertOutputContains(
getGradleRunner(PROJECT_NAME).withArguments("noConfigAfterExecution", "-s", "-i").buildAndFail().getOutput(),

View File

@ -72,8 +72,6 @@ class BuildPlugin implements Plugin<Project> {
project.pluginManager.apply('elasticsearch.publish')
project.pluginManager.apply(DependenciesInfoPlugin)
project.getTasks().register("buildResources", ExportElasticsearchBuildResourcesTask)
project.extensions.getByType(ExtraPropertiesExtension).set('versions', VersionProperties.versions)
PrecommitTasks.create(project, true)
}

View File

@ -56,7 +56,6 @@ public class ExportElasticsearchBuildResourcesTask extends DefaultTask {
public ExportElasticsearchBuildResourcesTask() {
outputDir = getProject().getObjects().directoryProperty();
outputDir.set(new File(getProject().getBuildDir(), "build-tools-exported"));
}
@OutputDirectory
@ -80,14 +79,13 @@ public class ExportElasticsearchBuildResourcesTask extends DefaultTask {
this.outputDir.set(outputDir);
}
public File copy(String resource) {
public void copy(String resource) {
if (getState().getExecuted() || getState().getExecuting()) {
throw new GradleException(
"buildResources can't be configured after the task ran. " + "Make sure task is not used after configuration time"
);
}
resources.add(resource);
return outputDir.file(resource).get().getAsFile();
}
@TaskAction

View File

@ -33,7 +33,7 @@ import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.SourceSetContainer;
import org.gradle.api.tasks.TaskProvider;
import java.io.File;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
@ -43,11 +43,19 @@ public class ForbiddenApisPrecommitPlugin extends PrecommitPlugin {
public TaskProvider<? extends Task> createTask(Project project) {
project.getPluginManager().apply(ForbiddenApisPlugin.class);
TaskProvider<ExportElasticsearchBuildResourcesTask> buildResources = project.getTasks()
.named("buildResources", ExportElasticsearchBuildResourcesTask.class);
TaskProvider<ExportElasticsearchBuildResourcesTask> resourcesTask = project.getTasks()
.register("forbiddenApisResources", ExportElasticsearchBuildResourcesTask.class);
Path resourcesDir = project.getBuildDir().toPath().resolve("forbidden-apis-config");
resourcesTask.configure(t -> {
t.setOutputDir(resourcesDir.toFile());
t.copy("forbidden/jdk-signatures.txt");
t.copy("forbidden/es-all-signatures.txt");
t.copy("forbidden/es-test-signatures.txt");
t.copy("forbidden/http-signatures.txt");
t.copy("forbidden/es-server-signatures.txt");
});
project.getTasks().withType(CheckForbiddenApis.class).configureEach(t -> {
ExportElasticsearchBuildResourcesTask buildResourcesTask = buildResources.get();
t.dependsOn(buildResources);
t.dependsOn(resourcesTask);
assert t.getName().startsWith(ForbiddenApisPlugin.FORBIDDEN_APIS_TASK_NAME);
String sourceSetName;
@ -71,10 +79,7 @@ public class ForbiddenApisPrecommitPlugin extends PrecommitPlugin {
}
t.setBundledSignatures(Set.of("jdk-unsafe", "jdk-deprecated", "jdk-non-portable", "jdk-system-out"));
t.setSignaturesFiles(
project.files(
buildResourcesTask.copy("forbidden/jdk-signatures.txt"),
buildResourcesTask.copy("forbidden/es-all-signatures.txt")
)
project.files(resourcesDir.resolve("forbidden/jdk-signatures.txt"), resourcesDir.resolve("forbidden/es-all-signatures.txt"))
);
t.setSuppressAnnotations(Set.of("**.SuppressForbidden"));
if (t.getName().endsWith("Test")) {
@ -82,23 +87,23 @@ public class ForbiddenApisPrecommitPlugin extends PrecommitPlugin {
t.getSignaturesFiles()
.plus(
project.files(
buildResourcesTask.copy("forbidden/es-test-signatures.txt"),
buildResourcesTask.copy("forbidden/http-signatures.txt")
resourcesDir.resolve("forbidden/es-test-signatures.txt"),
resourcesDir.resolve("forbidden/http-signatures.txt")
)
)
);
} else {
t.setSignaturesFiles(
t.getSignaturesFiles().plus(project.files(buildResourcesTask.copy("forbidden/es-server-signatures.txt")))
t.getSignaturesFiles().plus(project.files(resourcesDir.resolve("forbidden/es-server-signatures.txt")))
);
}
ExtraPropertiesExtension ext = t.getExtensions().getExtraProperties();
ext.set("replaceSignatureFiles", new Closure<Void>(t) {
@Override
public Void call(Object... names) {
List<File> resources = new ArrayList<>(names.length);
List<Path> resources = new ArrayList<>(names.length);
for (Object name : names) {
resources.add(buildResourcesTask.copy("forbidden/" + name + ".txt"));
resources.add(resourcesDir.resolve("forbidden/" + name + ".txt"));
}
t.setSignaturesFiles(project.files(resources));
return null;
@ -108,9 +113,9 @@ public class ForbiddenApisPrecommitPlugin extends PrecommitPlugin {
ext.set("addSignatureFiles", new Closure<Void>(t) {
@Override
public Void call(Object... names) {
List<File> resources = new ArrayList<>(names.length);
List<Path> resources = new ArrayList<>(names.length);
for (Object name : names) {
resources.add(buildResourcesTask.copy("forbidden/" + name + ".txt"));
resources.add(resourcesDir.resolve("forbidden/" + name + ".txt"));
}
t.setSignaturesFiles(t.getSignaturesFiles().plus(project.files(resources)));
return null;

View File

@ -26,6 +26,8 @@ import org.gradle.api.Project;
import org.gradle.api.Task;
import org.gradle.api.tasks.TaskProvider;
import java.nio.file.Path;
public class ThirdPartyAuditPrecommitPlugin extends PrecommitPlugin {
@Override
public TaskProvider<? extends Task> createTask(Project project) {
@ -33,16 +35,20 @@ public class ThirdPartyAuditPrecommitPlugin extends PrecommitPlugin {
project.getConfigurations().create("forbiddenApisCliJar");
project.getDependencies().add("forbiddenApisCliJar", "de.thetaphi:forbiddenapis:2.7");
TaskProvider<ExportElasticsearchBuildResourcesTask> resourcesTask = project.getTasks()
.register("thirdPartyAuditResources", ExportElasticsearchBuildResourcesTask.class);
Path resourcesDir = project.getBuildDir().toPath().resolve("third-party-audit-config");
resourcesTask.configure(t -> {
t.setOutputDir(resourcesDir.toFile());
t.copy("forbidden/third-party-audit.txt");
});
TaskProvider<ThirdPartyAuditTask> audit = project.getTasks().register("thirdPartyAudit", ThirdPartyAuditTask.class);
audit.configure(t -> {
t.dependsOn("buildResources");
t.dependsOn(resourcesTask);
t.setJavaHome(BuildParams.getRuntimeJavaHome().toString());
t.getTargetCompatibility().set(project.provider(BuildParams::getRuntimeJavaVersion));
t.setSignatureFile(resourcesDir.resolve("forbidden/third-party-audit.txt").toFile());
});
project.getTasks()
.withType(ExportElasticsearchBuildResourcesTask.class)
.configureEach((br) -> { audit.get().setSignatureFile(br.copy("forbidden/third-party-audit.txt")); });
return audit;
}
}

View File

@ -1,38 +1,26 @@
import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask
plugins {
id 'elasticsearch.build'
id 'base'
id 'elasticsearch.global-build-info'
}
ext.licenseFile = file("$buildDir/dummy/license")
ext.noticeFile = file("$buildDir/dummy/notice")
buildResources {
File buildResourcesDir = new File(project.getBuildDir(), 'build-tools-exported')
TaskProvider buildResourcesTask = tasks.register('buildResources', ExportElasticsearchBuildResourcesTask) {
outputDir = buildResourcesDir
copy 'checkstyle_suppressions.xml'
copy 'checkstyle.xml'
}
tasks.register("sampleCopyAll", Sync) {
tasks.register("sampleCopy", Sync) {
/** Note: no explicit dependency. This works with tasks that use the Provider API a.k.a "Lazy Configuration" **/
from buildResources
into "$buildDir/sampleCopyAll"
}
tasks.register("sample") {
// This does not work, task dependencies can't be providers
// dependsOn buildResources.resource('minimumRuntimeVersion')
// Nor does this, despite https://github.com/gradle/gradle/issues/3811
// dependsOn buildResources.outputDir
// for now it's just
dependsOn buildResources
// we have to reference it at configuration time in order to be picked up
ext.checkstyle_suppressions = buildResources.copy('checkstyle_suppressions.xml')
doLast {
println "This task is using ${file(checkstyle_suppressions)}"
}
from buildResourcesTask
into "$buildDir/sampleCopy"
}
tasks.register("noConfigAfterExecution") {
dependsOn buildResources
dependsOn buildResourcesTask
doLast {
println "This should cause an error because we are refferencing " +
"${buildResources.copy('checkstyle_suppressions.xml')} after the `buildResources` task has ran."
buildResourcesTask.get().copy('foo')
}
}