From 28310eb970da276a765f3f0c870af158236f7603 Mon Sep 17 00:00:00 2001 From: Ilya Goncharov Date: Mon, 8 Aug 2022 08:55:47 +0000 Subject: [PATCH] [Gradle, JS] Add error state to know when operation was failed --- .../kotlin/gradle/Kotlin2JsGradlePluginIT.kt | 31 ++++++++++++ .../gradle/targets/js/dukat/DukatTask.kt | 6 ++- .../targets/js/nodejs/NodeJsRootExtension.kt | 2 +- .../js/npm/KotlinNpmResolutionManager.kt | 49 +++++++++++++------ .../js/npm/resolver/KotlinRootNpmResolver.kt | 18 ++++--- .../js/npm/tasks/KotlinNpmInstallTask.kt | 3 +- .../gradle/targets/js/testing/KotlinJsTest.kt | 2 +- .../targets/js/webpack/KotlinWebpack.kt | 2 +- 8 files changed, 84 insertions(+), 29 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/Kotlin2JsGradlePluginIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/Kotlin2JsGradlePluginIT.kt index 2ff54ddce85..a09b7b62d5a 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/Kotlin2JsGradlePluginIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/Kotlin2JsGradlePluginIT.kt @@ -1564,6 +1564,37 @@ abstract class AbstractKotlin2JsGradlePluginIT(protected val irBackend: Boolean) } } } + + @DisplayName("smoothly fail npm install") + @GradleTest + fun testFailNpmInstall(gradleVersion: GradleVersion) { + project("kotlin-js-browser-project", gradleVersion) { + buildGradleKts.modify { originalScript -> + buildString { + append(originalScript) + append( + """ + | + |plugins.withType { + | val nodejs = the() + | tasks.named("kotlinNpmInstall") { + | doFirst { + | nodejs.npmResolutionManager.state = + | org.jetbrains.kotlin.gradle.targets.js.npm.KotlinNpmResolutionManager.ResolutionState.Error(GradleException("someSpecialException")) + | } + | } + |} + | + """.trimMargin() + ) + } + } + buildAndFail("build") { + assertTasksFailed(":kotlinNpmInstall") + assertOutputContains("someSpecialException") + } + } + } } @JsGradlePluginTests diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/dukat/DukatTask.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/dukat/DukatTask.kt index fba806a8e6a..15b9acaeaa8 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/dukat/DukatTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/dukat/DukatTask.kt @@ -10,6 +10,7 @@ import org.gradle.api.tasks.* import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinJsCompilation import org.jetbrains.kotlin.gradle.targets.js.RequiredKotlinJsDependency import org.jetbrains.kotlin.gradle.targets.js.nodejs.NodeJsRootPlugin +import org.jetbrains.kotlin.gradle.targets.js.npm.KotlinNpmResolutionManager import org.jetbrains.kotlin.gradle.targets.js.npm.RequiresNpmDependencies import org.jetbrains.kotlin.gradle.targets.js.npm.npmProject import java.io.File @@ -53,7 +54,8 @@ abstract class DukatTask( get() = resolutionManager.requireInstalled( services, logger - )[projectPath][compilationName] + )?.get(projectPath)?.get(compilationName) + ?: throw (resolutionManager.state as KotlinNpmResolutionManager.ResolutionState.Error).wrappedException @get:Internal val dts: List @@ -97,7 +99,7 @@ abstract class DukatTask( @TaskAction open fun run() { - resolutionManager.checkRequiredDependencies(this, services, logger, projectPath) + resolutionManager.checkRequiredDependencies(this) destinationDir.deleteRecursively() diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/nodejs/NodeJsRootExtension.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/nodejs/NodeJsRootExtension.kt index 570723e7673..96f4d53ac30 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/nodejs/NodeJsRootExtension.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/nodejs/NodeJsRootExtension.kt @@ -137,7 +137,7 @@ open class NodeJsRootExtension(@Transient val rootProject: Project) : Configurat val versions = NpmVersions() - internal val npmResolutionManager = KotlinNpmResolutionManager(this) + val npmResolutionManager = KotlinNpmResolutionManager(this) companion object { const val EXTENSION_NAME: String = "kotlinNodeJs" diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/KotlinNpmResolutionManager.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/KotlinNpmResolutionManager.kt index 4a2a90d2947..8b52e4befd2 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/KotlinNpmResolutionManager.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/KotlinNpmResolutionManager.kt @@ -86,7 +86,7 @@ class KotlinNpmResolutionManager(@Transient private val nodeJsSettings: NodeJsRo (nodeJsSettings ?: unavailableValueError("nodeJsSettings")).rootProject.isInIdeaSync } - internal val resolver = KotlinRootNpmResolver(nodeJsSettings, forceFullResolve) + val resolver = KotlinRootNpmResolver(nodeJsSettings, forceFullResolve) internal abstract class KotlinNpmResolutionManagerStateHolder : BuildService { @Volatile @@ -101,13 +101,13 @@ class KotlinNpmResolutionManager(@Transient private val nodeJsSettings: NodeJsRo private val stateHolder get() = stateHolderProvider.get() - private var state: ResolutionState + var state: ResolutionState get() = stateHolder.state ?: ResolutionState.Configuring(resolver) set(value) { stateHolder.state = value } - internal sealed class ResolutionState { + sealed class ResolutionState { abstract val npmProjects: List class Configuring(val resolver: KotlinRootNpmResolver) : ResolutionState() { @@ -120,11 +120,16 @@ class KotlinNpmResolutionManager(@Transient private val nodeJsSettings: NodeJsRo get() = npmProjectsByProjectResolutions(preparedInstallation.projectResolutions) } - class Installed(val resolved: KotlinRootNpmResolution) : ResolutionState() { + class Installed internal constructor(internal val resolved: KotlinRootNpmResolution) : ResolutionState() { override val npmProjects: List get() = npmProjectsByProjectResolutions(resolved.projects) } + class Error(val wrappedException: Throwable) : ResolutionState() { + override val npmProjects: List + get() = emptyList() + } + companion object { fun npmProjectsByProjectResolutions( resolutions: Map @@ -156,23 +161,30 @@ class KotlinNpmResolutionManager(@Transient private val nodeJsSettings: NodeJsRo args: List = emptyList(), services: ServiceRegistry, logger: Logger - ): KotlinRootNpmResolution { + ): KotlinRootNpmResolution? { synchronized(stateHolder) { if (state is ResolutionState.Installed) { return (state as ResolutionState.Installed).resolved } - val installUpToDate = nodeJsSettings?.npmInstallTaskProvider?.get()?.state?.upToDate ?: false - val forceUpToDate = installUpToDate && !forceFullResolve + if (state is ResolutionState.Error) { + return null + } - val installation = prepareIfNeeded(requireUpToDateReason = reason, logger = logger) - val resolution = installation - .install(forceUpToDate, args, services, logger) - state = ResolutionState.Installed(resolution) + return try { + val installUpToDate = nodeJsSettings?.npmInstallTaskProvider?.get()?.state?.upToDate ?: false + val forceUpToDate = installUpToDate && !forceFullResolve - installation.closePlugins(resolution) - - return resolution + val installation = prepareIfNeeded(requireUpToDateReason = reason, logger = logger) + val resolution = installation + .install(forceUpToDate, args, services, logger) + state = ResolutionState.Installed(resolution) + installation.closePlugins(resolution) + resolution + } catch (e: Exception) { + state = ResolutionState.Error(e) + throw e + } } } @@ -215,10 +227,12 @@ class KotlinNpmResolutionManager(@Transient private val nodeJsSettings: NodeJsRo } } is ResolutionState.Installed -> error("Project already installed") + is ResolutionState.Error -> throw state1.wrappedException } } } is ResolutionState.Installed -> error("Project already installed") + is ResolutionState.Error -> throw state0.wrappedException } } @@ -230,7 +244,7 @@ class KotlinNpmResolutionManager(@Transient private val nodeJsSettings: NodeJsRo val resolvedProject = if (forceFullResolve) { - installIfNeeded(reason = null, services = services, logger = logger)[projectPath] + installIfNeeded(reason = null, services = services, logger = logger)?.get(projectPath) ?: return null } else { // may return null only during npm resolution // (it can be called since NpmDependency added to configuration that @@ -243,13 +257,16 @@ class KotlinNpmResolutionManager(@Transient private val nodeJsSettings: NodeJsRo //error("Cannot use NpmDependency before :kotlinNpmInstall task execution") } is ResolutionState.Installed -> state0.resolved[projectPath] + is ResolutionState.Error -> { + return null + } } } return resolvedProject.npmProjectsByNpmDependency[npmDependency] ?: error("NPM project resolved without $this") } - internal fun checkRequiredDependencies(task: T, services: ServiceRegistry, logger: Logger, projectPath: String) + internal fun checkRequiredDependencies(task: T) where T : RequiresNpmDependencies, T : Task { val targetRequired = resolver.taskRequirements.byTask[task.path]?.toSet() ?: setOf() diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/resolver/KotlinRootNpmResolver.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/resolver/KotlinRootNpmResolver.kt index 9d11cbdc06f..8a8c5d2d8a8 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/resolver/KotlinRootNpmResolver.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/resolver/KotlinRootNpmResolver.kt @@ -36,7 +36,7 @@ import org.jetbrains.kotlin.gradle.utils.unavailableValueError * Regularly tasks share the same instance of this class, but with configuration cache each task that holds a reference to the instance will * create an own copy. We use build services as a single storage for the heavy state of this class. */ -internal class KotlinRootNpmResolver internal constructor( +class KotlinRootNpmResolver internal constructor( @Transient val nodeJs: NodeJsRootExtension?, val forceFullResolve: Boolean @@ -62,7 +62,7 @@ internal class KotlinRootNpmResolver internal constructor( @Volatile private var state_: RootResolverState? = RootResolverState.CONFIGURING - private var state + var state get() = state_ ?: resolverStateHolder.get().state set(value) { if (state_ != null) { @@ -82,7 +82,7 @@ internal class KotlinRootNpmResolver internal constructor( it.parameters.rootProjectDir.set(rootProject_.projectDir) } - val gradleNodeModules: GradleNodeModulesCache + internal val gradleNodeModules: GradleNodeModulesCache get() = gradleNodeModulesProvider.get().also { it.archiveOperations = archiveOperations it.fs = fs @@ -95,7 +95,7 @@ internal class KotlinRootNpmResolver internal constructor( it.parameters.rootProjectDir.set(rootProject_.projectDir) } - val compositeNodeModules: CompositeNodeModulesCache + internal val compositeNodeModules: CompositeNodeModulesCache get() = compositeNodeModulesProvider.get() @Transient @@ -157,7 +157,7 @@ internal class KotlinRootNpmResolver internal constructor( return projResolvers } - val plugins + internal val plugins get() = plugins_ ?: resolverStateHolder.get().parameters.plugins.get() private val projectResolvers @@ -190,7 +190,7 @@ internal class KotlinRootNpmResolver internal constructor( } } - operator fun get(projectPath: String) = projectResolvers[projectPath] ?: error("$projectPath is not configured for JS usage") + internal operator fun get(projectPath: String) = projectResolvers[projectPath] ?: error("$projectPath is not configured for JS usage") val compilations: Collection get() = projectResolvers.values.flatMap { it.compilationResolvers.map { it.compilation } } @@ -198,7 +198,7 @@ internal class KotlinRootNpmResolver internal constructor( internal fun getPackageJsonHandlers(projectPath: String, compilationDisambiguatedName: String): List Unit> = resolverStateHolder.get().parameters.packageJsonHandlers.get()["$projectPath:$compilationDisambiguatedName"] ?: emptyList() - fun findDependentResolver(src: Project, target: Project): List? { + internal fun findDependentResolver(src: Project, target: Project): List? { // todo: proper finding using KotlinTargetComponent.findUsageContext val targetResolver = this[target.path] val mainCompilations = targetResolver.compilationResolvers.filter { it.compilation.isMain() } @@ -224,6 +224,7 @@ internal class KotlinRootNpmResolver internal constructor( containsIrJs = true } } + else -> { check(!containsLegacyJs) { errorMessage } containsLegacyJs = true @@ -279,6 +280,9 @@ internal class KotlinRootNpmResolver internal constructor( logger: Logger ): KotlinRootNpmResolution { synchronized(projectResolvers) { + if (state == RootResolverState.INSTALLED) { + return KotlinRootNpmResolution(rootProject, projectResolutions) + } check(state == RootResolverState.PROJECTS_CLOSED) { "Projects must be closed" } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/tasks/KotlinNpmInstallTask.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/tasks/KotlinNpmInstallTask.kt index ed073b2abbe..975cbe02d3c 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/tasks/KotlinNpmInstallTask.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/tasks/KotlinNpmInstallTask.kt @@ -9,6 +9,7 @@ import org.gradle.api.DefaultTask import org.gradle.api.tasks.* import org.jetbrains.kotlin.gradle.targets.js.nodejs.NodeJsRootExtension import org.jetbrains.kotlin.gradle.targets.js.nodejs.NodeJsRootPlugin +import org.jetbrains.kotlin.gradle.targets.js.npm.KotlinNpmResolutionManager import org.jetbrains.kotlin.gradle.targets.js.npm.asNpmEnvironment import org.jetbrains.kotlin.gradle.utils.unavailableValueError import java.io.File @@ -70,7 +71,7 @@ open class KotlinNpmInstallTask : DefaultTask() { args = args, services = services, logger = logger - ) + ) ?: throw (resolutionManager.state as KotlinNpmResolutionManager.ResolutionState.Error).wrappedException } companion object { diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/testing/KotlinJsTest.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/testing/KotlinJsTest.kt index 7545a285aa4..2ce3aa19659 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/testing/KotlinJsTest.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/testing/KotlinJsTest.kt @@ -157,7 +157,7 @@ constructor( } override fun executeTests() { - npmResolutionManager.checkRequiredDependencies(task = this, services = services, logger = logger, projectPath = projectPath) + npmResolutionManager.checkRequiredDependencies(task = this) super.executeTests() } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/webpack/KotlinWebpack.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/webpack/KotlinWebpack.kt index d5fa3d9cedf..c300e02c4c9 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/webpack/KotlinWebpack.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/webpack/KotlinWebpack.kt @@ -297,7 +297,7 @@ constructor( @TaskAction fun doExecute() { - resolutionManager.checkRequiredDependencies(task = this, services = services, logger = logger, projectPath = projectPath) + resolutionManager.checkRequiredDependencies(task = this) val runner = createRunner()