From 02262d520e4a08b72fbf18e9abff68d3e4afb5ea Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Fri, 13 Jul 2018 11:37:24 +0300 Subject: [PATCH 01/10] Add a task to run forbiddenapis using cli Add a task that offers an equivalent check to the forbidden APIs plugin, but runs it using the forbiddenAPIs CLI instead. This isn't wired into precommit first, and doesn't work for projects that require specific signatures, etc. It's meant to show how this can be used. The next step is to make a custom task type and configure it based on the project extension from the pugin and make some minor adjustments to some build scripts as we can't bee 100% compatible with that at least due to how additional signatures are passed. Notes: - there's no `--target` for the CLI version so we have to pass in specific bundled signature names - the cli task already wires to `runtimeJavaHome` - no equivalent for `failOnUnsupportedJava = false` but doesn't seem to be a problem. Tested with Java 8 to 11 - there's no way to pass additional signatures as URL, these will have to be on the file system, and can't be resources on the cp unless we rely on how forbiddenapis is implemented and mimic them as boundled signatures. - the average of 3 runs is 4% slower using the CLI for :server. ( `./gradlew clean :server:forbiddenApis` vs `./gradlew clean :server:forbiddenApisCli`) - up-to-date checks don't work on the cli task yet, that will happen with the custom task. See also: #31715 --- .../gradle/precommit/PrecommitTasks.groovy | 62 ++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) 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 3709805680d7a..77d2670bdafdb 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -22,8 +22,11 @@ import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApis import de.thetaphi.forbiddenapis.gradle.ForbiddenApisPlugin import org.gradle.api.Project import org.gradle.api.Task +import org.gradle.api.file.FileCollection import org.gradle.api.plugins.JavaBasePlugin import org.gradle.api.plugins.quality.Checkstyle +import org.gradle.api.tasks.JavaExec +import org.gradle.api.tasks.StopExecutionException /** * Validation tasks which should be run before committing. These run before tests. @@ -40,7 +43,11 @@ class PrecommitTasks { project.tasks.create('licenseHeaders', LicenseHeadersTask.class), project.tasks.create('filepermissions', FilePermissionsTask.class), project.tasks.create('jarHell', JarHellTask.class), - project.tasks.create('thirdPartyAudit', ThirdPartyAuditTask.class)] + project.tasks.create('thirdPartyAudit', ThirdPartyAuditTask.class) + ] + + // Configure it but don't add it as a dependency yet + configureForbiddenApisCli(project) // tasks with just tests don't need dependency licenses, so this flag makes adding // the task optional @@ -96,9 +103,62 @@ class PrecommitTasks { } Task forbiddenApis = project.tasks.findByName('forbiddenApis') forbiddenApis.group = "" // clear group, so this does not show up under verification tasks + return forbiddenApis } + private static Task configureForbiddenApisCli(Project project) { + project.configurations.create("forbiddenApisCliJar") + project.dependencies { + forbiddenApisCliJar 'de.thetaphi:forbiddenapis:2.5' + } + Task forbiddenApisCli = project.tasks.create('forbiddenApisCli') + project.ext.getForbiddenSignaturesFile = { name -> project.getForbiddenSignaturesFile(name) } + + project.sourceSets.forEach { sourceSet -> + forbiddenApisCli.dependsOn( + project.tasks.create(sourceSet.getTaskName('forbiddenApisCli', null), JavaExec) { + classpath = project.files( + project.configurations.forbiddenApisCliJar, + sourceSet.compileClasspath, + sourceSet.runtimeClasspath + ) + main = 'de.thetaphi.forbiddenapis.cli.CliMain' + executable = "${project.runtimeJavaHome}/bin/java" + [ + 'jdk-unsafe-1.8', 'jdk-deprecated-1.8', 'jdk-non-portable', 'jdk-system-out' + ].forEach { args "-b", it } + [ + getForbiddenSignaturesFile(project, 'jdk-signatures'), + getForbiddenSignaturesFile(project, 'es-all-signatures') + ].forEach { args "-f", it } + if (sourceSet.name == 'test') { + args "-f", getForbiddenSignaturesFile(project, 'es-test-signatures') + args "-f", getForbiddenSignaturesFile(project, 'http-signatures') + } else { + args "-f", getForbiddenSignaturesFile(project, 'es-server-signatures') + } + args "--suppressannotation", '**.SuppressForbidden' + dependsOn sourceSet.classesTaskName + doFirst { + // Forbidden APIs expects only existing dirs, and requires at least one + FileCollection existingOutputs = sourceSet.output.classesDirs + .filter { it.exists() } + if (existingOutputs.isEmpty()) { + throw new StopExecutionException("${sourceSet.name} has no outputs") + } + existingOutputs.forEach { args "-d", it } + } + } + ) + } + return forbiddenApisCli + } + + private static File getForbiddenSignaturesFile(project, String name) { + return project.rootProject.file('buildSrc/src/main/resources/forbidden/' + name + '.txt') + } + private static Task configureCheckstyle(Project project) { // Always copy the checkstyle configuration files to 'buildDir/checkstyle' since the resources could be located in a jar // file. If the resources are located in a jar, Gradle will fail when it tries to turn the URL into a file From 4f3096323142424e8114dd22bf33b08bfcedbbc2 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 17 Jul 2018 14:45:30 +0300 Subject: [PATCH 02/10] PR review --- .../org/elasticsearch/gradle/precommit/PrecommitTasks.groovy | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 77d2670bdafdb..44aea81667fbc 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -113,7 +113,6 @@ class PrecommitTasks { forbiddenApisCliJar 'de.thetaphi:forbiddenapis:2.5' } Task forbiddenApisCli = project.tasks.create('forbiddenApisCli') - project.ext.getForbiddenSignaturesFile = { name -> project.getForbiddenSignaturesFile(name) } project.sourceSets.forEach { sourceSet -> forbiddenApisCli.dependsOn( @@ -155,7 +154,7 @@ class PrecommitTasks { return forbiddenApisCli } - private static File getForbiddenSignaturesFile(project, String name) { + private static File getForbiddenSignaturesFile(Project project, String name) { return project.rootProject.file('buildSrc/src/main/resources/forbidden/' + name + '.txt') } From 60570d7ea231ba50ff7921432eb7d9be8b4c0f99 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 18 Jul 2018 17:25:32 +0300 Subject: [PATCH 03/10] Add failing test to highlight paths issue Fails with: ``` ERROR: IO problem while reading files with API signatures: java.io.FileNotFoundException: /home/alpar/work/elastic/elasticsearch/buildSrc/src/testKit/forbidden-apis/buildSrc/src/main/resources/forbidden/jdk-signatures.txt (No such file or directory) ``` because signature definitiions are accessed on the FS directly. --- .../precommit/ForbiddenApisCLITaskIT.java | 34 +++++++++++++++++++ .../precommit/NamingConventionsTaskIT.java | 15 +++----- .../test/GradleIntegrationTestCase.java | 12 +++++++ .../src/testKit/forbidden-apis/build.gradle | 7 ++++ .../test/UnsafeImplementationClass.java | 28 +++++++++++++++ 5 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ForbiddenApisCLITaskIT.java create mode 100644 buildSrc/src/testKit/forbidden-apis/build.gradle create mode 100644 buildSrc/src/testKit/forbidden-apis/src/main/java/org/elasticsearch/test/UnsafeImplementationClass.java diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ForbiddenApisCLITaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ForbiddenApisCLITaskIT.java new file mode 100644 index 0000000000000..54b432120aa93 --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ForbiddenApisCLITaskIT.java @@ -0,0 +1,34 @@ +/* + * 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.precommit; + +import org.elasticsearch.gradle.test.GradleIntegrationTestCase; +import org.gradle.testkit.runner.BuildResult; + +public class ForbiddenApisCLITaskIT extends GradleIntegrationTestCase { + + private static final String SAMPLE_PROJECT = "forbidden-apis"; + + public void testRunCliTask() { + BuildResult result = getGradleRunner(SAMPLE_PROJECT) + .withArguments("forbiddenApisCli", "-s") + .build(); + } + +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java index 7e469e8597ddd..ddb5f4d9aff49 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java @@ -2,18 +2,17 @@ import org.elasticsearch.gradle.test.GradleIntegrationTestCase; import org.gradle.testkit.runner.BuildResult; -import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; import java.util.Arrays; public class NamingConventionsTaskIT extends GradleIntegrationTestCase { + public static final String SAMPLE_PROJECT = "namingConventionsSelfTest"; + public void testPluginCanBeApplied() { - BuildResult result = GradleRunner.create() - .withProjectDir(getProjectDir("namingConventionsSelfTest")) + BuildResult result = getGradleRunner(SAMPLE_PROJECT) .withArguments("hello", "-s", "-PcheckForTestsInMain=false") - .withPluginClasspath() .build(); assertEquals(TaskOutcome.SUCCESS, result.task(":hello").getOutcome()); @@ -22,10 +21,8 @@ public void testPluginCanBeApplied() { } public void testNameCheckFailsAsItShould() { - BuildResult result = GradleRunner.create() - .withProjectDir(getProjectDir("namingConventionsSelfTest")) + BuildResult result = getGradleRunner(SAMPLE_PROJECT) .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=false") - .withPluginClasspath() .buildAndFail(); assertNotNull("task did not run", result.task(":namingConventions")); @@ -47,10 +44,8 @@ public void testNameCheckFailsAsItShould() { } public void testNameCheckFailsAsItShouldWithMain() { - BuildResult result = GradleRunner.create() - .withProjectDir(getProjectDir("namingConventionsSelfTest")) + BuildResult result = getGradleRunner(SAMPLE_PROJECT) .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=true") - .withPluginClasspath() .buildAndFail(); assertNotNull("task did not run", result.task(":namingConventions")); diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java index 26da663182f7c..f124195b6aa17 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -1,5 +1,7 @@ package org.elasticsearch.gradle.test; +import org.gradle.testkit.runner.GradleRunner; + import java.io.File; public abstract class GradleIntegrationTestCase extends GradleUnitTestCase { @@ -13,4 +15,14 @@ protected File getProjectDir(String name) { return new File(root, name); } + protected GradleRunner getGradleRunner(String sampleProject) { + return GradleRunner.create() + .withProjectDir(getProjectDir(sampleProject)) + .withPluginClasspath( + //Collections.singleton( + // new File("/home/alpar/work/elastic/elasticsearch/buildSrc/build/pluginUnderTestMetaDataWithJar/") + //) + ); + } + } diff --git a/buildSrc/src/testKit/forbidden-apis/build.gradle b/buildSrc/src/testKit/forbidden-apis/build.gradle new file mode 100644 index 0000000000000..35ee9e5393e1f --- /dev/null +++ b/buildSrc/src/testKit/forbidden-apis/build.gradle @@ -0,0 +1,7 @@ +plugins { + id 'java' + id 'elasticsearch.build' +} + +ext.licenseFile = file("$buildDir/dummy/license") +ext.noticeFile = file("$buildDir/dummy/notice") \ No newline at end of file diff --git a/buildSrc/src/testKit/forbidden-apis/src/main/java/org/elasticsearch/test/UnsafeImplementationClass.java b/buildSrc/src/testKit/forbidden-apis/src/main/java/org/elasticsearch/test/UnsafeImplementationClass.java new file mode 100644 index 0000000000000..a64734f1d9374 --- /dev/null +++ b/buildSrc/src/testKit/forbidden-apis/src/main/java/org/elasticsearch/test/UnsafeImplementationClass.java @@ -0,0 +1,28 @@ +/* + * 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.test; + + +public class UnsafeImplementationClass { + + public UnsafeImplementationClass() { + + } +} From 0fbfdfb260a7f25f4625774f82a1c73042bddd20 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Thu, 19 Jul 2018 12:31:15 +0300 Subject: [PATCH 04/10] Task skeleton with test failing due to missing implementation --- .../elasticsearch/gradle/BuildPlugin.groovy | 3 + .../gradle/ElasticsearchBuildResource.java | 39 ++++++ ...ExportElasticsearchBuildResourcesTask.java | 131 ++++++++++++++++++ ...portElasticsearchBuildResourcesTaskIT.java | 62 +++++++++ .../build.gradle | 12 ++ 5 files changed, 247 insertions(+) create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchBuildResource.java create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java create mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java create mode 100644 buildSrc/src/testKit/elasticsearch-build-resources/build.gradle diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index 219d00ba64032..99437669d3449 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -77,6 +77,8 @@ class BuildPlugin implements Plugin { project.pluginManager.apply('nebula.info-scm') project.pluginManager.apply('nebula.info-jar') + project.getTasks().create("exportBuildResources", ExportElasticsearchBuildResourcesTask) + globalBuildInfo(project) configureRepositories(project) configureConfigurations(project) @@ -91,6 +93,7 @@ class BuildPlugin implements Plugin { configureDependenciesInfo(project) } + /** Performs checks on the build environment and prints information about the build environment. */ static void globalBuildInfo(Project project) { if (project.rootProject.ext.has('buildChecksDone') == false) { diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchBuildResource.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchBuildResource.java new file mode 100644 index 0000000000000..b4c530fe148e3 --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchBuildResource.java @@ -0,0 +1,39 @@ +/* + * 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; + +/** + * Enumerate resources in the build that are meant for consumption outside of the build code. + */ +public enum ElasticsearchBuildResource { + CHECKSTYLE_CONF("/checkstyle.xml"), + CHECKSTYLE_SURPRESSIONS_CONF("/checkstyle_suppressions.xml") + ; + + private final String fullName; + + ElasticsearchBuildResource(String fullName) { + this.fullName = fullName; + } + + public String getFullName() { + return fullName; + } + +} diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java new file mode 100644 index 0000000000000..961d0e6922ddf --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java @@ -0,0 +1,131 @@ +/* + * 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.DefaultTask; +import org.gradle.api.logging.Logger; +import org.gradle.api.logging.Logging; +import org.gradle.api.tasks.Classpath; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.OutputFiles; +import org.gradle.api.tasks.SkipWhenEmpty; +import org.gradle.api.tasks.StopExecutionException; +import org.gradle.api.tasks.TaskAction; + +import java.io.File; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Export Elasticsearch build resources to configurable paths + * + * Wil overwrite existing files and create missing directories. + * Useful for resources that that need to be passed to other processes trough the filesystem or otherwise can't be + * consumed from the classpath. + */ +public class ExportElasticsearchBuildResourcesTask extends DefaultTask { + + private final Logger logger = Logging.getLogger(ExportElasticsearchBuildResourcesTask.class); + + private final Set resources = new HashSet<>(); + + @Input + @SkipWhenEmpty + public Set getResources() { + return Collections.unmodifiableSet(resources); + } + + @Classpath + public String getResourceSourceFiles() { + logger.info("Classpath: {}", System.getProperty("java.class.path")); + return System.getProperty("java.class.path"); + } + + @OutputFiles + public List getOutputFiles() { + return resources.stream() + .map(ExportPair::getOutputFile) + .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); + } + + public void resource(Map resourcesMap) { + resourcesMap.forEach(this::resource); + } + + public void resource(ElasticsearchBuildResource resource, File outputFile) { + resources.add(new ExportPair(resource, outputFile)); + } + + @TaskAction + public void doExport() { + if (resources.isEmpty()) { + throw new StopExecutionException(); + } + logger.info("exporting resources"); + } + + // Gradle wants this to be Serializable, we achieve this by extending list, + // also alows for variable expansion in Groovy. + public static class ExportPair extends ArrayList { + private final ElasticsearchBuildResource resource; + private final File outputFile; + + public ExportPair(ElasticsearchBuildResource resource, File outputFile) { + this.resource = resource; + this.outputFile = outputFile; + super.add(resource); + super.add(outputFile); + } + + public ElasticsearchBuildResource getResource() { + return resource; + } + + public File getOutputFile() { + return outputFile; + } + + @Override + public boolean add(Object o) { + throw new UnsupportedOperationException(); + } + + @Override + public void add(int index, Object element) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean addAll(Collection c) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean addAll(int index, Collection c) { + throw new UnsupportedOperationException(); + } + } + +} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java new file mode 100644 index 0000000000000..3e8fe1173ba2e --- /dev/null +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java @@ -0,0 +1,62 @@ +package org.elasticsearch.gradle; + +/* + * 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. + */ + +import org.elasticsearch.gradle.test.GradleIntegrationTestCase; +import org.gradle.testkit.runner.BuildResult; +import org.gradle.testkit.runner.GradleRunner; +import org.gradle.testkit.runner.TaskOutcome; + +public class ExportElasticsearchBuildResourcesTaskIT extends GradleIntegrationTestCase { + + public void testUpToDate() { + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("elasticsearch-build-resources")) + .withArguments("exportBuildResources", "-s") + .withPluginClasspath() + .build(); + assertEquals(TaskOutcome.UP_TO_DATE, result.task(":exportBuildResources").getOutcome()); + } + + public void testUpToDateWithSourcesConfigured() { + GradleRunner.create() + .withProjectDir(getProjectDir("elasticsearch-build-resources")) + .withArguments("clean", "-s") + .withPluginClasspath() + .build(); + + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("elasticsearch-build-resources")) + .withArguments("exportCheckstyle", "-s", "-i") + .withPluginClasspath() + .build(); + assertEquals(TaskOutcome.SUCCESS, result.task(":exportCheckstyle").getOutcome()); + + result = GradleRunner.create() + .withProjectDir(getProjectDir("elasticsearch-build-resources")) + .withArguments("exportCheckstyle", "-s", "-i") + .withPluginClasspath() + .build(); + assertEquals(TaskOutcome.UP_TO_DATE, result.task(":exportCheckstyle").getOutcome()); + + + } + +} diff --git a/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle b/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle new file mode 100644 index 0000000000000..94f2b74bbb076 --- /dev/null +++ b/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle @@ -0,0 +1,12 @@ +import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask + +plugins { + id 'elasticsearch.build' +} + +ext.licenseFile = file("$buildDir/dummy/license") +ext.noticeFile = file("$buildDir/dummy/notice") + +task (exportCheckstyle, type: ExportElasticsearchBuildResourcesTask) { + resource 'CHECKSTYLE_CONF', file("$buildDir/checkstyle-1.xml") +} \ No newline at end of file From 02d7bf02032052820ced9f31f388d75f6394f608 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Thu, 19 Jul 2018 17:39:20 +0300 Subject: [PATCH 05/10] Task implementation and complete task --- ...ExportElasticsearchBuildResourcesTask.java | 26 +++++++++++++++++- ...portElasticsearchBuildResourcesTaskIT.java | 27 ++++++++++--------- .../test/GradleIntegrationTestCase.java | 4 +++ .../build.gradle | 8 +++--- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java index 961d0e6922ddf..9d5464484a866 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java @@ -18,7 +18,9 @@ */ package org.elasticsearch.gradle; +import groovy.lang.GString; import org.gradle.api.DefaultTask; +import org.gradle.api.GradleException; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.tasks.Classpath; @@ -29,6 +31,9 @@ import org.gradle.api.tasks.TaskAction; import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -78,12 +83,31 @@ public void resource(ElasticsearchBuildResource resource, File outputFile) { resources.add(new ExportPair(resource, outputFile)); } + public void resource(ElasticsearchBuildResource resource, String outputFile) { + resources.add(new ExportPair(resource, getProject().file(outputFile))); + } + + public void resource(ElasticsearchBuildResource resource, GString outputFile) { + resources.add(new ExportPair(resource, getProject().file(outputFile))); + } + @TaskAction public void doExport() { if (resources.isEmpty()) { throw new StopExecutionException(); } - logger.info("exporting resources"); + resources.stream().parallel() + .forEach(pair -> { + try { + try(InputStream is = getClass().getResourceAsStream(pair.getResource().getFullName())) { + Files.copy(is, pair.getOutputFile().toPath()); + } + } catch (IOException e) { + throw new GradleException( + "Can't write resource " + pair.getResource().name() + " to " + pair.getOutputFile() + ); + } + }); } // Gradle wants this to be Serializable, we achieve this by extending list, diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java index 3e8fe1173ba2e..7da95dca2797e 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java @@ -24,37 +24,38 @@ import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; +import java.io.File; + public class ExportElasticsearchBuildResourcesTaskIT extends GradleIntegrationTestCase { - public void testUpToDate() { - BuildResult result = GradleRunner.create() - .withProjectDir(getProjectDir("elasticsearch-build-resources")) - .withArguments("exportBuildResources", "-s") - .withPluginClasspath() - .build(); - assertEquals(TaskOutcome.UP_TO_DATE, result.task(":exportBuildResources").getOutcome()); - } + public static final String PROJECT_NAME = "elasticsearch-build-resources"; public void testUpToDateWithSourcesConfigured() { GradleRunner.create() - .withProjectDir(getProjectDir("elasticsearch-build-resources")) + .withProjectDir(getProjectDir(PROJECT_NAME)) .withArguments("clean", "-s") .withPluginClasspath() .build(); BuildResult result = GradleRunner.create() .withProjectDir(getProjectDir("elasticsearch-build-resources")) - .withArguments("exportCheckstyle", "-s", "-i") + .withArguments("exportBuildResources", "-s", "-i") .withPluginClasspath() .build(); - assertEquals(TaskOutcome.SUCCESS, result.task(":exportCheckstyle").getOutcome()); + assertEquals(TaskOutcome.SUCCESS, result.task(":exportBuildResources").getOutcome()); + assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-1.xml").exists()); + assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-2.xml").exists()); + assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-3.xml").exists()); result = GradleRunner.create() .withProjectDir(getProjectDir("elasticsearch-build-resources")) - .withArguments("exportCheckstyle", "-s", "-i") + .withArguments("exportBuildResources", "-s", "-i") .withPluginClasspath() .build(); - assertEquals(TaskOutcome.UP_TO_DATE, result.task(":exportCheckstyle").getOutcome()); + assertEquals(TaskOutcome.UP_TO_DATE, result.task(":exportBuildResources").getOutcome()); + assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-1.xml").exists()); + assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-2.xml").exists()); + assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-3.xml").exists()); } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java index 26da663182f7c..f83a88e6ae1b0 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -13,4 +13,8 @@ protected File getProjectDir(String name) { return new File(root, name); } + protected File getBuildDir(String name) { + return new File(getProjectDir(name), "build"); + } + } diff --git a/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle b/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle index 94f2b74bbb076..57ce586bd990c 100644 --- a/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle +++ b/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle @@ -1,5 +1,3 @@ -import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask - plugins { id 'elasticsearch.build' } @@ -7,6 +5,8 @@ plugins { ext.licenseFile = file("$buildDir/dummy/license") ext.noticeFile = file("$buildDir/dummy/notice") -task (exportCheckstyle, type: ExportElasticsearchBuildResourcesTask) { +exportBuildResources { resource 'CHECKSTYLE_CONF', file("$buildDir/checkstyle-1.xml") -} \ No newline at end of file + resource 'CHECKSTYLE_CONF', "$buildDir/checkstyle-2.xml" + resource 'checkstyle_conf', file("$buildDir/checkstyle-3.xml") +} From 5bb35ac6175b35ab0e558050887586db84660d07 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 25 Jul 2018 01:13:41 +0300 Subject: [PATCH 06/10] No more whitelist, reduce boilerplate --- ...ExportElasticsearchBuildResourcesTask.java | 116 ++++++------------ ...portElasticsearchBuildResourcesTaskIT.java | 54 ++++++-- .../test/GradleIntegrationTestCase.java | 2 +- .../build.gradle | 23 +++- 4 files changed, 100 insertions(+), 95 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java index 9d5464484a866..67d9fd26abab3 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java @@ -18,77 +18,73 @@ */ package org.elasticsearch.gradle; -import groovy.lang.GString; import org.gradle.api.DefaultTask; import org.gradle.api.GradleException; +import org.gradle.api.file.DirectoryProperty; +import org.gradle.api.file.RegularFileProperty; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.tasks.Classpath; import org.gradle.api.tasks.Input; -import org.gradle.api.tasks.OutputFiles; +import org.gradle.api.tasks.OutputDirectory; import org.gradle.api.tasks.SkipWhenEmpty; import org.gradle.api.tasks.StopExecutionException; import org.gradle.api.tasks.TaskAction; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; -import java.util.ArrayList; -import java.util.Collection; +import java.nio.file.Path; import java.util.Collections; import java.util.HashSet; -import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; /** * Export Elasticsearch build resources to configurable paths - * + *

