chore: Rework additional classpath and instrumenter classpath contributions to instrument plugin

Author: bric3

buildSrc/src/main/groovy/InstrumentPlugin.groovy

@@ -2,15 +2,13 @@ import org.gradle.api.Action
 import org.gradle.api.Plugin
 import org.gradle.api.Project
 import org.gradle.api.file.ConfigurableFileCollection
-import org.gradle.api.file.Directory
 import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.file.FileCollection
 import org.gradle.api.invocation.BuildInvocationDetails
 import org.gradle.api.logging.Logger
 import org.gradle.api.logging.Logging
 import org.gradle.api.provider.ListProperty
 import org.gradle.api.provider.Property
-import org.gradle.api.provider.Provider
 import org.gradle.api.tasks.SourceSet
 import org.gradle.api.tasks.SourceSetContainer
 import org.gradle.api.tasks.compile.AbstractCompile
@@ -102,20 +100,21 @@ class InstrumentPlugin implements Plugin<Project> {
           javaVersion = javaVersion ?: DEFAULT_JAVA_VERSION // null not accepted
           it.inputs.property("javaVersion", javaVersion)
 
-          def plugins = extension.plugins
-          it.inputs.property("plugins", plugins)
+          it.inputs.property("plugins", extension.plugins)
 
-          def compileTaskClasspath = it.classpath
-          def rawClassesDir = it.destinationDirectory
+          it.inputs.files(extension.additionalClasspath)
 
-          def additionalClassPath = findAdditionalClassPath(extension)
-          it.inputs.files(additionalClassPath)
+          // Temporary location for raw (un-instrumented) classes
+          DirectoryProperty tmpUninstrumentedClasses = project.objects.directoryProperty().value(
+            project.layout.buildDirectory.dir("tmp/${it.name}-raw-classes")
+          )
 
-          def instrumentingClassPath = project.objects.fileCollection()
+          // Class path to use for instrumentation post-processing
+          ConfigurableFileCollection instrumentingClassPath = project.objects.fileCollection()
           instrumentingClassPath.setFrom(
-            compileTaskClasspath,
-            rawClassesDir,
-            additionalClassPath,
+            it.classpath,
+            extension.additionalClasspath,
+            tmpUninstrumentedClasses
           )
 
           // This were the post processing happens, i.e. were the instrumentation is applied
@@ -124,30 +123,22 @@ class InstrumentPlugin implements Plugin<Project> {
             project.objects.newInstance(
               InstrumentPostProcessingAction,
               javaVersion,
-              plugins,
+              extension.plugins,
               instrumentingClassPath,
-              rawClassesDir,
+              it.destinationDirectory,
+              tmpUninstrumentedClasses
             )
           )
           logger.info("[InstrumentPlugin] Configured post-compile instrumentation for $compileTaskName for source-set $sourceSetName")
         }
       }
     }
   }
