Browse Source

Provide default jarHell configuration dependencies (#90810)

With the new StableBuildPlugin we do not add default dependencies to the plugin classpath.
That exposed an issue with the JarHellPlugin not really taking care of configuring the
jarHell configuration.
The JarHell Plugin only worked so far as the plugin build plugin has added a transitive dependency
to elasticsearch-core and therefore kind of only worked by accident-.

This adds a default configuration of the jarHell configuration that can be overridden manually.
Furthermore we clear some inter plugin dependencies explicit and add proper functional
test coverage
Rene Groeschke 3 years ago
parent
commit
fe93b8161b

+ 2 - 1
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/BaseInternalPluginBuildPlugin.java

@@ -12,6 +12,7 @@ import groovy.lang.Closure;
 
 import org.elasticsearch.gradle.internal.conventions.util.Util;
 import org.elasticsearch.gradle.internal.info.BuildParams;
+import org.elasticsearch.gradle.internal.precommit.JarHellPrecommitPlugin;
 import org.elasticsearch.gradle.plugin.PluginBuildPlugin;
 import org.elasticsearch.gradle.plugin.PluginPropertiesExtension;
 import org.elasticsearch.gradle.testclusters.ElasticsearchCluster;
@@ -33,13 +34,13 @@ public class BaseInternalPluginBuildPlugin implements Plugin<Project> {
     @Override
     public void apply(Project project) {
         project.getPluginManager().apply(PluginBuildPlugin.class);
+        project.getPluginManager().apply(JarHellPrecommitPlugin.class);
         project.getPluginManager().apply(ElasticsearchJavaPlugin.class);
         // Clear default dependencies added by public PluginBuildPlugin as we add our
         // own project dependencies for internal builds
         // TODO remove once we removed default dependencies from PluginBuildPlugin
         project.getConfigurations().getByName("compileOnly").getDependencies().clear();
         project.getConfigurations().getByName("testImplementation").getDependencies().clear();
-
         var extension = project.getExtensions().getByType(PluginPropertiesExtension.class);
 
         // We've ported this from multiple build scripts where we see this pattern into

+ 95 - 0
build-tools/src/integTest/groovy/org/elasticsearch/gradle/jarhell/JarHellPluginFuncTest.groovy

@@ -0,0 +1,95 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.gradle.jarhell
+
+import org.elasticsearch.gradle.VersionProperties
+import org.elasticsearch.gradle.fixtures.AbstractJavaGradleFuncTest
+import org.gradle.testkit.runner.TaskOutcome
+
+import static org.elasticsearch.gradle.fixtures.TestClasspathUtils.setupJarHellJar
+
+class JarHellPluginFuncTest extends AbstractJavaGradleFuncTest {
+
+    def setup() {
+        buildFile << """
+            plugins {
+                // bring the build tools on the test classpath
+                id 'elasticsearch.esplugin' apply false
+            }
+            
+            repositories {
+              maven {
+                name = "local-test"
+                url = file("local-repo")
+                metadataSources {
+                  artifact()
+                }
+              }
+            }
+        """
+
+        def version = VersionProperties.elasticsearch
+        setupJarHellJar(dir("local-repo/org/elasticsearch/elasticsearch-core/${version}/"), version)
+    }
+
+    def "skips jar hell task when no java plugin applied"(){
+        given:
+        buildFile << """
+            apply plugin: org.elasticsearch.gradle.jarhell.JarHellPlugin
+        """
+
+        when:
+        def result = gradleRunner("jarHell").build()
+        then:
+        result.task(":jarHell").outcome == TaskOutcome.NO_SOURCE
+    }
+
+    def "run jar hell when java plugin is applied"(){
+        given:
+        buildFile << """
+            apply plugin: org.elasticsearch.gradle.jarhell.JarHellPlugin
+            apply plugin: 'java'
+        
+        """
+
+        when:
+        def result = gradleRunner("jarHell").build()
+        then:
+        result.task(":jarHell").outcome == TaskOutcome.NO_SOURCE
+
+        when:
+        clazz("org.acme.SomeClass")
+        result = gradleRunner("jarHell").build()
+
+        then:
+        result.task(":jarHell").outcome == TaskOutcome.SUCCESS
+    }
+
+    def "jar hell configuration defaults can be overridden"(){
+        given:
+        def version = "1.0.0"
+        clazz("org.acme.SomeClass")
+
+        buildFile << """
+            apply plugin: org.elasticsearch.gradle.jarhell.JarHellPlugin
+            apply plugin: 'java'
+            
+            dependencies {
+                jarHell "org.elasticsearch:elasticsearch-core:$version"
+            }
+        """
+
+        setupJarHellJar(dir("local-repo/org/elasticsearch/elasticsearch-core/${version}/"), version)
+
+        when:
+        def result = gradleRunner("jarHell").build()
+        then:
+        result.task(":jarHell").outcome == TaskOutcome.SUCCESS
+    }
+}

+ 18 - 14
build-tools/src/main/java/org/elasticsearch/gradle/jarhell/JarHellPlugin.java

@@ -8,11 +8,13 @@
 
 package org.elasticsearch.gradle.jarhell;
 
+import org.elasticsearch.gradle.VersionProperties;
 import org.elasticsearch.gradle.util.GradleUtils;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
 import org.gradle.api.Task;
 import org.gradle.api.artifacts.Configuration;
+import org.gradle.api.artifacts.dsl.DependencyHandler;
 import org.gradle.api.tasks.SourceSet;
 import org.gradle.api.tasks.TaskProvider;
 import org.gradle.language.base.plugins.LifecycleBasePlugin;
@@ -21,7 +23,14 @@ public class JarHellPlugin implements Plugin<Project> {
 
     @Override
     public void apply(Project project) {
-        TaskProvider<? extends Task> jarHellTask = createTask(project);
+        Configuration jarHellConfig = project.getConfigurations().create("jarHell");
+        DependencyHandler dependencyHandler = project.getDependencies();
+        jarHellConfig.defaultDependencies(
+            deps -> deps.add(dependencyHandler.create("org.elasticsearch:elasticsearch-core:" + VersionProperties.getElasticsearch()))
+        );
+
+        TaskProvider<? extends Task> jarHellTask = createTask(jarHellConfig, project);
+
         project.getPluginManager()
             .withPlugin(
                 "lifecycle-base",
@@ -29,23 +38,18 @@ public class JarHellPlugin implements Plugin<Project> {
             );
     }
 
-    private TaskProvider<? extends Task> createTask(Project project) {
-        Configuration jarHellConfig = project.getConfigurations().create("jarHell");
+    private TaskProvider<? extends Task> createTask(Configuration jarHellConfig, Project project) {
         TaskProvider<JarHellTask> jarHell = project.getTasks().register("jarHell", JarHellTask.class);
-        jarHell.configure(t -> {
-            SourceSet testSourceSet = getJavaTestSourceSet(project);
-            t.setClasspath(testSourceSet.getRuntimeClasspath());
-            t.setJarHellRuntimeClasspath(jarHellConfig);
+
+        project.getPluginManager().withPlugin("java", p -> {
+            jarHell.configure(t -> {
+                SourceSet testSourceSet = GradleUtils.getJavaSourceSets(project).findByName(SourceSet.TEST_SOURCE_SET_NAME);
+                t.setClasspath(testSourceSet.getRuntimeClasspath());
+            });
         });
+        jarHell.configure(t -> { t.setJarHellRuntimeClasspath(jarHellConfig); });
 
         return jarHell;
     }
 
-    /**
-     * @param project The project to look for test Java resources.
-     */
-    private static SourceSet getJavaTestSourceSet(Project project) {
-        return GradleUtils.getJavaSourceSets(project).findByName(SourceSet.TEST_SOURCE_SET_NAME);
-    }
-
 }

+ 3 - 1
build-tools/src/main/java/org/elasticsearch/gradle/jarhell/JarHellTask.java

@@ -16,6 +16,7 @@ import org.gradle.api.tasks.CacheableTask;
 import org.gradle.api.tasks.Classpath;
 import org.gradle.api.tasks.CompileClasspath;
 import org.gradle.api.tasks.OutputFile;
+import org.gradle.api.tasks.SkipWhenEmpty;
 import org.gradle.api.tasks.TaskAction;
 import org.gradle.process.ExecOperations;
 
@@ -66,8 +67,9 @@ public class JarHellTask extends DefaultTask {
     // We use compile classpath normalization here because class implementation changes are irrelevant for the purposes of jar hell.
     // We only care about the runtime classpath ABI here.
     @CompileClasspath
+    @SkipWhenEmpty
     public FileCollection getClasspath() {
-        return classpath.filter(File::exists);
+        return classpath == null ? null : classpath.filter(File::exists);
     }
 
     public void setClasspath(FileCollection classpath) {

+ 14 - 4
build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/fixtures/TestClasspathUtils.groovy → build-tools/src/testFixtures/groovy/org/elasticsearch/gradle/fixtures/TestClasspathUtils.groovy

@@ -22,7 +22,11 @@ import static org.junit.Assert.fail
 class TestClasspathUtils {
 
     static void setupJarHellJar(File projectRoot) {
-        generateJdkJarHellCheck(projectRoot, "org.elasticsearch.jdk.JarHell", FixedValue.value(TypeDescription.VOID))
+        generateJdkJarHellCheck(projectRoot, "org.elasticsearch.jdk.JarHell", "current", FixedValue.value(TypeDescription.VOID))
+    }
+
+    static void setupJarHellJar(File projectRoot, String version) {
+        generateJdkJarHellCheck(projectRoot, "org.elasticsearch.jdk.JarHell", version, FixedValue.value(TypeDescription.VOID))
     }
 
     static void setupJarJdkClasspath(File projectRoot) {
@@ -35,6 +39,10 @@ class TestClasspathUtils {
     }
 
     private static void generateJdkJarHellCheck(File targetDir, String className, Implementation mainImplementation) {
+        generateJdkJarHellCheck(targetDir, className, "current", mainImplementation)
+    }
+
+        private static void generateJdkJarHellCheck(File targetDir, String className, String version, Implementation mainImplementation) {
         DynamicType.Unloaded<?> dynamicType = new ByteBuddy().subclass(Object.class)
             .name(className)
             .defineMethod("main", void.class, Visibility.PUBLIC, Ownership.STATIC)
@@ -42,15 +50,17 @@ class TestClasspathUtils {
             .intercept(mainImplementation)
             .make()
         try {
-            dynamicType.toJar(targetFile(targetDir))
+            dynamicType.toJar(targetFile(targetDir, version))
         } catch (IOException e) {
             e.printStackTrace()
             fail("Cannot setup jdk jar hell classpath")
         }
     }
 
-    private static File targetFile(File projectRoot) {
-        File targetFile = new File(projectRoot, "elasticsearch-core-current.jar")
+    private static File targetFile(File projectRoot, String version) {
+        File targetFile = new File(projectRoot, "elasticsearch-core-${version}.jar")
+
+        println "targetFile = $targetFile"
         targetFile.getParentFile().mkdirs()
         return targetFile
     }

+ 4 - 0
libs/core/build.gradle

@@ -41,3 +41,7 @@ tasks.named("thirdPartyAudit").configure {
           'org/osgi/framework/wiring/BundleWiring'
   )
 }
+
+dependencies {
+  jarHell sourceSets.main.output
+}