Skip to content

Commit

Permalink
Remove buildResources from BuildPlugin (#57482)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@rjernst
rjernst committed Jun 1, 2020
1 parent e525a38 commit 812c75d
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 74 deletions.
30 changes: 7 additions & 23 deletions .../src/integTest/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,36 +32,20 @@ public void testUpToDateWithSourcesConfigured() {
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");
assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle_suppressions.xml");
}

public void testImplicitTaskDependencyInputFileOfOther() {
BuildResult result = getGradleRunner(PROJECT_NAME).withArguments("clean", "sample", "-s", "-i").build();
public void testOutputAsInput() {
BuildResult result = getGradleRunner(PROJECT_NAME).withArguments("clean", "sampleCopy", "-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");
assertTaskSuccessful(result, ":sampleCopy");
assertBuildFileExists(result, PROJECT_NAME, "sampleCopy/checkstyle.xml");
assertBuildFileExists(result, PROJECT_NAME, "sampleCopy/checkstyle_suppressions.xml");
}

public void testIncorrectUsage() {
Expand Down
2 changes: 0 additions & 2 deletions buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 1 addition & 3 deletions buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -80,14 +79,13 @@ public void setOutputDir(File outputDir) {
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
Expand Down
37 changes: 21 additions & 16 deletions buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ForbiddenApisPrecommitPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
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;
Expand All @@ -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;
Expand All @@ -71,34 +79,31 @@ public TaskProvider<? extends Task> createTask(Project project) {
}
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")) {
t.setSignaturesFiles(
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;
Expand All @@ -108,9 +113,9 @@ public Void call(Object... names) {
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;
Expand Down
16 changes: 11 additions & 5 deletions ...dSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditPrecommitPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,29 @@
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) {
project.getPlugins().apply(CompileOnlyResolvePlugin.class);
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;
}
}
38 changes: 13 additions & 25 deletions buildSrc/src/testKit/elasticsearch-build-resources/build.gradle
View file
Original file line number Diff line number Diff line change
@@ -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')
}
}

0 comments on commit 812c75d

Please sign in to comment.