-
-  private static List<Provider<Directory>> findAdditionalClassPath(InstrumentExtension extension) {
-    return extension.additionalClasspath.getOrDefault('instrumentJava', []).collect { dirProperty ->
-      dirProperty.map {
-        def fileName = it.asFile.name
-        it.dir("../${fileName.replaceFirst('^main', 'raw')}") // TODO there's a hidden contract here
-      }
-    }
-  }
 }
 
 abstract class InstrumentExtension {
   abstract ListProperty<String> getPlugins()
-  Map<String /* taskName */, List<DirectoryProperty>> additionalClasspath = [:]
+  abstract ListProperty<DirectoryProperty> getAdditionalClasspath()
 }
 
 abstract class InstrumentPostProcessingAction implements Action<AbstractCompile> {
@@ -165,22 +156,26 @@ abstract class InstrumentPostProcessingAction implements Action<AbstractCompile>
   @Inject
   abstract WorkerExecutor getWorkerExecutor()
 
+  // Those cannot be private other wise Groovy will fail at runtime with a missing property ex
   final String javaVersion
   final ListProperty<String> plugins
   final FileCollection instrumentingClassPath
-  final DirectoryProperty rawClassesDirectory
+  final DirectoryProperty compilerOutputDirectory
+  final DirectoryProperty tmpDirectory
 
   @Inject
   InstrumentPostProcessingAction(
     String javaVersion,
     ListProperty<String> plugins,
     FileCollection instrumentingClassPath,
-    DirectoryProperty rawClassesDir
+    DirectoryProperty compilerOutputDirectory,
+    DirectoryProperty tmpDirectory
   ) {
     this.javaVersion = javaVersion
     this.plugins = plugins
     this.instrumentingClassPath = instrumentingClassPath
-    this.rawClassesDirectory = rawClassesDir
+    this.compilerOutputDirectory = compilerOutputDirectory
+    this.tmpDirectory = tmpDirectory
   }
 
   @Override
@@ -190,7 +185,7 @@ abstract class InstrumentPostProcessingAction implements Action<AbstractCompile>
         "  javaVersion=${javaVersion}, \n" +
         "  plugins=${plugins.get()}, \n" +
         "  instrumentingClassPath=${instrumentingClassPath.files}, \n" +
-        "  rawClassesDirectory=${rawClassesDirectory.get().asFile}"
+        "  rawClassesDirectory=${compilerOutputDirectory.get().asFile}"
     )
     def postCompileAction = this
     workQueue().submit(InstrumentAction.class, parameters -> {
@@ -201,8 +196,8 @@ abstract class InstrumentPostProcessingAction implements Action<AbstractCompile>
       )
       parameters.plugins.set(postCompileAction.plugins)
       parameters.instrumentingClassPath.setFrom(postCompileAction.instrumentingClassPath)
-      parameters.classesDirectory.set(postCompileAction.rawClassesDirectory)
-      parameters.tmpDirectory.set(project.layout.buildDirectory.dir("tmp/${task.name}-raw-classes"))
+      parameters.compilerOutputDirectory.set(postCompileAction.compilerOutputDirectory)
+      parameters.tmpDirectory.set(postCompileAction.tmpDirectory)
     })
   }
 
@@ -227,7 +222,7 @@ interface InstrumentWorkParameters extends WorkParameters {
   ConfigurableFileCollection getPluginClassPath()
   ListProperty<String> getPlugins()
   ConfigurableFileCollection getInstrumentingClassPath()
-  DirectoryProperty getClassesDirectory()
+  DirectoryProperty getCompilerOutputDirectory()
   DirectoryProperty getTmpDirectory()
 }
 
@@ -254,29 +249,29 @@ abstract class InstrumentAction implements WorkAction<InstrumentWorkParameters>
         }
       }
     }
-    Path classesDirectory = parameters.classesDirectory.get().asFile.toPath()
-    Path tmpSourceDir = parameters.tmpDirectory.get().asFile.toPath()
+    Path classesDirectory = parameters.compilerOutputDirectory.get().asFile.toPath()
+    Path tmpUninstrumentedDir = parameters.tmpDirectory.get().asFile.toPath()
 
