From 3e6253cedef703c4341bdc26d8ad2f5f8683a669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martynas=20Petu=C5=A1ka?= Date: Wed, 29 Jun 2022 12:52:30 +0100 Subject: [PATCH] KT-45789 Transitive npm dependencies --- .../gradle/KotlinJsLibraryGradlePluginIT.kt | 13 ++++++- .../kotlin/gradle/testbase/testDsl.kt | 7 ++-- .../build.gradle.kts | 2 +- .../simple-js-library/build.gradle.kts | 10 ++++- .../gradle/targets/js/npm/GradleNodeModule.kt | 25 ++++++++++++- .../resolver/KotlinCompilationNpmResolver.kt | 37 +++++++++++++++++-- 6 files changed, 82 insertions(+), 12 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinJsLibraryGradlePluginIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinJsLibraryGradlePluginIT.kt index c1741de826a..4076f202f18 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinJsLibraryGradlePluginIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinJsLibraryGradlePluginIT.kt @@ -5,11 +5,14 @@ package org.jetbrains.kotlin.gradle +import com.google.gson.Gson +import com.google.gson.JsonObject import org.gradle.util.GradleVersion import org.jetbrains.kotlin.gradle.plugin.KotlinJsCompilerType import org.jetbrains.kotlin.gradle.testbase.* import org.junit.jupiter.api.DisplayName import java.util.zip.ZipFile +import kotlin.io.path.reader import kotlin.test.assertNotNull // TODO: This suite is failing with deprecation error on Gradle <7.0 versions @@ -34,6 +37,14 @@ class KotlinJsIrLibraryGradlePluginIT : KGPBaseTest() { assertFileInProjectExists("build/productionLibrary/js-library.js") assertFileInProjectExists("build/productionLibrary/package.json") assertFileInProjectExists("build/productionLibrary/main.js") + projectPath.resolve("build/productionLibrary/package.json").reader() + .use { Gson().fromJson(it, JsonObject::class.java) } + .getAsJsonObject("dependencies") + ?.entrySet()?.associate { (k, v) -> k to v.asString } + .let { dependencies -> + assertNotNull(dependencies?.get("kotlin")) { "Direct npm dependency missing in package.json" } + assertNotNull(dependencies?.get("@js-joda/core")) { "Transitive npm dependency missing in package.json" } + } } } } @@ -77,4 +88,4 @@ class KotlinJsIrLibraryGradlePluginIT : KGPBaseTest() { } } } -} \ No newline at end of file +} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/testbase/testDsl.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/testbase/testDsl.kt index e8debe2bb3d..91add3fe7f2 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/testbase/testDsl.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/testbase/testDsl.kt @@ -74,12 +74,11 @@ fun KGPBaseTest.project( localRepoDir?.let { testProject.configureLocalRepository(localRepoDir) } if (buildJdk != null) testProject.setupNonDefaultJdk(buildJdk) - runCatching { + val result = runCatching { testProject.test() - }.onFailure { - // A convenient place to place a breakpoint to be able to inspect project output files - throw it } + // A convenient place to place a breakpoint to be able to inspect project output files + result.getOrThrow() return testProject } diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/js-library-with-executable/build.gradle.kts b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/js-library-with-executable/build.gradle.kts index f13eb65e819..93aa23cffc3 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/js-library-with-executable/build.gradle.kts +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/js-library-with-executable/build.gradle.kts @@ -24,4 +24,4 @@ kotlin { // We need to think about it, when we will support multiple binaries tasks.named("nodeProductionLibraryPrepare") { mustRunAfter("productionExecutableCompileSync") -} \ No newline at end of file +} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/simple-js-library/build.gradle.kts b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/simple-js-library/build.gradle.kts index 2a0fa5cb0b7..479fe62e535 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/simple-js-library/build.gradle.kts +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/simple-js-library/build.gradle.kts @@ -15,4 +15,12 @@ kotlin { binaries.library() nodejs() } -} \ No newline at end of file + sourceSets { + main { + dependencies { + implementation("org.jetbrains.kotlinx:kotlinx-datetime:latest.release") + implementation(npm("kotlin", "*")) + } + } + } +} diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/GradleNodeModule.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/GradleNodeModule.kt index b38cce8bcee..1ed99774e30 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/GradleNodeModule.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/GradleNodeModule.kt @@ -5,6 +5,8 @@ package org.jetbrains.kotlin.gradle.targets.js.npm +import com.google.gson.Gson +import com.google.gson.JsonObject import java.io.File import java.io.Serializable @@ -14,4 +16,25 @@ import java.io.Serializable data class GradleNodeModule(val name: String, val version: String, val path: File) : Serializable { val semver: SemVer get() = SemVer.from(version) -} \ No newline at end of file + + @get:Synchronized + val dependencies: Set by lazy { + val pJson = path.resolve("package.json").reader().use { + Gson().fromJson(it, JsonObject::class.java) + } + val normal = pJson.getAsJsonObject("dependencies") + val peer = pJson.getAsJsonObject("peerDependencies") + val optional = pJson.getAsJsonObject("optionalDependencies") + val dev = pJson.getAsJsonObject("devDependencies") + mapOf( + NpmDependency.Scope.NORMAL to normal, + NpmDependency.Scope.PEER to peer, + NpmDependency.Scope.OPTIONAL to optional, + NpmDependency.Scope.DEV to dev + ).mapValues { (_, deps) -> + deps?.entrySet()?.associate { (k, v) -> k to v.asString } + }.mapNotNull { (scope, deps) -> + deps?.map { (k, v) -> NpmDependencyDeclaration(scope, k, v, false) } + }.flatten().toSet() + } +} diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/resolver/KotlinCompilationNpmResolver.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/resolver/KotlinCompilationNpmResolver.kt index 567e4a06dea..2ae344b7d12 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/resolver/KotlinCompilationNpmResolver.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/js/npm/resolver/KotlinCompilationNpmResolver.kt @@ -446,6 +446,9 @@ internal class KotlinCompilationNpmResolver( ) } }.filterNotNull() + val transitiveNpmDependencies = importedExternalGradleDependencies.flatMap { + it.dependencies + }.filter { it.scope != NpmDependency.Scope.DEV } val compositeDependencies = internalCompositeDependencies.flatMap { dependency -> dependency.getPackages() @@ -456,8 +459,7 @@ internal class KotlinCompilationNpmResolver( file ) } - } - .filterNotNull() + }.filterNotNull() val toolsNpmDependencies = compilationResolver.rootResolver.taskRequirements .getCompilationNpmRequirements(projectPath, compilationResolver.compilationDisambiguatedName) @@ -473,7 +475,8 @@ internal class KotlinCompilationNpmResolver( ) } else emptySet() - val allNpmDependencies = externalNpmDependencies + toolsNpmDependencies + dukatIfNecessary + val otherNpmDependencies = toolsNpmDependencies + dukatIfNecessary + transitiveNpmDependencies + val allNpmDependencies = disambiguateDependencies(externalNpmDependencies, otherNpmDependencies) val packageJsonHandlers = if (compilationResolver.compilation != null) { compilationResolver.compilation.packageJsonHandlers } else { @@ -511,6 +514,32 @@ internal class KotlinCompilationNpmResolver( ) } + private fun disambiguateDependencies( + direct: Collection, + others: Collection, + ): Collection { + val unique = others.groupBy(NpmDependencyDeclaration::name) + .filterKeys { k -> direct.none { it.name == k } } + .mapNotNull { (name, dependencies) -> + dependencies.maxByOrNull { dep -> + SemVer.from(dep.version, true) + }?.also { selected -> + if (dependencies.size > 1) { + compilationResolver.project.logger.warn( + """ + Transitive npm dependency version clash for compilation "${compilationResolver.compilation.name}" + Candidates: + ${dependencies.joinToString("\n") { "\t\t" + it.name + "@" + it.version }} + Selected: + ${selected.name}@${selected.version} + """.trimIndent() + ) + } + } + } + return direct + unique + } + private fun CompositeDependency.getPackages(): List { val packages = includedBuildDir.resolve(projectPackagesDir.relativeTo(rootDir)) return packages @@ -520,4 +549,4 @@ internal class KotlinCompilationNpmResolver( ?: emptyList() } } -} \ No newline at end of file +}