From 9c7c09b67f217fa6cef4dad3b3ff0534d8fedd80 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Tue, 2 Oct 2018 11:13:55 +0300 Subject: [PATCH 1/2] make sure there are no duplicate classes in third party audit closes #33759 --- .../elasticsearch/gradle/JdkJarHellCheck.java | 2 +- .../gradle/precommit/ThirdPartyAuditTask.java | 38 ++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/JdkJarHellCheck.java b/buildSrc/src/main/java/org/elasticsearch/gradle/JdkJarHellCheck.java index 60de1981f9827..7a2504efdd0fc 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/JdkJarHellCheck.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/JdkJarHellCheck.java @@ -43,7 +43,7 @@ private void scanForJDKJarHell(Path root) throws IOException { @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { String entry = root.relativize(file).toString().replace('\\', '/'); - if (entry.endsWith(".class")) { + if (entry.endsWith(".class") && entry.endsWith("module-info.class") == false) { if (ext.getResource(entry) != null) { detected.add( entry diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java index 7e4766ada6541..78ea62c926697 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java @@ -20,12 +20,12 @@ import org.apache.commons.io.output.NullOutputStream; import org.elasticsearch.gradle.JdkJarHellCheck; -import org.elasticsearch.test.NamingConventionsCheck; import org.gradle.api.DefaultTask; import org.gradle.api.GradleException; import org.gradle.api.JavaVersion; import org.gradle.api.artifacts.Configuration; import org.gradle.api.file.FileCollection; +import org.gradle.api.file.FileTree; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.InputFile; import org.gradle.api.tasks.InputFiles; @@ -47,6 +47,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import java.util.stream.IntStream; public class ThirdPartyAuditTask extends DefaultTask { @@ -171,19 +172,30 @@ private void extractJars(FileCollection jars) { File jarExpandDir = getJarExpandDir(); // We need to clean up to make sure old dependencies don't linger getProject().delete(jarExpandDir); - jars.forEach(jar -> + + jars.forEach(jar -> { + FileTree jarFiles = getProject().zipTree(jar); getProject().copy(spec -> { + spec.from(jarFiles); + spec.into(jarExpandDir); + // exclude classes from multi release jars + spec.exclude("META-INF/versions/**"); + }); + // The order is important, we iterate here so we don't depend on the order in which Gradle executes the spec + IntStream.rangeClosed( + Integer.parseInt(JavaVersion.VERSION_1_9.getMajorVersion()), + Integer.parseInt(targetCompatibility.getMajorVersion()) + ).forEach(majorVersion -> getProject().copy(spec -> { spec.from(getProject().zipTree(jar)); spec.into(jarExpandDir); - // Exclude classes for multi release jars above target - for (int i = Integer.parseInt(targetCompatibility.getMajorVersion()) + 1; - i <= Integer.parseInt(JavaVersion.VERSION_HIGHER.getMajorVersion()); - i++ - ) { - spec.exclude("META-INF/versions/" + i + "/**"); - } - }) - ); + // exclude classes from multi release jars + String metaInfPrefix = "META-INF/versions/" + majorVersion; + spec.include(metaInfPrefix + "/**"); + // Drop the version specific prefix + spec.eachFile(details -> details.setPath(details.getPath().replace(metaInfPrefix, ""))); + spec.setIncludeEmptyDirs(false); + })); + }); } private void assertNoJarHell(Set jdkJarHellClasses) { @@ -276,9 +288,9 @@ private String formatClassList(Set classList) { private Set runJdkJarHellCheck() throws IOException { ByteArrayOutputStream standardOut = new ByteArrayOutputStream(); ExecResult execResult = getProject().javaexec(spec -> { - URL location = NamingConventionsCheck.class.getProtectionDomain().getCodeSource().getLocation(); + URL location = JdkJarHellCheck.class.getProtectionDomain().getCodeSource().getLocation(); if (location.getProtocol().equals("file") == false) { - throw new GradleException("Unexpected location for NamingConventionCheck class: " + location); + throw new GradleException("Unexpected location for JdkJarHellCheck class: " + location); } try { spec.classpath( From a109e79cfdb051fb5d79ab048569fd7c15ab9716 Mon Sep 17 00:00:00 2001 From: Alpar Torok Date: Thu, 4 Oct 2018 09:19:43 +0300 Subject: [PATCH 2/2] Explain the jar extraction better --- .../gradle/precommit/ThirdPartyAuditTask.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java index 78ea62c926697..bffa011cb7be2 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/precommit/ThirdPartyAuditTask.java @@ -181,14 +181,22 @@ private void extractJars(FileCollection jars) { // exclude classes from multi release jars spec.exclude("META-INF/versions/**"); }); + // Deal with multi release jars: // The order is important, we iterate here so we don't depend on the order in which Gradle executes the spec + // We extract multi release jar classes ( if these exist ) going from 9 - the first to support them, to the + // current `targetCompatibility` version. + // Each extract will overwrite the top level classes that existed before it, the result is that we end up + // with a single version of the class in `jarExpandDir`. + // This will be the closes version to `targetCompatibility`, the same class that would be loaded in a JVM + // that has `targetCompatibility` version. + // This means we only scan classes that would be loaded into `targetCompatibility`, and don't look at any + // pther version specific implementation of said classes. IntStream.rangeClosed( Integer.parseInt(JavaVersion.VERSION_1_9.getMajorVersion()), Integer.parseInt(targetCompatibility.getMajorVersion()) ).forEach(majorVersion -> getProject().copy(spec -> { spec.from(getProject().zipTree(jar)); spec.into(jarExpandDir); - // exclude classes from multi release jars String metaInfPrefix = "META-INF/versions/" + majorVersion; spec.include(metaInfPrefix + "/**"); // Drop the version specific prefix