diff --git a/CHANGELOG.md b/CHANGELOG.md index bf8a5bd..e378237 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # AtPlug releases ## [Unreleased] +### Added +- Cacheability, incremental task, and huge speed improvement. ([#66](https://github.com/diffplug/atplug/pull/66)) + - `plugGenerate` refactored into `plugFind` followed by `plugGenerate` ### Changed - Bump required JVM from 11 to 17. ([#63](https://github.com/diffplug/atplug/pull/63)) - Detect Kotlin version rather than harcode it. ([#64](https://github.com/diffplug/atplug/pull/64)) diff --git a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGenerator.kt b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGenerator.kt index 621f153..e576e38 100644 --- a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGenerator.kt +++ b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGenerator.kt @@ -22,18 +22,18 @@ import java.lang.UnsatisfiedLinkError import java.lang.reflect.Constructor import java.lang.reflect.Modifier import java.net.URLClassLoader -import java.nio.file.FileVisitResult -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.SimpleFileVisitor -import java.nio.file.attribute.BasicFileAttributes import java.util.* import java.util.function.Function import kotlin.reflect.KFunction import kotlin.reflect.full.companionObjectInstance import kotlin.reflect.full.memberFunctions -class PlugGenerator internal constructor(toSearches: List, toLinkAgainst: Set) { +class PlugGenerator +internal constructor( + plugToSocket: Map, + toSearches: List, + toLinkAgainst: Set +) { @JvmField val atplugInf: SortedMap = TreeMap() /** A cache from a plugin interface to a function that converts a class into its metadata. */ @@ -56,22 +56,7 @@ class PlugGenerator internal constructor(toSearches: List, toLinkAgainst: }!! as KFunction> try { - val parser = PlugParser() - // walk toSearch, passing each classfile to load() - for (toSearch in toSearches) { - if (toSearch.isDirectory) { - Files.walkFileTree( - toSearch.toPath(), - object : SimpleFileVisitor() { - override fun visitFile(file: Path, attrs: BasicFileAttributes): FileVisitResult { - if (file.toString().endsWith(EXT_CLASS)) { - maybeGeneratePlugin(parser, file) - } - return FileVisitResult.CONTINUE - } - }) - } - } + plugToSocket.forEach { (plug, socket) -> atplugInf[plug] = generatePlugin(plug, socket) } } finally { classLoader.close() System.setProperty("atplug.generate", "") @@ -82,18 +67,13 @@ class PlugGenerator internal constructor(toSearches: List, toLinkAgainst: * Loads a class by its FQN. If it's concrete and implements a plugin, then we'll call * generatePlugin. */ - private fun maybeGeneratePlugin(parser: PlugParser, path: Path) { - parser.parse(path.toFile()) - if (!parser.hasPlug()) { - return - } - val plugClass = classLoader.loadClass(parser.plugClassName) - val socketClass = classLoader.loadClass(parser.socketClassName) + private fun generatePlugin(plugClassName: String, socketClassName: String): String { + val plugClass = classLoader.loadClass(plugClassName) + val socketClass = classLoader.loadClass(socketClassName) require(!Modifier.isAbstract(plugClass.modifiers)) { "Class $plugClass has @Plug($socketClass) but it is abstract." } - val atplugInfContent = generatePlugin(plugClass, socketClass) - atplugInf[plugClass.name] = atplugInfContent + return generatePlugin(plugClass, socketClass) } private fun generatePlugin( @@ -128,14 +108,18 @@ class PlugGenerator internal constructor(toSearches: List, toLinkAgainst: * Returns a Map from a plugin's name to its ATPLUG-INF content. * * @param toSearch a directory containing class files where we will look for plugin - * implementations + * implementations * @param toLinkAgainst the classes that these plugins implementations need * @return a map from component name to is ATPLUG-INF string content */ - fun generate(toSearch: List, toLinkAgainst: Set): SortedMap { + fun generate( + plugToSocket: Map, + toSearch: List, + toLinkAgainst: Set + ): SortedMap { return try { - val ext = PlugGeneratorJavaExecable(toSearch, toLinkAgainst) - val metadataGen = PlugGenerator(ext.toSearch, ext.toLinkAgainst) + val ext = PlugGeneratorJavaExecable(plugToSocket, toSearch, toLinkAgainst) + val metadataGen = PlugGenerator(plugToSocket, ext.toSearch, ext.toLinkAgainst) // save our results, with no reference to the guts of what happened inside PluginMetadataGen metadataGen.atplugInf } catch (e: Exception) { diff --git a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGeneratorJavaExecable.kt b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGeneratorJavaExecable.kt index a380f68..22ed5d3 100644 --- a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGeneratorJavaExecable.kt +++ b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugGeneratorJavaExecable.kt @@ -20,21 +20,21 @@ import java.io.File import java.util.* /** [PlugGenerator.PlugGenerator] in a [JavaExecable] form. */ -class PlugGeneratorJavaExecable(toSearch: List?, toLinkAgainst: Set?) : JavaExecable { +class PlugGeneratorJavaExecable( + plugToSocket: Map, + toSearch: List, + toLinkAgainst: Set +) : JavaExecable { // inputs - var toSearch: List - var toLinkAgainst: Set + var plugToSocket: Map = LinkedHashMap(plugToSocket) + var toSearch: List = ArrayList(toSearch) + var toLinkAgainst: Set = LinkedHashSet(toLinkAgainst) // outputs @JvmField var atplugInf: SortedMap? = null - init { - this.toSearch = ArrayList(toSearch) - this.toLinkAgainst = LinkedHashSet(toLinkAgainst) - } - override fun run() { - val metadataGen = PlugGenerator(toSearch, toLinkAgainst) + val metadataGen = PlugGenerator(plugToSocket, toSearch, toLinkAgainst) atplugInf = metadataGen.atplugInf } } diff --git a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugParser.kt b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugParser.kt index 2716238..6808537 100644 --- a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugParser.kt +++ b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/PlugParser.kt @@ -26,7 +26,7 @@ import org.objectweb.asm.Type class PlugParser { private var buffer = ByteArray(64 * 1024) - fun parse(file: File) { + fun parse(file: File): Pair? { val filelen = (file.length() + 1).toInt() // +1 prevents infinite loop if (buffer.size < filelen) { buffer = ByteArray(filelen) @@ -42,18 +42,17 @@ class PlugParser { } } val reader = ClassReader(buffer, 0, pos) - plugClassName = asmToJava(reader.className) + val plugClassName = asmToJava(reader.className) + this.plugClassName = plugClassName socketClassName = null + // set socketClassName if there is an `@Plug` reader.accept( classVisitor, ClassReader.SKIP_FRAMES or ClassReader.SKIP_DEBUG or ClassReader.SKIP_CODE) + return socketClassName?.let { plugClassName to it } } - fun hasPlug(): Boolean { - return socketClassName != null - } - - var plugClassName: String? = null - var socketClassName: String? = null + private var plugClassName: String? = null + private var socketClassName: String? = null private val classVisitor: ClassVisitor = object : ClassVisitor(API) { @@ -73,7 +72,7 @@ class PlugParser { } companion object { - fun asmToJava(className: String): String { + private fun asmToJava(className: String): String { return className.replace("/", ".") } diff --git a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/FindPlugsTask.kt b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/FindPlugsTask.kt new file mode 100644 index 0000000..3be9eda --- /dev/null +++ b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/FindPlugsTask.kt @@ -0,0 +1,82 @@ +package com.diffplug.atplug.tooling.gradle + +import com.diffplug.atplug.tooling.PlugParser +import java.io.File +import org.gradle.api.DefaultTask +import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.DirectoryProperty +import org.gradle.api.tasks.* +import org.gradle.work.* + +/** + * Incrementally scans compiled classes for @Plug usage and writes discovered plug classes into an + * output directory. + */ +@CacheableTask +abstract class FindPlugsTask : DefaultTask() { + @get:CompileClasspath + @get:Incremental + @get:InputFiles + abstract val classesFolders: ConfigurableFileCollection + + /** Directory where we will store discovered plugs. */ + @get:OutputDirectory abstract val discoveredPlugsDir: DirectoryProperty + + @TaskAction + fun findPlugs(inputChanges: InputChanges) { + // If not incremental, clear everything and rescan + if (!inputChanges.isIncremental) { + discoveredPlugsDir.get().asFile.deleteRecursively() + } + + // Make sure our output directory exists + discoveredPlugsDir.get().asFile.mkdirs() + + // For each changed file in classesFolders, determine if it has @Plug + val parser = PlugParser() + for (change in inputChanges.getFileChanges(classesFolders)) { + if (!change.file.name.endsWith(".class")) { + continue + } + when (change.changeType) { + ChangeType.REMOVED -> { + // Remove old discovered data for this file + removeOldMetadata(change.file) + } + ChangeType.ADDED, + ChangeType.MODIFIED -> { + parseAndWriteMetadata(parser, change.file) + } + } + } + } + + private fun parseAndWriteMetadata(parser: PlugParser, classFile: File) { + val plugToSocket = parser.parse(classFile) + if (plugToSocket != null) { + // For example: write a single line containing the discovered plug FQN + val discoveredFile = discoveredPlugsDir.file(classFile.nameWithoutExtension).get().asFile + if (discoveredFile.exists()) { + val existing = discoveredFile.readText().split("|") + check(existing[0] == plugToSocket.first) { + "You need to rename one of these plugs because they have the same classfile name: ${existing[0]} and $plugToSocket" + } + } else { + discoveredFile.parentFile.mkdirs() + } + discoveredFile.writeText(plugToSocket.let { "${it.first}|${it.second}" }) + } else { + // If previously discovered, remove it + removeOldMetadata(classFile) + } + } + + private fun removeOldMetadata(classFile: File) { + // Remove any discovered file for the old .class + val possibleName = classFile.nameWithoutExtension + val discoveredFile = discoveredPlugsDir.file(possibleName).get().asFile + if (discoveredFile.exists()) { + discoveredFile.delete() + } + } +} diff --git a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugGenerateTask.kt b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugGenerateTask.kt index 2990f22..63e29f7 100644 --- a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugGenerateTask.kt +++ b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugGenerateTask.kt @@ -30,20 +30,15 @@ import java.util.jar.Manifest import javax.inject.Inject import org.gradle.api.DefaultTask import org.gradle.api.file.ConfigurableFileCollection +import org.gradle.api.file.DirectoryProperty import org.gradle.api.file.FileCollection import org.gradle.api.plugins.JavaPluginExtension import org.gradle.api.provider.Property -import org.gradle.api.tasks.Classpath -import org.gradle.api.tasks.InputFiles -import org.gradle.api.tasks.Internal -import org.gradle.api.tasks.JavaExec -import org.gradle.api.tasks.Nested -import org.gradle.api.tasks.Optional -import org.gradle.api.tasks.OutputDirectory -import org.gradle.api.tasks.TaskAction +import org.gradle.api.tasks.* import org.gradle.jvm.toolchain.JavaLauncher import org.gradle.jvm.toolchain.JavaToolchainService import org.gradle.process.JavaForkOptions +import org.gradle.work.Incremental import org.gradle.workers.ProcessWorkerSpec import org.gradle.workers.WorkerExecutor @@ -52,6 +47,8 @@ abstract class PlugGenerateTask : DefaultTask() { @get:Inject abstract val workerExecutor: WorkerExecutor + @get:InputDirectory abstract val discoveredPlugsDir: DirectoryProperty + @get:InputFiles @get:Classpath abstract val jarsToLinkAgainst: ConfigurableFileCollection @get:Internal var resourcesFolder: File? = null @@ -60,7 +57,7 @@ abstract class PlugGenerateTask : DefaultTask() { val atplugInfFolder: File get() = File(resourcesFolder, PlugPlugin.ATPLUG_INF) - @InputFiles var classesFolders: FileCollection? = null + @get:Incremental @get:InputFiles abstract val classesFolders: ConfigurableFileCollection init { this.outputs.upToDateWhen { @@ -74,20 +71,22 @@ abstract class PlugGenerateTask : DefaultTask() { launcher.set(service.launcherFor(spec)) } - fun setClassesFolders(files: Iterable) { - // if we don't copy, Gradle finds an implicit dependency which - // forces us to depend on `classes` even though we don't - val copy: MutableList = ArrayList() - for (file in files) { - copy.add(file) - } - classesFolders = project.files(copy) - } - @TaskAction fun build() { + val discoveredFiles = discoveredPlugsDir.get().asFile.listFiles().orEmpty().filter { it.isFile } + val plugsToSockets = + discoveredFiles.associate { + val pieces = it.readText().split("|") + pieces[0] to pieces[1] + } + if (plugsToSockets.isEmpty()) { + // no discovered plugs + FileMisc.cleanDir(atplugInfFolder) + return + } + // generate the metadata - val result = generate() + val result = generate(plugsToSockets) // clean out the ATPLUG-INF folder, and put the map's content into the folder FileMisc.cleanDir(atplugInfFolder) @@ -97,8 +96,8 @@ abstract class PlugGenerateTask : DefaultTask() { } // the resources directory *needs* the Service-Component entry of the manifest to exist in order - // for tests to work - // so we'll get a manifest (empty if necessary, but preferably we'll load what already exists) + // for tests to work so we'll get a manifest (empty if necessary, but preferably we'll load what + // already exists) val manifest = loadManifest() val componentsCmd = atplugComponents(atplugInfFolder) val componentsActual = manifest.mainAttributes.getValue(PlugPlugin.SERVICE_COMPONENT) @@ -138,9 +137,10 @@ abstract class PlugGenerateTask : DefaultTask() { } } - private fun generate(): SortedMap { + private fun generate(discoveredPlugs: Map): SortedMap { val input = - PlugGeneratorJavaExecable(ArrayList(classesFolders!!.files), jarsToLinkAgainst.files) + PlugGeneratorJavaExecable( + discoveredPlugs, ArrayList(classesFolders.files), jarsToLinkAgainst.files) return if (launcher.isPresent) { val workQueue = workerExecutor.processIsolation { workerSpec: ProcessWorkerSpec -> diff --git a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugPlugin.kt b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugPlugin.kt index a2de995..9460011 100644 --- a/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugPlugin.kt +++ b/atplug-plugin-gradle/src/main/java/com/diffplug/atplug/tooling/gradle/PlugPlugin.kt @@ -50,32 +50,37 @@ class PlugPlugin : Plugin { plugGen.dependencies.add(dep) } - // jar --dependsOn--> plugGenerate - val generateTask = - project.tasks.register(GENERATE, PlugGenerateTask::class.java) { - task: PlugGenerateTask -> - task.setClassesFolders(main.output.classesDirs) - task.jarsToLinkAgainst.setFrom(plugGenConfig) - task.resourcesFolder = main.resources.sourceDirectories.singleFile + val findPlugsTask = + project.tasks.register("plugFind", FindPlugsTask::class.java) { + it.classesFolders.setFrom(main.output.classesDirs) + it.discoveredPlugsDir.set(project.layout.buildDirectory.dir("foundPlugs")) // dep on java for (taskName in mutableListOf("compileJava", "compileKotlin")) { try { - task.dependsOn(project.tasks.named(taskName)) + it.dependsOn(project.tasks.named(taskName)) } catch (e: UnknownTaskException) { - // not a problem + // not a problem if we only have kotlin } } } - project.tasks.named(JavaPlugin.JAR_TASK_NAME).configure { jarTaskUntyped: Task -> - val jarTask = jarTaskUntyped as Jar - val metadataTask = generateTask.get() + + val generatePlugsTask = + project.tasks.register("plugGenerate", PlugGenerateTask::class.java) { + it.discoveredPlugsDir.set(findPlugsTask.flatMap { it.discoveredPlugsDir }) + it.classesFolders.setFrom(main.output.classesDirs) + it.jarsToLinkAgainst.setFrom(plugGenConfig) + it.resourcesFolder = main.resources.sourceDirectories.singleFile + it.dependsOn(findPlugsTask) + } + project.tasks.named(JavaPlugin.JAR_TASK_NAME).configure { + val jarTask = it as Jar + val metadataTask = generatePlugsTask.get() jarTask.inputs.dir(metadataTask.atplugInfFolder) jarTask.doFirst( - "Set " + SERVICE_COMPONENT + " header", - SetServiceComponentHeader(metadataTask.atplugInfFolder)) + "Set $SERVICE_COMPONENT header", SetServiceComponentHeader(metadataTask.atplugInfFolder)) } project.tasks.named(JavaPlugin.PROCESS_RESOURCES_TASK_NAME).configure { t: Task -> - t.dependsOn(generateTask) + t.dependsOn(generatePlugsTask) } } @@ -93,7 +98,6 @@ class PlugPlugin : Plugin { } companion object { - const val GENERATE = "plugGenerate" const val SERVICE_COMPONENT = "AtPlug-Component" const val DOT_JSON = ".json" const val ATPLUG_INF = "ATPLUG-INF/" diff --git a/atplug-plugin-gradle/src/test/java/com/diffplug/atplug/tooling/PlugGeneratorTest.kt b/atplug-plugin-gradle/src/test/java/com/diffplug/atplug/tooling/PlugGeneratorTest.kt index 40a540e..4cb6200 100644 --- a/atplug-plugin-gradle/src/test/java/com/diffplug/atplug/tooling/PlugGeneratorTest.kt +++ b/atplug-plugin-gradle/src/test/java/com/diffplug/atplug/tooling/PlugGeneratorTest.kt @@ -33,6 +33,11 @@ class PlugGeneratorTest : ResourceHarness() { fun generateMetadata() { val maps = PlugGenerator.generate( + mapOf( + "com.diffplug.atplug.Apple" to "com.diffplug.atplug.Fruit", + "com.diffplug.atplug.Orange" to "com.diffplug.atplug.Fruit", + "com.diffplug.atplug.Shape\$Circle" to "com.diffplug.atplug.Shape", + "com.diffplug.atplug.Shape\$Square" to "com.diffplug.atplug.Shape"), listOf("java", "kotlin").map { File("../atplug-runtime/build/classes/$it/test") }, deps()) Assertions.assertEquals(