* Wil overwrite existing files and create missing directories. * Useful for resources that that need to be passed to other processes trough the filesystem or otherwise can't be * consumed from the classpath. */ public class ExportElasticsearchBuildResourcesTask extends DefaultTask { - private final Logger logger = Logging.getLogger(ExportElasticsearchBuildResourcesTask.class); + private final Logger logger = Logging.getLogger(ExportElasticsearchBuildResourcesTask.class); + + private final Set resources = new HashSet<>(); - private final Set resources = new HashSet<>(); + private DirectoryProperty outputDir; + + public ExportElasticsearchBuildResourcesTask() { + outputDir = getProject().getLayout().directoryProperty( + getProject().getLayout().getBuildDirectory().dir("build-tools-exported") + ); + } + + @OutputDirectory + public DirectoryProperty getOutputDir() { + return outputDir; + } @Input @SkipWhenEmpty - public Set getResources() { + public Set getResources() { return Collections.unmodifiableSet(resources); } @Classpath - public String getResourceSourceFiles() { + public String getResourcesClasspath() { + // This will make sure the task is not considered up to date if the resources are changed. logger.info("Classpath: {}", System.getProperty("java.class.path")); return System.getProperty("java.class.path"); } - @OutputFiles - public List getOutputFiles() { - return resources.stream() - .map(ExportPair::getOutputFile) - .collect(Collectors.collectingAndThen(Collectors.toList(), Collections::unmodifiableList)); - } - - public void resource(Map resourcesMap) { - resourcesMap.forEach(this::resource); - } - - public void resource(ElasticsearchBuildResource resource, File outputFile) { - resources.add(new ExportPair(resource, outputFile)); + public void setOutputDir(DirectoryProperty outputDir) { + this.outputDir = outputDir; } - public void resource(ElasticsearchBuildResource resource, String outputFile) { - resources.add(new ExportPair(resource, getProject().file(outputFile))); - } - - public void resource(ElasticsearchBuildResource resource, GString outputFile) { - resources.add(new ExportPair(resource, getProject().file(outputFile))); + public RegularFileProperty resource(String resource) { + resources.add(resource); + return getProject().getLayout().fileProperty(outputDir.file(resource)); } @TaskAction @@ -97,59 +93,17 @@ public void doExport() { throw new StopExecutionException(); } resources.stream().parallel() - .forEach(pair -> { - try { - try(InputStream is = getClass().getResourceAsStream(pair.getResource().getFullName())) { - Files.copy(is, pair.getOutputFile().toPath()); + .forEach(resourcePath -> { + Path destination = outputDir.get().file(resourcePath).getAsFile().toPath(); + try (InputStream is = getClass().getClassLoader().getResourceAsStream(resourcePath)) { + if (is == null) { + throw new GradleException("Can't export `" + resourcePath + "` from build-tools: not found"); } + Files.copy(is, destination); } catch (IOException e) { - throw new GradleException( - "Can't write resource " + pair.getResource().name() + " to " + pair.getOutputFile() - ); + throw new GradleException("Can't write resource `" + resourcePath + "` to " + destination); } }); } - // Gradle wants this to be Serializable, we achieve this by extending list, - // also alows for variable expansion in Groovy. - public static class ExportPair extends ArrayList { - private final ElasticsearchBuildResource resource; - private final File outputFile; - - public ExportPair(ElasticsearchBuildResource resource, File outputFile) { - this.resource = resource; - this.outputFile = outputFile; - super.add(resource); - super.add(outputFile); - } - - public ElasticsearchBuildResource getResource() { - return resource; - } - - public File getOutputFile() { - return outputFile; - } - - @Override - public boolean add(Object o) { - throw new UnsupportedOperationException(); - } - - @Override - public void add(int index, Object element) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean addAll(Collection c) { - throw new UnsupportedOperationException(); - } - - @Override - public boolean addAll(int index, Collection c) { - throw new UnsupportedOperationException(); - } - } - } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java index 7da95dca2797e..3c269e01903f2 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTaskIT.java @@ -21,10 +21,12 @@ import org.elasticsearch.gradle.test.GradleIntegrationTestCase; import org.gradle.testkit.runner.BuildResult; +import org.gradle.testkit.runner.BuildTask; import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; -import java.io.File; +import java.nio.file.Files; +import java.nio.file.Path; public class ExportElasticsearchBuildResourcesTaskIT extends GradleIntegrationTestCase { @@ -38,26 +40,58 @@ public void testUpToDateWithSourcesConfigured() { .build(); BuildResult result = GradleRunner.create() - .withProjectDir(getProjectDir("elasticsearch-build-resources")) + .withProjectDir(getProjectDir(PROJECT_NAME)) .withArguments("exportBuildResources", "-s", "-i") .withPluginClasspath() .build(); - assertEquals(TaskOutcome.SUCCESS, result.task(":exportBuildResources").getOutcome()); - assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-1.xml").exists()); - assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-2.xml").exists()); - assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-3.xml").exists()); + assertTaskSuccessFull(result, ":exportBuildResources"); + assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml"); + result = GradleRunner.create() - .withProjectDir(getProjectDir("elasticsearch-build-resources")) + .withProjectDir(getProjectDir(PROJECT_NAME)) .withArguments("exportBuildResources", "-s", "-i") .withPluginClasspath() .build(); assertEquals(TaskOutcome.UP_TO_DATE, result.task(":exportBuildResources").getOutcome()); - assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-1.xml").exists()); - assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-2.xml").exists()); - assertTrue(new File(getBuildDir(PROJECT_NAME), "checkstyle-3.xml").exists()); + assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml"); + } + + public void testImplicitTaskDependencyCopy() { + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir(PROJECT_NAME)) + .withArguments("clean", "sampleCopyAll", "-s", "-i") + .withPluginClasspath() + .build(); + assertTaskSuccessFull(result, ":exportBuildResources"); + assertTaskSuccessFull(result, ":sampleCopyAll"); + assertBuildFileExists(result, PROJECT_NAME, "sampleCopyAll/checkstyle.xml"); + } + public void testImplicitTaskDependencyInputFileOfOther() { + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir(PROJECT_NAME)) + .withArguments("clean", "sample", "-s", "-i") + .withPluginClasspath() + .build(); + + assertBuildFileExists(result, PROJECT_NAME, "build-tools-exported/checkstyle.xml"); + } + + private void assertTaskSuccessFull(BuildResult result, String taskName) { + BuildTask task = result.task(taskName); + if (task == null) { + fail("Expected task `" + taskName + "` to be successful, but it did not run"); + } + assertEquals(TaskOutcome.SUCCESS, task.getOutcome()); + } + private void assertBuildFileExists(BuildResult result, String projectName, String path) { + Path absPath = getBuildDir(projectName).toPath().resolve(path); + assertTrue( + result.getOutput() + "\n\nExpected `" + absPath + "` to exists but it did not", + Files.exists(absPath) + ); } } diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java index f83a88e6ae1b0..3b2c7c3d639b6 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -10,7 +10,7 @@ protected File getProjectDir(String name) { throw new RuntimeException("Could not find resources dir for integration tests. " + "Note that these tests can only be ran by Gradle and are not currently supported by the IDE"); } - return new File(root, name); + return new File(root, name).getAbsoluteFile(); } protected File getBuildDir(String name) { diff --git a/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle b/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle index 57ce586bd990c..cc618918a726b 100644 --- a/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle +++ b/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle @@ -6,7 +6,24 @@ ext.licenseFile = file("$buildDir/dummy/license") ext.noticeFile = file("$buildDir/dummy/notice") exportBuildResources { - resource 'CHECKSTYLE_CONF', file("$buildDir/checkstyle-1.xml") - resource 'CHECKSTYLE_CONF', "$buildDir/checkstyle-2.xml" - resource 'checkstyle_conf', file("$buildDir/checkstyle-3.xml") + resource 'checkstyle.xml' + resource 'minimumCompilerVersion' } + +task sampleCopyAll(type: Sync) { + /** Note: no explicit dependency. This works with tasks that use the Provider API a.k.a "Lazy Configuration" **/ + from exportBuildResources + into "$buildDir/sampleCopyAll" +} + +task sample { + // This does not work, task dependencies can't be providers + // dependsOn exportBuildResources.resource('minimumRuntimeVersion') + // Nor does this, despite https://github.com/gradle/gradle/issues/3811 + // dependsOn exportBuildResources.outputDir + // for now it's just + dependsOn exportBuildResources + doLast { + println "This task is using ${file(exportBuildResources.resource('minimumRuntimeVersion'))}" + } +} \ No newline at end of file From 8cfbd6737b14b1dbad7721c914cde6d0819b3578 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 7 Aug 2018 08:12:11 +0300 Subject: [PATCH 07/10] remove unused enum --- .../gradle/ElasticsearchBuildResource.java | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchBuildResource.java diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchBuildResource.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchBuildResource.java deleted file mode 100644 index b4c530fe148e3..0000000000000 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchBuildResource.java +++ /dev/null @@ -1,39 +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; - -/** - * Enumerate resources in the build that are meant for consumption outside of the build code. - */ -public enum ElasticsearchBuildResource { - CHECKSTYLE_CONF("/checkstyle.xml"), - CHECKSTYLE_SURPRESSIONS_CONF("/checkstyle_suppressions.xml") - ; - - private final String fullName; - - ElasticsearchBuildResource(String fullName) { - this.fullName = fullName; - } - - public String getFullName() { - return fullName; - } - -} From f771138560198cad06d28f263a071f34d3d469c2 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 8 Aug 2018 08:03:43 +0300 Subject: [PATCH 08/10] Revert "Add failing test to highlight paths issue" This reverts commit 60570d7ea231ba50ff7921432eb7d9be8b4c0f99. --- .../precommit/ForbiddenApisCLITaskIT.java | 34 ------------------- .../precommit/NamingConventionsTaskIT.java | 15 +++++--- .../test/GradleIntegrationTestCase.java | 12 ------- .../src/testKit/forbidden-apis/build.gradle | 7 ---- .../test/UnsafeImplementationClass.java | 28 --------------- 5 files changed, 10 insertions(+), 86 deletions(-) delete mode 100644 buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ForbiddenApisCLITaskIT.java delete mode 100644 buildSrc/src/testKit/forbidden-apis/build.gradle delete mode 100644 buildSrc/src/testKit/forbidden-apis/src/main/java/org/elasticsearch/test/UnsafeImplementationClass.java diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ForbiddenApisCLITaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ForbiddenApisCLITaskIT.java deleted file mode 100644 index 54b432120aa93..0000000000000 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/ForbiddenApisCLITaskIT.java +++ /dev/null @@ -1,34 +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.precommit; - -import org.elasticsearch.gradle.test.GradleIntegrationTestCase; -import org.gradle.testkit.runner.BuildResult; - -public class ForbiddenApisCLITaskIT extends GradleIntegrationTestCase { - - private static final String SAMPLE_PROJECT = "forbidden-apis"; - - public void testRunCliTask() { - BuildResult result = getGradleRunner(SAMPLE_PROJECT) - .withArguments("forbiddenApisCli", "-s") - .build(); - } - -} diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java index ddb5f4d9aff49..7e469e8597ddd 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/precommit/NamingConventionsTaskIT.java @@ -2,17 +2,18 @@ import org.elasticsearch.gradle.test.GradleIntegrationTestCase; import org.gradle.testkit.runner.BuildResult; +import org.gradle.testkit.runner.GradleRunner; import org.gradle.testkit.runner.TaskOutcome; import java.util.Arrays; public class NamingConventionsTaskIT extends GradleIntegrationTestCase { - public static final String SAMPLE_PROJECT = "namingConventionsSelfTest"; - public void testPluginCanBeApplied() { - BuildResult result = getGradleRunner(SAMPLE_PROJECT) + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("namingConventionsSelfTest")) .withArguments("hello", "-s", "-PcheckForTestsInMain=false") + .withPluginClasspath() .build(); assertEquals(TaskOutcome.SUCCESS, result.task(":hello").getOutcome()); @@ -21,8 +22,10 @@ public void testPluginCanBeApplied() { } public void testNameCheckFailsAsItShould() { - BuildResult result = getGradleRunner(SAMPLE_PROJECT) + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("namingConventionsSelfTest")) .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=false") + .withPluginClasspath() .buildAndFail(); assertNotNull("task did not run", result.task(":namingConventions")); @@ -44,8 +47,10 @@ public void testNameCheckFailsAsItShould() { } public void testNameCheckFailsAsItShouldWithMain() { - BuildResult result = getGradleRunner(SAMPLE_PROJECT) + BuildResult result = GradleRunner.create() + .withProjectDir(getProjectDir("namingConventionsSelfTest")) .withArguments("namingConventions", "-s", "-PcheckForTestsInMain=true") + .withPluginClasspath() .buildAndFail(); assertNotNull("task did not run", result.task(":namingConventions")); diff --git a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java index f124195b6aa17..26da663182f7c 100644 --- a/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java +++ b/buildSrc/src/test/java/org/elasticsearch/gradle/test/GradleIntegrationTestCase.java @@ -1,7 +1,5 @@ package org.elasticsearch.gradle.test; -import org.gradle.testkit.runner.GradleRunner; - import java.io.File; public abstract class GradleIntegrationTestCase extends GradleUnitTestCase { @@ -15,14 +13,4 @@ protected File getProjectDir(String name) { return new File(root, name); } - protected GradleRunner getGradleRunner(String sampleProject) { - return GradleRunner.create() - .withProjectDir(getProjectDir(sampleProject)) - .withPluginClasspath( - //Collections.singleton( - // new File("/home/alpar/work/elastic/elasticsearch/buildSrc/build/pluginUnderTestMetaDataWithJar/") - //) - ); - } - } diff --git a/buildSrc/src/testKit/forbidden-apis/build.gradle b/buildSrc/src/testKit/forbidden-apis/build.gradle deleted file mode 100644 index 35ee9e5393e1f..0000000000000 --- a/buildSrc/src/testKit/forbidden-apis/build.gradle +++ /dev/null @@ -1,7 +0,0 @@ -plugins { - id 'java' - id 'elasticsearch.build' -} - -ext.licenseFile = file("$buildDir/dummy/license") -ext.noticeFile = file("$buildDir/dummy/notice") \ No newline at end of file diff --git a/buildSrc/src/testKit/forbidden-apis/src/main/java/org/elasticsearch/test/UnsafeImplementationClass.java b/buildSrc/src/testKit/forbidden-apis/src/main/java/org/elasticsearch/test/UnsafeImplementationClass.java deleted file mode 100644 index a64734f1d9374..0000000000000 --- a/buildSrc/src/testKit/forbidden-apis/src/main/java/org/elasticsearch/test/UnsafeImplementationClass.java +++ /dev/null @@ -1,28 +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.test; - - -public class UnsafeImplementationClass { - - public UnsafeImplementationClass() { - - } -} From 3249464e82c052db88ca884881c0399a04fa471d Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Wed, 8 Aug 2018 14:30:17 +0300 Subject: [PATCH 09/10] Use the build resources task for running forbidden apis with teh CLI --- .../gradle/precommit/PrecommitTasks.groovy | 28 +++++++++---------- .../test/StandaloneRestTestPlugin.groovy | 4 +-- ...ExportElasticsearchBuildResourcesTask.java | 12 ++++---- 3 files changed, 22 insertions(+), 22 deletions(-) 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 44aea81667fbc..976d9084f24ed 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -20,6 +20,7 @@ package org.elasticsearch.gradle.precommit import de.thetaphi.forbiddenapis.gradle.CheckForbiddenApis import de.thetaphi.forbiddenapis.gradle.ForbiddenApisPlugin +import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask import org.gradle.api.Project import org.gradle.api.Task import org.gradle.api.file.FileCollection @@ -117,6 +118,8 @@ class PrecommitTasks { project.sourceSets.forEach { sourceSet -> forbiddenApisCli.dependsOn( project.tasks.create(sourceSet.getTaskName('forbiddenApisCli', null), JavaExec) { + ExportElasticsearchBuildResourcesTask exportBuildResources = project.tasks.getByName('exportBuildResources') + dependsOn(exportBuildResources) classpath = project.files( project.configurations.forbiddenApisCliJar, sourceSet.compileClasspath, @@ -124,20 +127,19 @@ class PrecommitTasks { ) main = 'de.thetaphi.forbiddenapis.cli.CliMain' executable = "${project.runtimeJavaHome}/bin/java" - [ - 'jdk-unsafe-1.8', 'jdk-deprecated-1.8', 'jdk-non-portable', 'jdk-system-out' - ].forEach { args "-b", it } - [ - getForbiddenSignaturesFile(project, 'jdk-signatures'), - getForbiddenSignaturesFile(project, 'es-all-signatures') - ].forEach { args "-f", it } + args "-b", 'jdk-unsafe-1.8' + args "-b", 'jdk-deprecated-1.8' + args "-b", 'jdk-non-portable' + args "-b", 'jdk-system-out' + args "-f", exportBuildResources.resource("forbidden/jdk-signatures.txt") + args "-f", exportBuildResources.resource("forbidden/es-all-signatures.txt") + args "--suppressannotation", '**.SuppressForbidden' if (sourceSet.name == 'test') { - args "-f", getForbiddenSignaturesFile(project, 'es-test-signatures') - args "-f", getForbiddenSignaturesFile(project, 'http-signatures') + args "-f", exportBuildResources.resource("forbidden/es-test-signatures.txt") + args "-f", exportBuildResources.resource("forbidden/http-signatures.txt") } else { - args "-f", getForbiddenSignaturesFile(project, 'es-server-signatures') + args "-f", exportBuildResources.resource("forbidden/es-server-signatures.txt") } - args "--suppressannotation", '**.SuppressForbidden' dependsOn sourceSet.classesTaskName doFirst { // Forbidden APIs expects only existing dirs, and requires at least one @@ -154,10 +156,6 @@ class PrecommitTasks { return forbiddenApisCli } - private static File getForbiddenSignaturesFile(Project project, String name) { - return project.rootProject.file('buildSrc/src/main/resources/forbidden/' + name + '.txt') - } - private static Task configureCheckstyle(Project project) { // Always copy the checkstyle configuration files to 'buildDir/checkstyle' since the resources could be located in a jar // file. If the resources are located in a jar, Gradle will fail when it tries to turn the URL into a file diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneRestTestPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneRestTestPlugin.groovy index 390821c80ff39..5c8ce0d392a01 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneRestTestPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/StandaloneRestTestPlugin.groovy @@ -22,15 +22,14 @@ package org.elasticsearch.gradle.test import com.carrotsearch.gradle.junit4.RandomizedTestingPlugin import org.elasticsearch.gradle.BuildPlugin +import org.elasticsearch.gradle.ExportElasticsearchBuildResourcesTask import org.elasticsearch.gradle.VersionProperties import org.elasticsearch.gradle.precommit.PrecommitTasks import org.gradle.api.InvalidUserDataException import org.gradle.api.Plugin import org.gradle.api.Project -import org.gradle.api.Task import org.gradle.api.plugins.JavaBasePlugin import org.gradle.api.tasks.compile.JavaCompile - /** * Configures the build to compile tests against Elasticsearch's test framework * and run REST tests. Use BuildPlugin if you want to build main code as well @@ -48,6 +47,7 @@ public class StandaloneRestTestPlugin implements Plugin { project.pluginManager.apply(JavaBasePlugin) project.pluginManager.apply(RandomizedTestingPlugin) + project.getTasks().create("exportBuildResources", ExportElasticsearchBuildResourcesTask) BuildPlugin.globalBuildInfo(project) BuildPlugin.configureRepositories(project) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java index 67d9fd26abab3..0db7859c74488 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java @@ -21,7 +21,6 @@ import org.gradle.api.DefaultTask; import org.gradle.api.GradleException; import org.gradle.api.file.DirectoryProperty; -import org.gradle.api.file.RegularFileProperty; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.tasks.Classpath; @@ -31,10 +30,12 @@ import org.gradle.api.tasks.StopExecutionException; import org.gradle.api.tasks.TaskAction; +import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.Collections; import java.util.HashSet; import java.util.Set; @@ -82,9 +83,9 @@ public void setOutputDir(DirectoryProperty outputDir) { this.outputDir = outputDir; } - public RegularFileProperty resource(String resource) { + public File resource(String resource) { resources.add(resource); - return getProject().getLayout().fileProperty(outputDir.file(resource)); + return outputDir.file(resource).get().getAsFile(); } @TaskAction @@ -96,12 +97,13 @@ public void doExport() { .forEach(resourcePath -> { Path destination = outputDir.get().file(resourcePath).getAsFile().toPath(); try (InputStream is = getClass().getClassLoader().getResourceAsStream(resourcePath)) { + Files.createDirectories(destination.getParent()); if (is == null) { throw new GradleException("Can't export `" + resourcePath + "` from build-tools: not found"); } - Files.copy(is, destination); + Files.copy(is, destination, StandardCopyOption.REPLACE_EXISTING); } catch (IOException e) { - throw new GradleException("Can't write resource `" + resourcePath + "` to " + destination); + throw new GradleException("Can't write resource `" + resourcePath + "` to " + destination, e); } }); } From 55316381853a649f59c90616019d6dbf3cc3b6c0 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Mon, 13 Aug 2018 08:45:43 +0300 Subject: [PATCH 10/10] rename method to `copy` --- .../gradle/precommit/PrecommitTasks.groovy | 10 +++++----- .../gradle/ExportElasticsearchBuildResourcesTask.java | 2 +- .../testKit/elasticsearch-build-resources/build.gradle | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) 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 b985eecdc1abd..598d82d5d9aa7 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy @@ -131,14 +131,14 @@ class PrecommitTasks { args "-b", 'jdk-deprecated-1.8' args "-b", 'jdk-non-portable' args "-b", 'jdk-system-out' - args "-f", buildResources.take("forbidden/jdk-signatures.txt") - args "-f", buildResources.take("forbidden/es-all-signatures.txt") + args "-f", buildResources.copy("forbidden/jdk-signatures.txt") + args "-f", buildResources.copy("forbidden/es-all-signatures.txt") args "--suppressannotation", '**.SuppressForbidden' if (sourceSet.name == 'test') { - args "-f", buildResources.take("forbidden/es-test-signatures.txt") - args "-f", buildResources.take("forbidden/http-signatures.txt") + args "-f", buildResources.copy("forbidden/es-test-signatures.txt") + args "-f", buildResources.copy("forbidden/http-signatures.txt") } else { - args "-f", buildResources.take("forbidden/es-server-signatures.txt") + args "-f", buildResources.copy("forbidden/es-server-signatures.txt") } dependsOn sourceSet.classesTaskName doFirst { diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java index 769719a8563ac..03c18f54e67ef 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ExportElasticsearchBuildResourcesTask.java @@ -82,7 +82,7 @@ public void setOutputDir(DirectoryProperty outputDir) { this.outputDir = outputDir; } - public File take(String resource) { + public File 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" diff --git a/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle b/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle index bd5a64e5c74b8..95d1453025e92 100644 --- a/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle +++ b/buildSrc/src/testKit/elasticsearch-build-resources/build.gradle @@ -6,7 +6,7 @@ ext.licenseFile = file("$buildDir/dummy/license") ext.noticeFile = file("$buildDir/dummy/notice") buildResources { - take 'checkstyle.xml' + copy 'checkstyle.xml' } task sampleCopyAll(type: Sync) { @@ -23,7 +23,7 @@ task sample { // for now it's just dependsOn buildResources // we have to refference it at configuration time in order to be picked up - ext.checkstyle_suppressions = buildResources.take('checkstyle_suppressions.xml') + ext.checkstyle_suppressions = buildResources.copy('checkstyle_suppressions.xml') doLast { println "This task is using ${file(checkstyle_suppressions)}" } @@ -33,6 +33,6 @@ task noConfigAfterExecution { dependsOn buildResources doLast { println "This should cause an error because we are refferencing " + - "${buildResources.take('checkstyle_suppressions.xml')} after the `buildResources` task has ran." + "${buildResources.copy('checkstyle_suppressions.xml')} after the `buildResources` task has ran." } } \ No newline at end of file