From 458b40eafcc7a2095107ccd1549ca2a7c09ad11b Mon Sep 17 00:00:00 2001 From: Cedric Beust Date: Mon, 18 Apr 2016 02:51:29 -0800 Subject: [PATCH] Revamp the graph dependency/ordering logic. --- .../com/beust/kobalt/api/ITaskContributor.kt | 3 +- .../com/beust/kobalt/api/TaskContributor.kt | 12 +- .../kobalt/api/annotation/Annotations.kt | 24 +- .../com/beust/kobalt/internal/DynamicGraph.kt | 2 +- .../kobalt/internal/JvmCompilerPlugin.kt | 4 +- .../com/beust/kobalt/internal/TaskManager.kt | 227 +++++++----------- .../plugin/application/ApplicationPlugin.kt | 2 +- .../plugin/packaging/PackagingPlugin.kt | 4 +- .../kobalt/plugin/publish/PublishPlugin.kt | 6 +- .../beust/kobalt/internal/TaskManagerTest.kt | 19 +- 10 files changed, 128 insertions(+), 175 deletions(-) diff --git a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/ITaskContributor.kt b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/ITaskContributor.kt index 1cd39e8c..8a93b813 100644 --- a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/ITaskContributor.kt +++ b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/ITaskContributor.kt @@ -11,7 +11,8 @@ interface ITaskContributor : IContributor { } class DynamicTask(val plugin: IPlugin, val name: String, val description: String = "", + val dependsOn: List = listOf(), + val reverseDependsOn: List = listOf(), val runBefore: List = listOf(), val runAfter: List = listOf(), - val alwaysRunAfter: List = listOf(), val closure: (Project) -> TaskResult) diff --git a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/TaskContributor.kt b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/TaskContributor.kt index 29e02f08..4d901040 100644 --- a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/TaskContributor.kt +++ b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/TaskContributor.kt @@ -23,16 +23,18 @@ class TaskContributor @Inject constructor(val incrementalManagerFactory: Increme * depends on variants of that task. */ fun addVariantTasks(plugin: IPlugin, project: Project, context: KobaltContext, taskName: String, + dependsOn: List = emptyList(), + reverseDependsOn : List = emptyList(), runBefore : List = emptyList(), runAfter : List = emptyList(), - alwaysRunAfter : List = emptyList(), runTask: (Project) -> TaskResult) { Variant.allVariants(project).forEach { variant -> val variantTaskName = variant.toTask(taskName) dynamicTasks.add(DynamicTask(plugin, variantTaskName, variantTaskName, + dependsOn = dependsOn.map { variant.toTask(it) }, + reverseDependsOn = reverseDependsOn.map { variant.toTask(it) }, runBefore = runBefore.map { variant.toTask(it) }, runAfter = runAfter.map { variant.toTask(it) }, - alwaysRunAfter = alwaysRunAfter.map { variant.toTask(it) }, closure = { p: Project -> context.variant = variant runTask(project) @@ -41,16 +43,18 @@ class TaskContributor @Inject constructor(val incrementalManagerFactory: Increme } fun addIncrementalVariantTasks(plugin: IPlugin, project: Project, context: KobaltContext, taskName: String, + dependsOn: List = emptyList(), + reverseDependsOn : List = emptyList(), runBefore : List = emptyList(), runAfter : List = emptyList(), - alwaysRunAfter : List = emptyList(), runTask: (Project) -> IncrementalTaskInfo) { Variant.allVariants(project).forEach { variant -> val variantTaskName = variant.toTask(taskName) dynamicTasks.add(DynamicTask(plugin, variantTaskName, variantTaskName, + dependsOn = dependsOn.map { variant.toTask(it) }, + reverseDependsOn = reverseDependsOn.map { variant.toTask(it) }, runBefore = runBefore.map { variant.toTask(it) }, runAfter = runAfter.map { variant.toTask(it) }, - alwaysRunAfter = alwaysRunAfter.map { variant.toTask(it) }, closure = incrementalManagerFactory.create().toIncrementalTaskClosure(taskName, runTask, variant))) } } diff --git a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/annotation/Annotations.kt b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/annotation/Annotations.kt index 5488be5a..3d3c9038 100644 --- a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/annotation/Annotations.kt +++ b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/api/annotation/Annotations.kt @@ -11,14 +11,17 @@ annotation class Task( val name: String, val description: String = "", - /** Tasks that this task depends on */ + /** Dependency: tasks this task depends on */ + val dependsOn: Array = arrayOf(), + + /** Dependency: tasks this task will be made dependend upon */ + val reverseDependsOn: Array = arrayOf(), + + /** Ordering: tasks that need to be run before this one */ val runBefore: Array = arrayOf(), - /** Tasks that this task will run after if they get run */ + /** Ordering: tasks this task runs after */ val runAfter: Array = arrayOf(), - - /** Tasks that this task will always run after */ - val alwaysRunAfter: Array = arrayOf() ) @Retention(AnnotationRetention.RUNTIME) @@ -26,14 +29,17 @@ annotation class IncrementalTask( val name: String, val description: String = "", + /** Dependency: tasks this task depends on */ + val dependsOn: Array = arrayOf(), + + /** Dependency: tasks this task will be made dependend upon */ + val reverseDependsOn: Array = arrayOf(), + /** Tasks that this task depends on */ val runBefore: Array = arrayOf(), - /** Tasks that this task will run after if they get run */ + /** Ordering: tasks this task runs after */ val runAfter: Array = arrayOf(), - - /** Tasks that this task will always run after */ - val alwaysRunAfter: Array = arrayOf() ) /** diff --git a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/DynamicGraph.kt b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/DynamicGraph.kt index e90dad5d..998ed73e 100644 --- a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/DynamicGraph.kt +++ b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/DynamicGraph.kt @@ -24,7 +24,7 @@ class Node(val value: T) { } class DynamicGraph { - val VERBOSE = 1 + val VERBOSE = 2 val values : Collection get() = nodes.map { it.value } val nodes = hashSetOf>() private val dependedUpon = HashMultimap.create, Node>() diff --git a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/JvmCompilerPlugin.kt b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/JvmCompilerPlugin.kt index 93a4f5ab..baaa1de3 100644 --- a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/JvmCompilerPlugin.kt +++ b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/JvmCompilerPlugin.kt @@ -78,7 +78,7 @@ open class JvmCompilerPlugin @Inject constructor( val taskName = if (config.name.isEmpty()) "test" else "test" + config.name taskManager.addTask(this, project, taskName, - runAfter = listOf(JvmCompilerPlugin.TASK_COMPILE, JvmCompilerPlugin.TASK_COMPILE_TEST), + dependsOn = listOf(JvmCompilerPlugin.TASK_COMPILE, JvmCompilerPlugin.TASK_COMPILE_TEST), task = { taskTest(project, config.name)} ) } @@ -140,7 +140,7 @@ open class JvmCompilerPlugin @Inject constructor( } @IncrementalTask(name = TASK_COMPILE_TEST, description = "Compile the tests", - runAfter = arrayOf(TASK_COMPILE)) + dependsOn = arrayOf(TASK_COMPILE)) fun taskCompileTest(project: Project): IncrementalTaskInfo { sourceTestDirectories.addAll(context.variant.sourceDirectories(project, context, SourceSet.of(isTest = true))) return IncrementalTaskInfo( diff --git a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/TaskManager.kt b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/TaskManager.kt index ae4aa860..8ca71672 100644 --- a/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/TaskManager.kt +++ b/modules/kobalt-plugin-api/src/main/kotlin/com/beust/kobalt/internal/TaskManager.kt @@ -21,30 +21,32 @@ import javax.inject.Inject import javax.inject.Singleton @Singleton -public class TaskManager @Inject constructor(val args: Args, +class TaskManager @Inject constructor(val args: Args, val incrementalManagerFactory: IncrementalManager.IFactory) { + private val dependsOn = TreeMultimap.create() + private val reverseDependsOn = TreeMultimap.create() private val runBefore = TreeMultimap.create() private val runAfter = TreeMultimap.create() - private val alwaysRunAfter = TreeMultimap.create() /** - * Called by plugins to indicate task dependencies defined at runtime. Keys depend on values. - * Declare that `task1` depends on `task2`. - * - * Note: there is no runAfter on this class since a runAfter(a, b) in a task simply translates - * to a runBefore(b, a) here. + * Dependency: task2 depends on task 1. */ - fun runBefore(task1: String, task2: String) { - runBefore.put(task1, task2) - } + fun dependsOn(task1: String, task2: String) = dependsOn.put(task2, task1) - fun runAfter(task1: String, task2: String) { - runAfter.put(task1, task2) - } + /** + * Dependency: task2 depends on task 1. + */ + fun reverseDependsOn(task1: String, task2: String) = reverseDependsOn.put(task1, task2) - fun alwaysRunAfter(task1: String, task2: String) { - alwaysRunAfter.put(task1, task2) - } + /** + * Ordering: task1 runs before task 2. + */ + fun runBefore(task1: String, task2: String) = runBefore.put(task1, task2) + + /** + * Ordering: task2 runs after task 1. + */ + fun runAfter(task1: String, task2: String) = runAfter.put(task2, task1) data class TaskInfo(val id: String) { constructor(project: String, task: String) : this(project + ":" + task) @@ -58,7 +60,7 @@ public class TaskManager @Inject constructor(val args: Args, class RunTargetResult(val exitCode: Int, val messages: List) - public fun runTargets(taskNames: List, projects: List) : RunTargetResult { + fun runTargets(taskNames: List, projects: List) : RunTargetResult { var result = 0 val failedProjects = hashSetOf() val messages = Collections.synchronizedList(arrayListOf()) @@ -94,7 +96,7 @@ public class TaskManager @Inject constructor(val args: Args, } val graph = createGraph(project.name, taskNames, tasksByNames, - runBefore, runAfter, alwaysRunAfter, + dependsOn, reverseDependsOn, runBefore, runAfter, { task: PluginTask -> task.name }, { task: PluginTask -> task.plugin.accept(project) }) @@ -122,109 +124,81 @@ public class TaskManager @Inject constructor(val args: Args, return RunTargetResult(result, messages) } + /** + * Create a dynamic graph representing all the tasks that need to be run. + */ @VisibleForTesting - fun createGraph(projectName: String, taskNames: List, dependencies: Multimap, + fun createGraph(projectName: String, taskNames: List, nodeMap: Multimap, + dependsOn: TreeMultimap, + reverseDependsOn: TreeMultimap, runBefore: TreeMultimap, runAfter: TreeMultimap, - alwaysRunAfter: TreeMultimap, toName: (T) -> String, accept: (T) -> Boolean): DynamicGraph { - val graph = DynamicGraph() + + val result = DynamicGraph() taskNames.forEach { taskName -> val ti = TaskInfo(taskName) - if (!dependencies.keys().contains(ti.taskName)) { + if (!nodeMap.keys().contains(ti.taskName)) { throw KobaltException("Unknown task: $taskName") } if (ti.matches(projectName)) { - dependencies[ti.taskName].forEach { task -> + nodeMap[ti.taskName].forEach { task -> if (task != null && accept(task)) { - val reverseAfter = hashMapOf() - alwaysRunAfter.keys().forEach { from -> - val tasks = alwaysRunAfter.get(from) - tasks.forEach { - reverseAfter.put(it, from) - } - } + val toProcess = arrayListOf(task) + val seen = hashSetOf() + val newToProcess = arrayListOf() + while (toProcess.size > 0) { + toProcess.forEach { current -> + result.addNode(current) + seen.add(toName(current)) - // - // If the current target is free, add it as a single node to the graph - // - val allFreeTasks = calculateFreeTasks(dependencies, reverseAfter) - val currentFreeTask = allFreeTasks.filter { - TaskInfo(projectName, toName(it)).taskName == toName(task) - } - if (currentFreeTask.size == 1) { - currentFreeTask[0].let { - graph.addNode(it) - } - } - - // - // Add the transitive closure of the current task as edges to the graph - // - val transitiveClosure = calculateTransitiveClosure(projectName, dependencies, ti) - transitiveClosure.forEach { pluginTask -> - val rb = runBefore.get(toName(pluginTask)) - rb.forEach { - val tos = dependencies[it] - if (tos != null && tos.size > 0) { - tos.forEach { to -> - graph.addEdge(pluginTask, to) - } - } else { - log(2, "Couldn't find node $it: not applicable to project $projectName") - } - } - } - - // - // runAfter nodes are run only if they are explicitly requested - // - arrayListOf().let { allNodes -> - allNodes.addAll(graph.values) - allNodes.forEach { node -> - val nodeName = toName(node) - if (taskNames.contains(nodeName)) { - val ra = runAfter[nodeName] - ra?.forEach { o -> - dependencies[o]?.forEach { - if (taskNames.contains(toName(it))) { - graph.addEdge(node, it) + fun maybeAddEdge(taskName: String, mm: Multimap, isDependency: Boolean, + reverseEdges: Boolean = false) { + mm[taskName]?.forEach { + val addEdge = isDependency || (!isDependency && taskNames.contains(it)) + log(3, " addEdge: $addEdge $taskName") + if (addEdge) { + nodeMap[it].forEach { to -> + if (reverseEdges) { + log(3, " Adding reverse edge $to -> $task") + result.addEdge(to, task) + } else { + log(3, " Adding edge $task -> $to") + result.addEdge(task, to) + } + if (!seen.contains(it)) newToProcess.add(to) } + seen.add(it) } } } + maybeAddEdge(ti.taskName, dependsOn, true, false) + maybeAddEdge(ti.taskName, reverseDependsOn, true, true) + maybeAddEdge(ti.taskName, runBefore, false, false) + maybeAddEdge(ti.taskName, runAfter, false, false) } - } - - // - // If any of the nodes in the graph has an "alwaysRunAfter", add that edge too - // - arrayListOf().let { allNodes -> - allNodes.addAll(graph.values) - allNodes.forEach { node -> - val ra = alwaysRunAfter[toName(node)] - ra?.forEach { o -> - dependencies[o]?.forEach { - graph.addEdge(it, node) - } - } - } + toProcess.clear() + toProcess.addAll(newToProcess) + newToProcess.clear() } } } + } else { + log(3, "Task $taskName does not match the current project $projectName, skipping it") } } - println("@@@ " + graph.dump()) - return graph + return result } /** * Find the free tasks of the graph. */ - private fun calculateFreeTasks(tasksByNames: Multimap, reverseAfter: HashMap) + private fun calculateFreeTasks(tasksByNames: Multimap, runBefore: TreeMultimap, + reverseAfter: HashMap) : Collection { val freeTaskMap = hashMapOf() tasksByNames.keys().forEach { @@ -238,49 +212,6 @@ public class TaskManager @Inject constructor(val args: Args, return freeTaskMap.values } - /** - * Find the transitive closure for the given TaskInfo - */ - private fun calculateTransitiveClosure(projectName: String, tasksByNames: Multimap, ti: TaskInfo): - HashSet { - log(3, "Processing ${ti.taskName}") - - val transitiveClosure = hashSetOf() - val seen = hashSetOf(ti.taskName) - val toProcess = hashSetOf(ti) - var done = false - while (! done) { - val newToProcess = hashSetOf() - log(3, "toProcess size: " + toProcess.size) - toProcess.forEach { target -> - - val currentTask = TaskInfo(projectName, target.taskName) - val thisTask = tasksByNames[currentTask.taskName] - if (thisTask != null) { - transitiveClosure.addAll(thisTask) - val dependencyNames = runBefore.get(currentTask.taskName) - dependencyNames.forEach { dependencyName -> - if (!seen.contains(dependencyName)) { - newToProcess.add(currentTask) - seen.add(dependencyName) - } - } - - dependencyNames.forEach { - newToProcess.add(TaskInfo(projectName, it)) - } - } else { - log(1, "Couldn't find task ${currentTask.taskName}: not applicable to project $projectName") - } - } - done = newToProcess.isEmpty() - toProcess.clear() - toProcess.addAll(newToProcess) - } - - return transitiveClosure - } - ///// // Manage the tasks // @@ -290,14 +221,16 @@ public class TaskManager @Inject constructor(val args: Args, private val taskAnnotations = arrayListOf() class TaskAnnotation(val method: Method, val plugin: IPlugin, val name: String, val description: String, - val runBefore: Array, val runAfter: Array, val alwaysRunAfter: Array, + val dependsOn: Array, val reverseDependsOn: Array, + val runBefore: Array, val runAfter: Array, val callable: (Project) -> TaskResult) /** * Invoking a @Task means simply calling the method and returning its returned TaskResult. */ fun toTaskAnnotation(method: Method, plugin: IPlugin, ta: Task) - = TaskAnnotation(method, plugin, ta.name, ta.description, ta.runBefore, ta.runAfter, ta.alwaysRunAfter, + = TaskAnnotation(method, plugin, ta.name, ta.description, ta.dependsOn, ta.reverseDependsOn, + ta.runBefore, ta.runAfter, { project -> method.invoke(plugin, project) as TaskResult }) @@ -307,7 +240,8 @@ public class TaskManager @Inject constructor(val args: Args, * of the returned IncrementalTaskInfo. */ fun toTaskAnnotation(method: Method, plugin: IPlugin, ta: IncrementalTask) - = TaskAnnotation(method, plugin, ta.name, ta.description, ta.runBefore, ta.runAfter, ta.alwaysRunAfter, + = TaskAnnotation(method, plugin, ta.name, ta.description, ta.dependsOn, ta.reverseDependsOn, + ta.runBefore, ta.runAfter, incrementalManagerFactory.create().toIncrementalTaskClosure(ta.name, { project -> method.invoke(plugin, project) as IncrementalTaskInfo })) @@ -338,8 +272,9 @@ public class TaskManager @Inject constructor(val args: Args, dynamicTasks.forEach { dynamicTask -> val task = dynamicTask.task projects.filter { dynamicTask.plugin.accept(it) }.forEach { project -> - addTask(dynamicTask.plugin, project, task.name, task.description, task.runBefore, task.runAfter, - task.alwaysRunAfter, task.closure) + addTask(dynamicTask.plugin, project, task.name, task.description, + task.dependsOn, task.reverseDependsOn, task.runBefore, task.runAfter, + task.closure) } } } @@ -360,14 +295,17 @@ public class TaskManager @Inject constructor(val args: Args, private fun addAnnotationTask(plugin: IPlugin, project: Project, annotation: TaskAnnotation, task: (Project) -> TaskResult) { - addTask(plugin, project, annotation.name, annotation.description, annotation.runBefore.toList(), - annotation.runAfter.toList(), annotation.alwaysRunAfter.toList(), task) + addTask(plugin, project, annotation.name, annotation.description, + annotation.dependsOn.toList(), annotation.reverseDependsOn.toList(), + annotation.runBefore.toList(), annotation.runAfter.toList(), + task) } fun addTask(plugin: IPlugin, project: Project, name: String, description: String = "", + dependsOn: List = listOf(), + reverseDependsOn: List = listOf(), runBefore: List = listOf(), runAfter: List = listOf(), - alwaysRunAfter: List = listOf(), task: (Project) -> TaskResult) { annotationTasks.add( object : BasePluginTask(plugin, name, description, project) { @@ -376,9 +314,10 @@ public class TaskManager @Inject constructor(val args: Args, return TaskResult2(taskResult.success, taskResult.errorMessage, this) } }) + dependsOn.forEach { dependsOn(it, name) } + reverseDependsOn.forEach { reverseDependsOn(it, name) } runBefore.forEach { runBefore(it, name) } runAfter.forEach { runAfter(it, name) } - alwaysRunAfter.forEach { alwaysRunAfter(it, name)} } // diff --git a/src/main/kotlin/com/beust/kobalt/plugin/application/ApplicationPlugin.kt b/src/main/kotlin/com/beust/kobalt/plugin/application/ApplicationPlugin.kt index 234239d1..a4855109 100644 --- a/src/main/kotlin/com/beust/kobalt/plugin/application/ApplicationPlugin.kt +++ b/src/main/kotlin/com/beust/kobalt/plugin/application/ApplicationPlugin.kt @@ -52,7 +52,7 @@ class ApplicationPlugin @Inject constructor(val configActor: ConfigActor) : List { - val runBefore = TreeMultimap.create().apply { + val dependsOn = TreeMultimap.create().apply { put("assemble", "compile") } + val reverseDependsOn = TreeMultimap.create().apply { + put("clean", "copyVersion") + put("compile", "postCompile") + } + val runBefore = TreeMultimap.create().apply { + } val runAfter = TreeMultimap.create().apply { put("compile", "clean") - put("postCompile", "compile") - } - val alwaysRunAfter = TreeMultimap.create().apply { - put("clean", "copyVersion") } val dependencies = TreeMultimap.create().apply { listOf("assemble", "compile", "clean", "copyVersion", "postCompile").forEach { put(it, it) } } - val graph = taskManager.createGraph("", tasks, dependencies, runBefore, runAfter, alwaysRunAfter, + val graph = taskManager.createGraph("", tasks, dependencies, dependsOn, reverseDependsOn, runBefore, runAfter, { it }, { t -> true }) val result = DryRunGraphExecutor(graph).run() return result @@ -55,11 +57,12 @@ class TaskManagerTest @Inject constructor(val taskManager: TaskManager) { @Test fun graphTest() { KobaltLogger.LOG_LEVEL = 3 + Assert.assertEquals(runTasks(listOf("compile")), listOf("compile", "postCompile")) Assert.assertEquals(runTasks(listOf("postCompile")), listOf("postCompile")) - Assert.assertEquals(runTasks(listOf("compile")), listOf("compile")) Assert.assertEquals(runTasks(listOf("compile", "postCompile")), listOf("compile", "postCompile")) Assert.assertEquals(runTasks(listOf("clean")), listOf("clean", "copyVersion")) - Assert.assertEquals(runTasks(listOf("clean", "compile")), listOf("clean", "compile", "copyVersion")) + Assert.assertEquals(runTasks(listOf("clean", "compile")), listOf("clean", "compile", "copyVersion", + "postCompile")) Assert.assertEquals(runTasks(listOf("assemble")), listOf("compile", "assemble")) Assert.assertEquals(runTasks(listOf("clean", "assemble")), listOf("clean", "compile", "assemble", "copyVersion"))