-    // Delete tmpSourceDir contents recursively
-    if (Files.exists(tmpSourceDir)) {
-      Files.walk(tmpSourceDir)
+    // Delete previous tmpSourceDir contents recursively
+    if (Files.exists(tmpUninstrumentedDir)) {
+      Files.walk(tmpUninstrumentedDir)
         .sorted(Comparator.reverseOrder())
         .forEach { p ->
-          if (!p.equals(tmpSourceDir)) {
+          if (!p.equals(tmpUninstrumentedDir)) {
             Files.deleteIfExists(p)
           }
         }
     }
 
     Files.move(
       classesDirectory,
-      tmpSourceDir,
+      tmpUninstrumentedDir,
       StandardCopyOption.REPLACE_EXISTING,
       StandardCopyOption.ATOMIC_MOVE,
     )
 
     ClassLoader instrumentingCL = createClassLoader(parameters.instrumentingClassPath, pluginCL)
-    InstrumentingPlugin.instrumentClasses(plugins, instrumentingCL, tmpSourceDir.toFile(), classesDirectory.toFile())
+    InstrumentingPlugin.instrumentClasses(plugins, instrumentingCL, tmpUninstrumentedDir.toFile(), classesDirectory.toFile())
   }
 
   static ClassLoader createClassLoader(cp, parent = InstrumentAction.classLoader) {

dd-java-agent/instrumentation/jetty-9/build.gradle

@@ -85,26 +85,23 @@ jar {
   from sourceSets.main_jetty10.output
 }
 
-List<DirectoryProperty> extraInstrumentJavaDirs = []
-['main_jetty904', 'main_jetty93', 'main_jetty9421', 'main_jetty10'].each {
-  JavaCompile compileTask = tasks["compile${it.capitalize()}Java"]
-  extraInstrumentJavaDirs << compileTask.destinationDirectory
-  compileTask.dependsOn tasks['compileJava']
-  project.afterEvaluate { p ->
-    tasks["forbiddenApis${it.capitalize()}"].dependsOn("instrument${it.capitalize()}Java")
+['main_jetty904', 'main_jetty93', 'main_jetty9421', 'main_jetty10'].each { String sourceSet ->
+  tasks.named("compile${sourceSet.capitalize()}Java", JavaCompile) {
+    it.dependsOn tasks.named('compileJava')
+    // the instrumenters are in main, but helpers/advice in possibly other sourceSets
+    // The muzzle generator of references run as part of InstrumentJava needs access to
+    // these extra classes. The task dependencies for instrumentJava are added above
+    instrument.additionalClasspath.add(it.destinationDirectory)
   }
-}
 
-instrument {
-  // the instrumenters are in main, but helpers/advice in possibly other sourceSets
-  // The muzzle generator of references run as part of InstrumentJava needs access to
-  // these extra classes. The task dependencies for instrumentJava are added above
-  additionalClasspath = [
-    instrumentJava: extraInstrumentJavaDirs
-  ]
+  project.afterEvaluate {
+    tasks.named("forbiddenApis${sourceSet.capitalize()}") {
+      it.dependsOn("compile${sourceSet.capitalize()}Java")
+    }
+  }
 }
 
-tasks['compileMain_jetty10Java'].configure {
+tasks.named('compileMain_jetty10Java').configure {
   setJavaVersion(it, 11)
 }
 

dd-java-agent/instrumentation/play-2.4/build.gradle

@@ -46,7 +46,7 @@ repositories {
   }
 }
 
-tasks.withType(org.gradle.api.tasks.scala.ScalaCompile) {
+tasks.withType(ScalaCompile).configureEach {
   it.javaLauncher = getJavaLauncherFor(8)
 }
 
@@ -60,14 +60,17 @@ sourceSets {
 jar {
   from sourceSets.main_play25.output
 }
-project.afterEvaluate { p ->
-  //  instrumentJava.dependsOn compileMain_play25Java  // TODO remove
-  forbiddenApisMain_play25.dependsOn compileMain_play25Java
+
+project.afterEvaluate {
+  tasks.named('forbiddenApisMain_play25') {
+    dependsOn(tasks.named('compileMain_play25Java'))
+  }
 }
+
 instrument {
-  additionalClasspath = [
-    instrumentJava: compileMain_play25Java.destinationDirectory
-  ]
+  additionalClasspath.add(
+    tasks.named('compileMain_play25Java', AbstractCompile).map { it.destinationDirectory }
+  )
 }
 
 dependencies {

dd-java-agent/instrumentation/play-2.6/build.gradle

@@ -75,15 +75,19 @@ sourceSets {
 jar {
   from sourceSets.main_play27.output
 }
-compileMain_play27Java.dependsOn compileJava
-project.afterEvaluate { p ->
-  //  instrumentJava.dependsOn compileMain_play27Java
-  forbiddenApisMain_play27.dependsOn compileMain_play27Java
+tasks.named('compileMain_play27Java') {
+  dependsOn(tasks.named('compileJava'))
 }
+project.afterEvaluate {
+  tasks.named('forbiddenApisMain_play27') {
+    dependsOn(tasks.named('compileMain_play27Java'))
+  }
+}
+
 instrument {
-  additionalClasspath = [
-    instrumentJava: compileMain_play27Java.destinationDirectory
-  ]
+  additionalClasspath.add(
+    tasks.named('compileMain_play27Java', AbstractCompile).map { it.destinationDirectory }
+  )
 }
 
 dependencies {