From 069941cdaf6d99a27939e266223d0b1bbe5cc359 Mon Sep 17 00:00:00 2001 From: Yahor Berdnikau Date: Thu, 11 Feb 2021 14:02:32 +0100 Subject: [PATCH] Provide compiler classpath as task input. This ensures that compiler classpath is what is expected by Kotlin Plugin and removes possibility of leaking wrong jars from Gradle wrapper classpath. For 'kotlin.useFallbackCompilerSearch' old behaviour is still present, but this option should be marked as deprecated and removed in one of the Kotlin releases. --- .../jetbrains/kotlin/gradle/KotlinDaemonIT.kt | 40 +++++++++++++++++ .../gradle/plugin/KotlinPluginWrapper.kt | 28 +++++++++--- .../gradle/targets/js/KotlinJsDcePlugin.kt | 1 + .../targets/js/subtargets/KotlinBrowserJs.kt | 2 + .../jetbrains/kotlin/gradle/tasks/Tasks.kt | 43 +++++++++++-------- 5 files changed, 89 insertions(+), 25 deletions(-) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinDaemonIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinDaemonIT.kt index e01016439ff..51d1375cb2a 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinDaemonIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/KotlinDaemonIT.kt @@ -20,6 +20,7 @@ import org.jetbrains.kotlin.compilerRunner.* import org.junit.Assert import org.junit.Test import java.io.File +import kotlin.test.assertTrue // todo: test client file creation/deletion // todo: test daemon start (does not start every build) @@ -91,4 +92,43 @@ class KotlinDaemonIT : BaseGradleIT() { } } } + + @Test + fun testGradleBuildClasspathShouldNotBeLeakedIntoDaemonClasspath() { + val testProject = Project("kotlinProject") + testProject.setupWorkingDir() + + testProject.build("assemble") { + assertGradleClasspathNotLeaked() + } + } + + @Test + fun testGradleBuildClasspathShouldNotBeLeakedIntoDaemonClasspathOnUsingCustomCompilerJar() { + val testProject = Project( + projectName = "customCompilerFile", + ) + testProject.setupWorkingDir() + + // copy compiler embeddable into project dir using custom name + val classpath = System.getProperty("java.class.path").split(File.pathSeparator) + val kotlinEmbeddableJar = File(classpath.find { it.contains("kotlin-compiler-embeddable") }!!) + + val compilerJar = File(testProject.projectDir, "compiler.jar") + kotlinEmbeddableJar.copyTo(compilerJar) + + testProject.build("build") { + assertGradleClasspathNotLeaked() + } + } + + private fun CompiledProject.assertGradleClasspathNotLeaked() { + assertContains("Kotlin compiler classpath:") + val daemonClasspath = output.lineSequence().find { + it.contains("Kotlin compiler classpath:") + }!! + assertTrue("Daemon classpath contains embeddable daemon jar leaked from Gradle dist classpath: $daemonClasspath") { + !daemonClasspath.contains(".gradle/wrapper/dists") + } + } } \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginWrapper.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginWrapper.kt index 0a5f0cf2420..11549412937 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginWrapper.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinPluginWrapper.kt @@ -21,10 +21,8 @@ import org.gradle.api.NamedDomainObjectFactory import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.internal.FeaturePreviews -import org.gradle.api.internal.file.FileResolver import org.gradle.api.logging.Logger import org.gradle.api.logging.Logging -import org.gradle.build.event.BuildEventsListenerRegistry import org.gradle.tooling.provider.model.ToolingModelBuilderRegistry import org.jetbrains.kotlin.gradle.dsl.* import org.jetbrains.kotlin.gradle.logging.kotlinDebug @@ -36,10 +34,10 @@ import org.jetbrains.kotlin.gradle.plugin.statistics.KotlinBuildStatsService import org.jetbrains.kotlin.gradle.targets.js.KotlinJsCompilerAttribute import org.jetbrains.kotlin.gradle.targets.js.KotlinJsPlugin import org.jetbrains.kotlin.gradle.targets.js.npm.addNpmDependencyExtension +import org.jetbrains.kotlin.gradle.tasks.* import org.jetbrains.kotlin.gradle.tasks.KOTLIN_COMPILER_EMBEDDABLE import org.jetbrains.kotlin.gradle.tasks.KOTLIN_KLIB_COMMONIZER_EMBEDDABLE import org.jetbrains.kotlin.gradle.tasks.KOTLIN_MODULE_GROUP -import org.jetbrains.kotlin.gradle.tasks.throwGradleExceptionIfError import org.jetbrains.kotlin.gradle.testing.internal.KotlinTestsRegistry import org.jetbrains.kotlin.gradle.utils.checkGradleCompatibility import org.jetbrains.kotlin.gradle.utils.loadPropertyFromResources @@ -68,9 +66,8 @@ abstract class KotlinBasePluginWrapper : Plugin { whenBuildEvaluated(project) } - project.configurations.maybeCreate(COMPILER_CLASSPATH_CONFIGURATION_NAME).defaultDependencies { - it.add(project.dependencies.create("$KOTLIN_MODULE_GROUP:$KOTLIN_COMPILER_EMBEDDABLE:$kotlinPluginVersion")) - } + addKotlinCompilerConfiguration(project) + project.configurations.maybeCreate(PLUGIN_CLASSPATH_CONFIGURATION_NAME) project.configurations.maybeCreate(NATIVE_COMPILER_PLUGIN_CLASSPATH_CONFIGURATION_NAME).apply { isTransitive = false @@ -106,6 +103,25 @@ abstract class KotlinBasePluginWrapper : Plugin { project.addNpmDependencyExtension() } + private fun addKotlinCompilerConfiguration(project: Project) { + project + .configurations + .maybeCreate(COMPILER_CLASSPATH_CONFIGURATION_NAME) + .defaultDependencies { + it.add( + project.dependencies.create("$KOTLIN_MODULE_GROUP:$KOTLIN_COMPILER_EMBEDDABLE:$kotlinPluginVersion") + ) + } + project + .tasks + .withType(AbstractKotlinCompile::class.java) + .configureEach { task -> + task.defaultCompilerClasspath.setFrom( + project.configurations.named(COMPILER_CLASSPATH_CONFIGURATION_NAME) + ) + } + } + open fun whenBuildEvaluated(project: Project) { } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/KotlinJsDcePlugin.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/KotlinJsDcePlugin.kt index b5d537b0eaa..7bcda618c80 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/KotlinJsDcePlugin.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/KotlinJsDcePlugin.kt @@ -64,6 +64,7 @@ class KotlinJsDcePlugin : Plugin { val dceTask = project.registerTask(dceTaskName) { it.dependsOn(kotlinTask) + it.defaultCompilerClasspath.setFrom(project.configurations.named(COMPILER_CLASSPATH_CONFIGURATION_NAME)) } project.tasks.named("build").dependsOn(dceTask) diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/subtargets/KotlinBrowserJs.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/subtargets/KotlinBrowserJs.kt index 34dff65c12b..e93d5cdf14f 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/subtargets/KotlinBrowserJs.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/targets/js/subtargets/KotlinBrowserJs.kt @@ -10,6 +10,7 @@ import org.gradle.api.tasks.Copy import org.gradle.api.tasks.TaskProvider import org.gradle.language.base.plugins.LifecycleBasePlugin import org.jetbrains.kotlin.gradle.dsl.KotlinJsDce +import org.jetbrains.kotlin.gradle.plugin.COMPILER_CLASSPATH_CONFIGURATION_NAME import org.jetbrains.kotlin.gradle.plugin.KotlinCompilation import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinJsCompilation import org.jetbrains.kotlin.gradle.targets.js.KotlinJsTarget @@ -287,6 +288,7 @@ open class KotlinBrowserJs @Inject constructor(target: KotlinJsTarget) : it.classpath = project.configurations.getByName(compilation.runtimeDependencyConfigurationName) it.destinationDir = it.dceOptions.outputDirectory?.let { File(it) } ?: compilation.npmProject.dir.resolve(if (dev) DCE_DEV_DIR else DCE_DIR) + it.defaultCompilerClasspath.setFrom(project.configurations.named(COMPILER_CLASSPATH_CONFIGURATION_NAME)) it.source(kotlinTask.map { it.outputFile }) } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt index 9509274f7e0..4564dede92e 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/tasks/Tasks.kt @@ -7,6 +7,7 @@ package org.jetbrains.kotlin.gradle.tasks import org.gradle.api.Project import org.gradle.api.Task +import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.FileCollection import org.gradle.api.logging.Logger import org.gradle.api.provider.Provider @@ -98,28 +99,32 @@ abstract class AbstractKotlinCompileTool override val metrics: BuildMetricsReporter = BuildMetricsReporterImpl() + /** + * By default should be set by plugin from [COMPILER_CLASSPATH_CONFIGURATION_NAME] configuration. + * + * Empty classpath will fail the build. + */ + @get:Classpath + internal val defaultCompilerClasspath: ConfigurableFileCollection = + project.objects.fileCollection() + @get:Classpath @get:InputFiles internal val computedCompilerClasspath: List by lazy { - compilerClasspath?.takeIf { it.isNotEmpty() } - ?: compilerJarFile?.let { - // a hack to remove compiler jar from the cp, will be dropped when compilerJarFile will be removed - listOf(it) + findKotlinCompilerClasspath(project).filter { !it.name.startsWith("kotlin-compiler") } - } - ?: if (!useFallbackCompilerSearch) { - try { - project.configurations.getByName(COMPILER_CLASSPATH_CONFIGURATION_NAME).resolve().toList() - } catch (e: Exception) { - logger.error( - "Could not resolve compiler classpath. " + - "Check if Kotlin Gradle plugin repository is configured in $project." - ) - throw e - } - } else { - findKotlinCompilerClasspath(project) - } - ?: throw IllegalStateException("Could not find Kotlin Compiler classpath") + require(!defaultCompilerClasspath.isEmpty) { + "Default Kotlin compiler classpath is empty! Task: ${this::class.qualifiedName}" + } + + when { + !compilerClasspath.isNullOrEmpty() -> compilerClasspath!! + compilerJarFile != null -> listOf(compilerJarFile!!) + + defaultCompilerClasspath + .filterNot { + it.nameWithoutExtension.startsWith("kotlin-compiler") + } + useFallbackCompilerSearch -> findKotlinCompilerClasspath(project) + else -> defaultCompilerClasspath.toList() + } }