From e76119b5eb58b65375a6f9b51cfae38685809f26 Mon Sep 17 00:00:00 2001 From: Sebastian Sellmair Date: Thu, 16 Mar 2023 17:41:31 +0100 Subject: [PATCH] [Gradle] KotlinPluginLifecycle: Remove unnecessary before stages ^KT-34662 Verification Pending --- .../gradle/plugin/KotlinPluginLifecycle.kt | 38 ++------- .../KotlinMetadataTargetConfigurator.kt | 2 +- ...tlinAndroidTargetVariantTypeDslImplTest.kt | 2 +- .../unitTests/KotlinPluginLifecycleTest.kt | 78 +++++++++---------- .../unitTests/LifecycleAwarePropertyTest.kt | 8 +- 5 files changed, 50 insertions(+), 78 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycle.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycle.kt index 53217998df8..0773d789b28 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycle.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginLifecycle.kt @@ -241,18 +241,8 @@ internal interface KotlinPluginLifecycle { * Configure Phase of Gradle: No .afterEvaluate {} listeners have been called yet, * the buildscript is still evaluated! */ - Configure, - - /** - * The buildscript has been evaluated, first .afterEvaluate {} listeners get executed. - * This can include .afterEvaluates from other plugins as well as other users! - */ - AfterEvaluate, - - /** - * Before [FinaliseDsl] - */ - BeforeFinaliseDsl, + EvaluateBuildscript, + AfterEvaluateBuildscript, /** * Last changes are allowed to be done to the DSL. @@ -260,42 +250,22 @@ internal interface KotlinPluginLifecycle { * disallow further changes */ FinaliseDsl, - - /** - * After [FinaliseDsl] - */ AfterFinaliseDsl, - /** - * Before [FinaliseRefinesEdges] - */ - BeforeFinaliseRefinesEdges, /** * All refines edges ([KotlinSourceSet.dependsOn]) have to be finalised here. * Adding edges after this stage is forbidden and will throw an exception! */ FinaliseRefinesEdges, - - /** - * After [FinaliseRefinesEdges] - */ AfterFinaliseRefinesEdges, - /** - * Before [FinaliseCompilations] - */ - BeforeFinaliseCompilations, /** * [KotlinCompilation] instances have to finalised: Creating compilations after this stage is forbidden. * Values and configuration of compilations also shall be finalised already */ FinaliseCompilations, - - /** - * After [FinaliseCompilations] - */ AfterFinaliseCompilations, /** @@ -305,6 +275,8 @@ internal interface KotlinPluginLifecycle { val previousOrFirst: Stage get() = previousOrNull ?: values.first() val previousOrNull: Stage? get() = values.getOrNull(ordinal - 1) + val previousOrThrow: Stage + get() = previousOrNull ?: throw IllegalArgumentException("'$this' does not have a next ${Stage::class.simpleName}") val nextOrNull: Stage? get() = values.getOrNull(ordinal + 1) val nextOrLast: Stage get() = nextOrNull ?: values.last() val nextOrThrow: Stage @@ -447,7 +419,7 @@ private class KotlinPluginLifecycleImpl(override val project: Project) : KotlinP enqueuedActions.getValue(stage).addLast(action) - if (stage == Stage.Configure && isStarted.get()) { + if (stage == Stage.EvaluateBuildscript && isStarted.get()) { loopIfNecessary() } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt index ac231cfbe8d..092acd57a71 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/metadata/KotlinMetadataTargetConfigurator.kt @@ -183,7 +183,7 @@ class KotlinMetadataTargetConfigurator : private fun createMetadataCompilationsForCommonSourceSets( target: KotlinMetadataTarget, allMetadataJar: TaskProvider - ) = target.project.launchInStage(KotlinPluginLifecycle.Stage.BeforeFinaliseCompilations) { + ) = target.project.launchInStage(KotlinPluginLifecycle.Stage.AfterFinaliseDsl) { withRestrictedStages(KotlinPluginLifecycle.Stage.upTo(KotlinPluginLifecycle.Stage.FinaliseCompilations)) { // Do this after all targets are configured by the user build script diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinAndroidTargetVariantTypeDslImplTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinAndroidTargetVariantTypeDslImplTest.kt index a94d61f4dc8..bb9e15b004f 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinAndroidTargetVariantTypeDslImplTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinAndroidTargetVariantTypeDslImplTest.kt @@ -44,7 +44,7 @@ class KotlinAndroidTargetVariantTypeDslImplTest { @Test fun `test - module - cannot be set after FinaliseDsl`() = project.runLifecycleAwareTest { - launchInStage(KotlinPluginLifecycle.Stage.BeforeFinaliseDsl) { + launchInStage(KotlinPluginLifecycle.Stage.FinaliseDsl.previousOrThrow) { dsl.targetHierarchy.module.set(KotlinTargetHierarchy.ModuleName("x")) } diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinPluginLifecycleTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinPluginLifecycleTest.kt index 30363be17e4..b30621b0284 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinPluginLifecycleTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/KotlinPluginLifecycleTest.kt @@ -28,7 +28,7 @@ class KotlinPluginLifecycleTest { @Test fun `test - configure phase is executed right away`() { val invocations = AtomicInteger(0) - lifecycle.enqueue(Configure) { + lifecycle.enqueue(EvaluateBuildscript) { invocations.incrementAndGet() } assertEquals(1, invocations.get(), "Expected one invocation") @@ -37,8 +37,8 @@ class KotlinPluginLifecycleTest { @Test fun `test - launchInState - Configure`() { val invocations = AtomicInteger(0) - project.launchInStage(Configure) { - assertEquals(Configure, stage) + project.launchInStage(EvaluateBuildscript) { + assertEquals(EvaluateBuildscript, stage) assertEquals(1, invocations.incrementAndGet()) } assertEquals(1, invocations.get()) @@ -50,20 +50,20 @@ class KotlinPluginLifecycleTest { val nestedAInvocations = AtomicInteger(0) val nestedBInvocations = AtomicInteger(0) val nestedCInvocations = AtomicInteger(0) - lifecycle.enqueue(Configure) { + lifecycle.enqueue(EvaluateBuildscript) { assertEquals(1, outerInvocations.incrementAndGet()) - lifecycle.enqueue(Configure) nestedA@{ + lifecycle.enqueue(EvaluateBuildscript) nestedA@{ assertEquals(0, nestedBInvocations.get(), "Expected nestedA to be executed before nestedB") assertEquals(1, nestedAInvocations.incrementAndGet()) - lifecycle.enqueue(Configure) nestedC@{ + lifecycle.enqueue(EvaluateBuildscript) nestedC@{ assertEquals(1, nestedBInvocations.get(), "Expected nestedB to be executed before nestedC") assertEquals(1, nestedCInvocations.incrementAndGet()) } } - lifecycle.enqueue(Configure) nestedB@{ + lifecycle.enqueue(EvaluateBuildscript) nestedB@{ assertEquals(1, nestedAInvocations.get(), "Expected nestedA to be executed before nestedB") assertEquals(0, nestedCInvocations.get(), "Expected nestedB to be executed before nestedC") assertEquals(1, nestedBInvocations.incrementAndGet()) @@ -110,13 +110,13 @@ class KotlinPluginLifecycleTest { assertEquals(1, action3Invocations.incrementAndGet()) } - lifecycle.enqueue(AfterEvaluate) action1@{ + lifecycle.enqueue(AfterEvaluateBuildscript) action1@{ assertEquals(0, action2Invocations.get(), "Expected action1 to be executed before action2") assertEquals(0, action3Invocations.get(), "Expected action1 to be executed before action3") assertEquals(1, action1Invocations.incrementAndGet()) } - lifecycle.enqueue(AfterEvaluate) action2@{ + lifecycle.enqueue(AfterEvaluateBuildscript) action2@{ assertEquals(1, action1Invocations.get(), "Expected action1 to be executed before action2") assertEquals(0, action3Invocations.get(), "Expected action2 to be executed before action3") assertEquals(1, action2Invocations.incrementAndGet()) @@ -139,20 +139,20 @@ class KotlinPluginLifecycleTest { val nestedAInvocations = AtomicInteger(0) val nestedBInvocations = AtomicInteger(0) val nestedCInvocations = AtomicInteger(0) - lifecycle.enqueue(AfterEvaluate) { + lifecycle.enqueue(AfterEvaluateBuildscript) { assertEquals(1, outerInvocations.incrementAndGet()) - lifecycle.enqueue(AfterEvaluate) nestedA@{ + lifecycle.enqueue(AfterEvaluateBuildscript) nestedA@{ assertEquals(0, nestedBInvocations.get(), "Expected nestedA to be executed before nestedB") assertEquals(1, nestedAInvocations.incrementAndGet()) - lifecycle.enqueue(AfterEvaluate) nestedC@{ + lifecycle.enqueue(AfterEvaluateBuildscript) nestedC@{ assertEquals(1, nestedBInvocations.get(), "Expected nestedB to be executed before nestedC") assertEquals(1, nestedCInvocations.incrementAndGet()) } } - lifecycle.enqueue(AfterEvaluate) nestedB@{ + lifecycle.enqueue(AfterEvaluateBuildscript) nestedB@{ assertEquals(1, nestedAInvocations.get(), "Expected nestedA to be executed before nestedB") assertEquals(0, nestedCInvocations.get(), "Expected nestedB to be executed before nestedC") assertEquals(1, nestedBInvocations.incrementAndGet()) @@ -177,7 +177,7 @@ class KotlinPluginLifecycleTest { val executed = AtomicBoolean(false) lifecycle.enqueue(ReadyForExecution) { assertFailsWith { - lifecycle.enqueue(AfterEvaluate) { fail("This code shall not be executed!") } + lifecycle.enqueue(AfterEvaluateBuildscript) { fail("This code shall not be executed!") } } assertFalse(executed.getAndSet(true)) } @@ -202,14 +202,14 @@ class KotlinPluginLifecycleTest { val action2Invocations = AtomicInteger(0) val action3Invocations = AtomicInteger(0) - lifecycle.enqueue(Configure) action1@{ + lifecycle.enqueue(EvaluateBuildscript) action1@{ assertEquals(0, action2Invocations.get()) assertEquals(0, action3Invocations.get()) assertEquals(1, action1Invocations.incrementAndGet()) } - lifecycle.enqueue(Configure) action2@{ - lifecycle.enqueue(Configure) action3@{ + lifecycle.enqueue(EvaluateBuildscript) action2@{ + lifecycle.enqueue(EvaluateBuildscript) action3@{ assertEquals(1, action1Invocations.get()) assertEquals(1, action2Invocations.get()) assertEquals(1, action3Invocations.incrementAndGet()) @@ -259,9 +259,9 @@ class KotlinPluginLifecycleTest { val executionPointB = AtomicBoolean(false) lifecycle.launch action1@{ assertFalse(executionPointA.getAndSet(true)) - assertEquals(Configure, stage) - await(AfterEvaluate) - assertEquals(AfterEvaluate, stage) + assertEquals(EvaluateBuildscript, stage) + await(AfterEvaluateBuildscript) + assertEquals(AfterEvaluateBuildscript, stage) assertFalse(executionPointB.getAndSet(true)) } @@ -276,9 +276,9 @@ class KotlinPluginLifecycleTest { fun `test - launch - await - launch`() { val executedInnerAction = AtomicBoolean(false) lifecycle.launch { - await(AfterEvaluate) + await(AfterEvaluateBuildscript) launch { - assertEquals(AfterEvaluate, stage) + assertEquals(AfterEvaluateBuildscript, stage) await(FinaliseRefinesEdges) assertEquals(FinaliseRefinesEdges, stage) assertFalse(executedInnerAction.getAndSet(true)) @@ -303,7 +303,7 @@ class KotlinPluginLifecycleTest { fun `test - launch - await - exception`() { val testException = object : Throwable() {} lifecycle.launch { - await(AfterEvaluate) + await(AfterEvaluateBuildscript) launch { throw testException } @@ -318,7 +318,7 @@ class KotlinPluginLifecycleTest { @Test fun `test - require current stage`() = project.runLifecycleAwareTest { - launchInStage(AfterEvaluate) { + launchInStage(AfterEvaluateBuildscript) { requireCurrentStage { } // OK requireCurrentStage { @@ -330,10 +330,10 @@ class KotlinPluginLifecycleTest { @Test fun `test - launch in required stage`() = project.runLifecycleAwareTest { - launchInRequiredStage(AfterEvaluate) { - assertEquals(AfterEvaluate, stage) - await(AfterEvaluate) - assertEquals(AfterEvaluate, stage) + launchInRequiredStage(AfterEvaluateBuildscript) { + assertEquals(AfterEvaluateBuildscript, stage) + await(AfterEvaluateBuildscript) + assertEquals(AfterEvaluateBuildscript, stage) assertFailsWith { await(ReadyForExecution) } } } @@ -342,13 +342,13 @@ class KotlinPluginLifecycleTest { fun `test - withRestrictedStages`() = project.runLifecycleAwareTest { launch { withRestrictedStages(Stage.upTo(FinaliseRefinesEdges)) { - assertEquals(Configure, stage) + assertEquals(EvaluateBuildscript, stage) - await(AfterEvaluate) - assertEquals(AfterEvaluate, stage) + await(AfterEvaluateBuildscript) + assertEquals(AfterEvaluateBuildscript, stage) - await(BeforeFinaliseRefinesEdges) - assertEquals(BeforeFinaliseRefinesEdges, stage) + await(FinaliseDsl) + assertEquals(FinaliseDsl, stage) await(FinaliseRefinesEdges) assertEquals(FinaliseRefinesEdges, stage) @@ -366,12 +366,12 @@ class KotlinPluginLifecycleTest { afterEvaluate { launch { - assertEquals(AfterEvaluate.nextOrThrow, currentKotlinPluginLifecycle().stage) + assertEquals(AfterEvaluateBuildscript.nextOrThrow, currentKotlinPluginLifecycle().stage) assertEquals(1, actionInvocations.incrementAndGet()) } } - await(AfterEvaluate.nextOrThrow.nextOrThrow) + await(AfterEvaluateBuildscript.nextOrThrow.nextOrThrow) assertEquals(1, actionInvocations.get()) await(Stage.values.last()) } @@ -411,13 +411,13 @@ class KotlinPluginLifecycleTest { @Test fun `test - Stage - range utils`() { - assertEquals(setOf(BeforeFinaliseDsl, FinaliseDsl, AfterFinaliseDsl), BeforeFinaliseDsl..AfterFinaliseDsl) - assertEquals(emptySet(), AfterFinaliseDsl..BeforeFinaliseDsl) - assertEquals(setOf(BeforeFinaliseDsl), BeforeFinaliseDsl..BeforeFinaliseDsl) + assertEquals(setOf(AfterEvaluateBuildscript, FinaliseDsl, AfterFinaliseDsl), AfterEvaluateBuildscript..AfterFinaliseDsl) + assertEquals(emptySet(), AfterFinaliseDsl..AfterFinaliseDsl.previousOrThrow) + assertEquals(setOf(FinaliseDsl), FinaliseDsl..FinaliseDsl) assertTrue(FinaliseDsl in Stage.upTo(FinaliseDsl)) assertTrue(Stage.values.first() in Stage.upTo(FinaliseDsl)) - assertTrue(BeforeFinaliseDsl in Stage.upTo(FinaliseDsl)) + assertTrue(FinaliseDsl.previousOrThrow in Stage.upTo(FinaliseDsl)) assertTrue(FinaliseDsl.nextOrThrow !in Stage.upTo(FinaliseDsl)) assertTrue(FinaliseDsl !in Stage.until(FinaliseDsl)) diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/LifecycleAwarePropertyTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/LifecycleAwarePropertyTest.kt index faaf4bb6dca..668e476df09 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/LifecycleAwarePropertyTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/LifecycleAwarePropertyTest.kt @@ -30,7 +30,7 @@ class LifecycleAwarePropertyTest { val property by project.newKotlinPluginLifecycleAwareProperty(AfterFinaliseRefinesEdges) assertTrue(property.isKotlinPluginLifecycleAware()) - launchInStage(BeforeFinaliseRefinesEdges) { + launchInStage(FinaliseRefinesEdges.previousOrThrow) { property.set(1) } @@ -39,7 +39,7 @@ class LifecycleAwarePropertyTest { property.set(2) } - assertEquals(Configure, currentKotlinPluginLifecycle().stage) + assertEquals(EvaluateBuildscript, currentKotlinPluginLifecycle().stage) assertEquals(2, property.awaitFinalValue()) assertEquals(AfterFinaliseRefinesEdges, currentKotlinPluginLifecycle().stage) } @@ -53,10 +53,10 @@ class LifecycleAwarePropertyTest { @Test fun `test - changing value after finalized`() = project.runLifecycleAwareTest { - val property by project.newKotlinPluginLifecycleAwareProperty(AfterEvaluate) + val property by project.newKotlinPluginLifecycleAwareProperty(AfterEvaluateBuildscript) property.set(1) - launchInStage(AfterEvaluate) { + launchInStage(AfterEvaluateBuildscript) { assertFailsWith { property.set(2) } } }