From 1103ca3a38f1e84ba45ced9e3421927ffac94e1d Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov Date: Thu, 24 Jan 2019 16:45:05 +0300 Subject: [PATCH] Fix logging in workers Setting `forkMode` to `ForkMode.NEVER` switches `isolationMode` to `IsolationMode.CLASSLOADER`. Classloader isolation mode prevented passing task's logger to the unit of work. Also it might resulted in multiple daemon instances starting in parallel. This change removes setting `forMode` (it was unnecessary). Also a logger is used for the fallback message instead of printing it to stderr. An assertion is added to tests, ensuring that the fallback does not happen. #KT-29394 Fixed --- .../kotlin/gradle/NewMultiplatformIT.kt | 29 ------------ .../org/jetbrains/kotlin/gradle/WorkersIT.kt | 44 +++++++++++++++++++ .../GradleCompilerRunnerWithWorkers.kt | 1 - .../GradleKotlinCompilerWork.kt | 21 +++++++-- 4 files changed, 62 insertions(+), 33 deletions(-) create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/WorkersIT.kt diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt index 7d85b1d1f6a..9b4820d1941 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/NewMultiplatformIT.kt @@ -328,35 +328,6 @@ class NewMultiplatformIT : BaseGradleIT() { } } - @Test - fun testParallelTasks() { - parallelTasksImpl(isParallel = true) - } - - @Test - fun testNoParallelTasks() { - parallelTasksImpl(isParallel = false) - } - - private fun parallelTasksImpl(isParallel: Boolean) = with(Project("new-mpp-parallel", gradleVersion)) { - val options = defaultBuildOptions().copy(parallelTasksInProject = isParallel) - build("assemble", options = options) { - assertSuccessful() - val tasks = arrayOf(":compileKotlinMetadata", ":compileKotlinJvm", ":compileKotlinJs") - if (isParallel) { - assertTasksSubmittedWork(*tasks) - } else { - assertTasksDidNotSubmitWork(*tasks) - } - val expectedKotlinOutputFiles = listOf( - kotlinClassesDir(sourceSet = "metadata/main") + "common/A.kotlin_metadata", - kotlinClassesDir(sourceSet = "jvm/main") + "common/A.class", - kotlinClassesDir(sourceSet = "js/main") + "new-mpp-parallel.js" - ) - expectedKotlinOutputFiles.forEach { assertFileExists(it) } - } - } - @Test fun testLibWithTests() = doTestLibWithTests(Project("new-mpp-lib-with-tests", gradleVersion)) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/WorkersIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/WorkersIT.kt new file mode 100644 index 00000000000..66cce2053c0 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/WorkersIT.kt @@ -0,0 +1,44 @@ +/* + * Copyright 2010-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.gradle + +import org.junit.Test + +class WorkersIT : BaseGradleIT() { + @Test + fun testParallelTasks() { + parallelTasksImpl(isParallel = true) + } + + @Test + fun testNoParallelTasks() { + parallelTasksImpl(isParallel = false) + } + + private fun parallelTasksImpl(isParallel: Boolean) = + with(Project("new-mpp-parallel", GradleVersionRequired.AtLeast("4.7"))) { + val options = defaultBuildOptions().copy(parallelTasksInProject = isParallel, withDaemon = false) + val traceLoading = "-Dorg.jetbrains.kotlin.compilerRunner.GradleKotlinCompilerWork.trace.loading=true" + build("assemble", traceLoading, options = options) { + assertSuccessful() + val tasks = arrayOf(":compileKotlinMetadata", ":compileKotlinJvm", ":compileKotlinJs") + if (isParallel) { + assertTasksSubmittedWork(*tasks) + } else { + assertTasksDidNotSubmitWork(*tasks) + } + val expectedKotlinOutputFiles = listOf( + kotlinClassesDir(sourceSet = "metadata/main") + "common/A.kotlin_metadata", + kotlinClassesDir(sourceSet = "jvm/main") + "common/A.class", + kotlinClassesDir(sourceSet = "js/main") + "new-mpp-parallel.js" + ) + expectedKotlinOutputFiles.forEach { assertFileExists(it) } + assertSubstringCount("Loaded GradleKotlinCompilerWork", 1) + assertCompiledKotlinSources(project.relativize(project.allKotlinFiles)) + assertNotContains("Falling back to sl4j logger") + } + } +} \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleCompilerRunnerWithWorkers.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleCompilerRunnerWithWorkers.kt index fbff69f87ba..b542eb4fc28 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleCompilerRunnerWithWorkers.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleCompilerRunnerWithWorkers.kt @@ -20,7 +20,6 @@ internal class GradleCompilerRunnerWithWorkers( // todo: write tests with Workers enabled; workersExecutor.submit(GradleKotlinCompilerWork::class.java) { config -> config.isolationMode = IsolationMode.NONE - config.forkMode = ForkMode.NEVER config.params(workArgs) } } diff --git a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerWork.kt b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerWork.kt index 6a3bf75177a..9c3848409dc 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerWork.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/compilerRunner/GradleKotlinCompilerWork.kt @@ -66,6 +66,15 @@ internal class GradleKotlinCompilerWork @Inject constructor( */ config: GradleKotlinCompilerWorkArguments ) : Runnable { + + companion object { + init { + if (System.getProperty("org.jetbrains.kotlin.compilerRunner.GradleKotlinCompilerWork.trace.loading") == "true") { + System.out.println("Loaded GradleKotlinCompilerWork") + } + } + } + private val projectRootFile = config.projectFiles.projectRootFile private val clientIsAliveFlagFile = config.projectFiles.clientIsAliveFlagFile private val sessionFlagFile = config.projectFiles.sessionFlagFile @@ -80,10 +89,16 @@ internal class GradleKotlinCompilerWork @Inject constructor( private val taskPath = config.taskPath private val log: KotlinLogger = - TaskLoggers.get(taskPath)?.let { GradleKotlinLogger(it) } + TaskLoggers.get(taskPath)?.let { GradleKotlinLogger(it).apply { debug("Using '$taskPath' logger") } } ?: run { - System.err.println("Could not get logger for '$taskPath'. Falling back to sl4j logger") - SL4JKotlinLogger(LoggerFactory.getLogger("GradleKotlinCompilerWork")) + val logger = LoggerFactory.getLogger("GradleKotlinCompilerWork") + val kotlinLogger = if (logger is org.gradle.api.logging.Logger) { + GradleKotlinLogger(logger) + } else SL4JKotlinLogger(logger) + + kotlinLogger.apply { + debug("Could not get logger for '$taskPath'. Falling back to sl4j logger") + } } private val isIncremental: Boolean