From d42cb77a24dfa65ab2206da9de57654ff47e302b Mon Sep 17 00:00:00 2001 From: Rene Groeschke Date: Wed, 27 May 2020 14:28:33 +0200 Subject: [PATCH 1/2] Fix up-to-date checks for precommit related tasks - Do not use lambdas for doFirst / doLast action declarations as this is not supported by gradle up-to-date check - Use marker output folder for dependencies license task to make task incremental build compliant --- .../gradle/ElasticsearchJavaPlugin.java | 15 ++++++---- .../precommit/CheckstylePrecommitPlugin.java | 28 +++++++++++-------- .../precommit/DependencyLicensesTask.java | 10 +++++++ 3 files changed, 37 insertions(+), 16 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java index 400b2d8171f9f..89925d08dd8e1 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java @@ -149,7 +149,9 @@ public static void configureConfigurations(Project project) { private static final Pattern LUCENE_SNAPSHOT_REGEX = Pattern.compile("\\w+-snapshot-([a-z0-9]+)"); - /** Adds repositories used by ES dependencies */ + /** + * Adds repositories used by ES dependencies + */ public static void configureRepositories(Project project) { // ensure all repositories use secure urls // TODO: remove this with gradle 7.0, which no longer allows insecure urls @@ -216,7 +218,9 @@ private static void assertRepositoryURIIsSecure(final String repositoryName, fin } } - /** Adds compiler settings to the project */ + /** + * Adds compiler settings to the project + */ public static void configureCompile(Project project) { project.getExtensions().getExtraProperties().set("compactProfile", "full"); @@ -483,8 +487,9 @@ static void configureJars(Project project) { // we put all our distributable files under distributions jarTask.getDestinationDirectory().set(new File(project.getBuildDir(), "distributions")); // fixup the jar manifest - jarTask.doFirst( - t -> { + jarTask.doFirst(new Action() { + @Override + public void execute(Task task) { // this doFirst is added before the info plugin, therefore it will run // after the doFirst added by the info plugin, and we can override attributes jarTask.getManifest() @@ -497,7 +502,7 @@ static void configureJars(Project project) { ) ); } - ); + }); } ); project.getPluginManager().withPlugin("com.github.johnrengelman.shadow", p -> { diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/CheckstylePrecommitPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/CheckstylePrecommitPlugin.java index 2c6fd329750df..ba937c0245c99 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/CheckstylePrecommitPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/CheckstylePrecommitPlugin.java @@ -21,6 +21,7 @@ import org.elasticsearch.gradle.VersionProperties; import org.elasticsearch.gradle.util.Util; +import org.gradle.api.Action; import org.gradle.api.Project; import org.gradle.api.Task; import org.gradle.api.artifacts.dsl.DependencyHandler; @@ -62,17 +63,22 @@ public TaskProvider createTask(Project project) { copyCheckstyleConf.configure(t -> t.getInputs().files(checkstyleConfUrl.getFile(), checkstyleSuppressionsUrl.getFile())); } - copyCheckstyleConf.configure(t -> t.doLast(task -> { - checkstyleDir.mkdirs(); - try (InputStream stream = checkstyleConfUrl.openStream()) { - Files.copy(stream, checkstyleConf.toPath(), StandardCopyOption.REPLACE_EXISTING); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - try (InputStream stream = checkstyleSuppressionsUrl.openStream()) { - Files.copy(stream, checkstyleSuppressions.toPath(), StandardCopyOption.REPLACE_EXISTING); - } catch (IOException e) { - throw new UncheckedIOException(e); + // Explicitly using an Action interface as java lambdas + // are not supported by Gradle up-to-date checks + copyCheckstyleConf.configure(t -> t.doLast(new Action() { + @Override + public void execute(Task task) { + checkstyleDir.mkdirs(); + try (InputStream stream = checkstyleConfUrl.openStream()) { + Files.copy(stream, checkstyleConf.toPath(), StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + try (InputStream stream = checkstyleSuppressionsUrl.openStream()) { + Files.copy(stream, checkstyleSuppressions.toPath(), StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + throw new UncheckedIOException(e); + } } })); diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/DependencyLicensesTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/DependencyLicensesTask.java index 5c7b5e55d86e6..81c4d35cbb15e 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/DependencyLicensesTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/DependencyLicensesTask.java @@ -31,6 +31,7 @@ import org.gradle.api.tasks.InputFiles; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.Optional; +import org.gradle.api.tasks.OutputDirectory; import org.gradle.api.tasks.TaskAction; import java.io.File; @@ -223,6 +224,15 @@ public void checkDependencies() throws IOException, NoSuchAlgorithmException { if (shaFiles.isEmpty() == false) { throw new GradleException("Unused sha files found: \n" + joinFilenames(shaFiles)); } + + } + + // this is just a marker output folder to allow this task being up-to-date + // the check logic is exception driven so a failed task will not be defined + // by this output but when successful we can safely mark the task as up-to-date + @OutputDirectory + public File getOutputMarker() { + return new File(getProject().getBuildDir(), "dependencyLicense"); } private void failIfAnyMissing(String item, Boolean exists, String type) { From c4c25484a9acacfa714d5364748667fcded2d65a Mon Sep 17 00:00:00 2001 From: Rene Groeschke Date: Wed, 27 May 2020 14:43:19 +0200 Subject: [PATCH 2/2] Tweak formatting --- .../org/elasticsearch/gradle/ElasticsearchJavaPlugin.java | 2 ++ .../gradle/precommit/DependencyLicensesTask.java | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java index 89925d08dd8e1..19b04a48b53c0 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/ElasticsearchJavaPlugin.java @@ -487,6 +487,8 @@ static void configureJars(Project project) { // we put all our distributable files under distributions jarTask.getDestinationDirectory().set(new File(project.getBuildDir(), "distributions")); // fixup the jar manifest + // Explicitly using an Action interface as java lambdas + // are not supported by Gradle up-to-date checks jarTask.doFirst(new Action() { @Override public void execute(Task task) { diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/DependencyLicensesTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/DependencyLicensesTask.java index 81c4d35cbb15e..ff1d6186a42ce 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/DependencyLicensesTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/DependencyLicensesTask.java @@ -227,9 +227,9 @@ public void checkDependencies() throws IOException, NoSuchAlgorithmException { } - // this is just a marker output folder to allow this task being up-to-date - // the check logic is exception driven so a failed task will not be defined - // by this output but when successful we can safely mark the task as up-to-date + // This is just a marker output folder to allow this task being up-to-date. + // The check logic is exception driven so a failed tasks will not be defined + // by this output but when successful we can safely mark the task as up-to-date. @OutputDirectory public File getOutputMarker() { return new File(getProject().getBuildDir(), "dependencyLicense");