From de4953adf6bafd7ae129938f4e1342e277e67890 Mon Sep 17 00:00:00 2001 From: Evgenii Mazhukin Date: Mon, 8 Jan 2024 20:03:08 +0000 Subject: [PATCH] [IC] Baseline fix for common sources getting access to platform declarations K2-only issue. In an incremental build, sourceSet boundary isn't preserved in certain conditions: SourceSet A depends on SourceSet B A and B overload a function, for example: A -> fun foo(p: Parent) // (1) B -> fun foo(c: Child) // (2) If some source file in A is calling foo(..), only (1) is supposed to be visible for that call site. However, if a) it is an incremental build, b) the declaration of (1) is not a part of the compilation set, and c) call to foo(c: Child) is applicable, then (2) would be called from the generated code. So, the build result is not consistent between the full build and an incremental build. As a workaround, we fallback to a non-incremental build, if any source from A needs to be compiled. To enable "risky" incremental builds, use Gradle property kotlin.internal.incremental.enableUnsafeOptimizationsForMultiplatform=true ^KT-62686 ^KT-63837 Fixed Merge-request: KT-MR-13695 Merged-by: Evgenii Mazhukin --- .../kotlin/incremental/FragmentContext.kt | 74 +++++++++ .../IncrementalCompilationContext.kt | 1 + .../IncrementalCompilationFeatures.kt | 12 +- ...uireRebuildForCorrectnessInKMPException.kt | 14 ++ .../build/report/metrics/BuildAttribute.kt | 5 +- .../incremental/IncrementalCompilerRunner.kt | 31 +++- .../mpp/CommonCodeWithPlatformSymbolsIT.kt | 156 ++++++++++++++++++ .../kotlin/gradle/mpp/KmpIncrementalITBase.kt | 7 +- .../ExpectActualIncrementalCompilationIT.kt | 5 +- .../kotlin/gradle/testbase/BuildOptions.kt | 5 + .../build.gradle.kts | 18 ++ .../kotlin/dependedOnByRiskyCode.kt | 1 + .../dependedOnByRiskyCode.kt.changeType | 1 + .../src/commonMain/kotlin/riskyCode.kt | 6 + ...kyCode.kt.useMemberFunctionFromExpectClass | 6 + .../kotlin/riskyCode.kt.useTopLevelFunction | 6 + ...eseDeclarationsShouldBeUsedInCommonCode.kt | 9 + .../actualClassAndFunWithExtraOverloads.kt | 18 ++ .../kotlin/dependedOnByActualDeclaration.kt | 1 + ...ependedOnByActualDeclaration.kt.changeType | 1 + .../actualClassAndFunWithExtraOverloads.kt | 18 ++ .../kotlin/dependedOnByActualDeclaration.kt | 1 + ...ependedOnByActualDeclaration.kt.changeType | 1 + .../gradle/plugin/PropertiesProvider.kt | 8 + .../gradle/tasks/AbstractKotlinCompile.kt | 5 + .../AbstractKotlinCompileConfig.kt | 3 + 26 files changed, 401 insertions(+), 12 deletions(-) create mode 100644 build-common/src/org/jetbrains/kotlin/incremental/FragmentContext.kt create mode 100644 build-common/src/org/jetbrains/kotlin/incremental/RequireRebuildForCorrectnessInKMPException.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/CommonCodeWithPlatformSymbolsIT.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/build.gradle.kts create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/dependedOnByRiskyCode.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/dependedOnByRiskyCode.kt.changeType create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt.useMemberFunctionFromExpectClass create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt.useTopLevelFunction create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/theseDeclarationsShouldBeUsedInCommonCode.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/actualClassAndFunWithExtraOverloads.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/dependedOnByActualDeclaration.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/dependedOnByActualDeclaration.kt.changeType create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/actualClassAndFunWithExtraOverloads.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/dependedOnByActualDeclaration.kt create mode 100644 libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/dependedOnByActualDeclaration.kt.changeType diff --git a/build-common/src/org/jetbrains/kotlin/incremental/FragmentContext.kt b/build-common/src/org/jetbrains/kotlin/incremental/FragmentContext.kt new file mode 100644 index 00000000000..09f9e35c705 --- /dev/null +++ b/build-common/src/org/jetbrains/kotlin/incremental/FragmentContext.kt @@ -0,0 +1,74 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * 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.incremental + +import org.jetbrains.kotlin.cli.common.arguments.CommonCompilerArguments +import org.jetbrains.kotlin.config.LanguageVersion +import java.io.File + +/** + * Holds current mapping from source files to fragments, in the K2 KMP context + * + * Note: for KT-62686, we don't really care about removed files: if a source isn't recompiled, + * it can't get illegal lookups. + * + * So there's no need to store previous source-to-fragment mapping + */ +class FragmentContext( + /** + * Map from path to source to this source's fragment + */ + private val fileToFragment: Map, + /** + * If a fragment isn't refined by any other fragments, it's allowed to have incremental compilation. + * Otherwise, issues from KT-62686 are applicable. + */ + private val leafFragments: Set +) { + /** + * Returns true, if any file from dirtySet is a part of refined fragment (for example: common, apple, linux). + * It is relevant to KT-62686, because in k2 "refined" fragments aren't supposed to see symbols from "refining" fragments, + * but they do. + * + * Use of `absolutePath` is coordinated with K2MultiplatformStructure.fragmentSourcesCompilerArgs + */ + fun dirtySetTouchesNonLeafFragments(dirtySet: Iterable): Boolean { + return dirtySet.any { file -> + !leafFragments.contains(fileToFragment[file.absolutePath]) + } + } + + companion object { + private fun canCreateFragmentContext(args: CommonCompilerArguments): Boolean { + // fragmentContext solves k2-only issues; also, in k1 mode we don't even expect the fragment-related args + val isCorrectLanguageVersion = (LanguageVersion.fromVersionString(args.languageVersion) ?: LanguageVersion.LATEST_STABLE).usesK2 + + val hasAllRequiredArguments = listOf(args.fragments, args.fragmentRefines, args.fragmentSources).none { + it.isNullOrEmpty() + } + return isCorrectLanguageVersion && hasAllRequiredArguments + } + + fun fromCompilerArguments(args: CommonCompilerArguments): FragmentContext? { + if (!canCreateFragmentContext(args)) { + return null + } + + val fileToFragment = args.fragmentSources!!.associate { + // expected format: -Xfragment-sources=jvmMain:/tmp/<..>/kotlin/main.kt,<...> + val fragmentSource = it.split(":", limit=2) + Pair(fragmentSource.last(), fragmentSource.first()) + } + val refinedFragments = args.fragmentRefines!!.map { fragmentRefine -> + // expected format: -Xfragment-refines=jvmMain:commonMain,<...> + fragmentRefine.split(":", limit = 2).last() + }.toSet() + val leafFragments = args.fragments!!.toSet() - refinedFragments + + return FragmentContext(fileToFragment, leafFragments) + } + } +} \ No newline at end of file diff --git a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationContext.kt b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationContext.kt index a493697957c..882859211f1 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationContext.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationContext.kt @@ -24,6 +24,7 @@ class IncrementalCompilationContext( */ val trackChangesInLookupCache: Boolean = false, val icFeatures: IncrementalCompilationFeatures = IncrementalCompilationFeatures.DEFAULT_CONFIGURATION, + val fragmentContext: FragmentContext? = null, ) { @Deprecated("This constructor is scheduled to be removed. KSP is using it") constructor( diff --git a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationFeatures.kt b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationFeatures.kt index a5059cebd02..9d366019816 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationFeatures.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalCompilationFeatures.kt @@ -19,7 +19,7 @@ import java.io.Serializable * * To add a new feature toggle, consider this checklist: * - * 1. Gradle input: see [AbstractKotlinCompile.getIncrementalCompilationFeatures]. + * 1. Gradle input: see [AbstractKotlinCompile.makeIncrementalCompilationFeatures]. * Platform-agnostic properties should be the default, but you can extend one of the subclasses, if needed. * If new property might affect the result of compilation, annotate it with @Input. * 2. Gradle.properties support: declare the property in [org.jetbrains.kotlin.gradle.plugin.PropertiesProvider]. @@ -48,11 +48,17 @@ data class IncrementalCompilationFeatures( * Requires [preciseCompilationResultsBackup] */ val keepIncrementalCompilationCachesInMemory: Boolean = false, + /** + * By default, with k2 KMP, we recompile the whole module, if any common sources are recompiled. + * It provides consistent builds at the cost of compilation speed. (See KT-62686 for the underlying issue.) + * You can enable "unsafeIC" to use pre-2.0 behavior with potentially incorrect incremental builds. + */ + val enableUnsafeIncrementalCompilationForMultiplatform: Boolean = false, ) : Serializable { companion object { val DEFAULT_CONFIGURATION = IncrementalCompilationFeatures() - const val serialVersionUID: Long = 0 + const val serialVersionUID: Long = 1 } -} \ No newline at end of file +} diff --git a/build-common/src/org/jetbrains/kotlin/incremental/RequireRebuildForCorrectnessInKMPException.kt b/build-common/src/org/jetbrains/kotlin/incremental/RequireRebuildForCorrectnessInKMPException.kt new file mode 100644 index 00000000000..1ecdad11079 --- /dev/null +++ b/build-common/src/org/jetbrains/kotlin/incremental/RequireRebuildForCorrectnessInKMPException.kt @@ -0,0 +1,14 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * 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.incremental + +/** + * Normally we can RequireRebuild based on input's change, + * but the decision can also come later, between incremental compilation steps. + * + * This exception is used for KT-63837 where common sources need to be rebuilt for consistency. + */ +class RequireRebuildForCorrectnessInKMPException() : Exception() diff --git a/compiler/build-tools/kotlin-build-statistics/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt b/compiler/build-tools/kotlin-build-statistics/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt index 6473e8d7adc..fe5efde5d0e 100644 --- a/compiler/build-tools/kotlin-build-statistics/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt +++ b/compiler/build-tools/kotlin-build-statistics/src/org/jetbrains/kotlin/build/report/metrics/BuildAttribute.kt @@ -36,9 +36,10 @@ enum class BuildAttribute(val kind: BuildAttributeKind, val readableString: Stri DEP_CHANGE_NON_INCREMENTAL_BUILD_IN_DEP(BuildAttributeKind.REBUILD_REASON, "Non incremental build in history"), IN_PROCESS_EXECUTION(BuildAttributeKind.REBUILD_REASON, "In-process execution"), OUT_OF_PROCESS_EXECUTION(BuildAttributeKind.REBUILD_REASON, "Out of process execution"), - IC_IS_NOT_ENABLED(BuildAttributeKind.REBUILD_REASON, "Incremental compilation is not enabled"); + IC_IS_NOT_ENABLED(BuildAttributeKind.REBUILD_REASON, "Incremental compilation is not enabled"), + UNSAFE_INCREMENTAL_CHANGE_KT_62686(BuildAttributeKind.REBUILD_REASON, "Incremental compilation might be incorrect (KT-62686)"); companion object { - const val serialVersionUID = 0L + const val serialVersionUID = 1L } } diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt index b18e9c4f155..3213b3d4d8d 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalCompilerRunner.kt @@ -91,6 +91,7 @@ abstract class IncrementalCompilerRunner< private fun createIncrementalCompilationContext( fileLocations: FileLocations?, transaction: CompilationTransaction, + fragmentContext: FragmentContext? = null, ) = IncrementalCompilationContext( pathConverterForSourceFiles = fileLocations?.let { it.getRelocatablePathConverterForSourceFiles() } ?: BasicFileToPathConverter, pathConverterForOutputFiles = fileLocations?.let { it.getRelocatablePathConverterForOutputFiles() } ?: BasicFileToPathConverter, @@ -99,6 +100,7 @@ abstract class IncrementalCompilerRunner< trackChangesInLookupCache = shouldTrackChangesInLookupCache, storeFullFqNamesInLookupCache = shouldStoreFullFqNamesInLookupCache, icFeatures = icFeatures, + fragmentContext = fragmentContext, ) protected abstract val shouldTrackChangesInLookupCache: Boolean @@ -165,6 +167,9 @@ abstract class IncrementalCompilerRunner< class Failed(val reason: BuildAttribute, val cause: Throwable) : ICResult } + // Be aware that [tryCompileIncrementally] catches a lot of exceptions internally. + // So this transformer should be used for very specific things, like cache closing, that are + // related to the transaction as a whole rather than any compilation step. private fun incrementalCompilationExceptionTransformer(t: Throwable): ICResult = when (t) { is CachesManagerCloseException -> ICResult.Failed(IC_FAILED_TO_CLOSE_CACHES, t) else -> throw t @@ -186,10 +191,19 @@ abstract class IncrementalCompilerRunner< if (changedFiles is ChangedFiles.Unknown) { return ICResult.RequiresRebuild(UNKNOWN_CHANGES_IN_GRADLE_INPUTS) } - changedFiles as ChangedFiles.Known? + + val fragmentContext = if (!icFeatures.enableUnsafeIncrementalCompilationForMultiplatform) { //see KT-62686 + FragmentContext.fromCompilerArguments(args) + } else { + null + } return createTransaction().runWithin(::incrementalCompilationExceptionTransformer) { transaction -> - val icContext = createIncrementalCompilationContext(fileLocations, transaction) + val icContext = createIncrementalCompilationContext( + fileLocations, + transaction, + fragmentContext + ) val caches = createCacheManager(icContext, args).also { // this way we make the transaction to be responsible for closing the caches manager transaction.cachesManager = it @@ -198,7 +212,7 @@ abstract class IncrementalCompilerRunner< fun compile(): ICResult { // Step 1: Get changed files val knownChangedFiles: ChangedFiles.Known = try { - getChangedFiles(changedFiles, allSourceFiles, caches) + getChangedFiles(changedFiles as ChangedFiles.Known?, allSourceFiles, caches) } catch (e: Throwable) { return ICResult.Failed(IC_FAILED_TO_GET_CHANGED_FILES, e) } @@ -241,6 +255,8 @@ abstract class IncrementalCompilerRunner< abiSnapshotData, messageCollector, ) + } catch (e: RequireRebuildForCorrectnessInKMPException) { + return ICResult.RequiresRebuild(UNSAFE_INCREMENTAL_CHANGE_KT_62686) } catch (e: Throwable) { return ICResult.Failed(IC_FAILED_TO_COMPILE_INCREMENTALLY, e) } @@ -463,9 +479,14 @@ abstract class IncrementalCompilerRunner< val lookupTracker = LookupTrackerImpl(getLookupTrackerDelegate()) val expectActualTracker = ExpectActualTrackerImpl() - //TODO(valtman) sourceToCompile calculate based on abiSnapshot val (sourcesToCompile, removedKotlinSources) = dirtySources.partition { it.exists() && allKotlinSources.contains(it) } + icContext.fragmentContext?.let { + if (it.dirtySetTouchesNonLeafFragments(sourcesToCompile)) { + throw RequireRebuildForCorrectnessInKMPException() + } + } + val services = makeServices( args, lookupTracker, expectActualTracker, caches, dirtySources.toSet(), compilationMode is CompilationMode.Incremental @@ -686,4 +707,4 @@ fun extractKotlinSourcesFromFreeCompilerArguments( } compilerArguments.freeArgs = freeArgs return allKotlinFiles -} \ No newline at end of file +} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/CommonCodeWithPlatformSymbolsIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/CommonCodeWithPlatformSymbolsIT.kt new file mode 100644 index 00000000000..fa9c305a572 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/CommonCodeWithPlatformSymbolsIT.kt @@ -0,0 +1,156 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * 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.mpp + +import org.gradle.api.logging.LogLevel +import org.gradle.util.GradleVersion +import org.jetbrains.kotlin.build.report.metrics.BuildAttribute +import org.jetbrains.kotlin.gradle.testbase.* +import org.jetbrains.kotlin.gradle.util.replaceWithVersion +import org.jetbrains.kotlin.test.TestMetadata +import org.junit.jupiter.api.DisplayName + +/** + * Normal behavior is tested by smokes: [org.jetbrains.kotlin.gradle.mpp.smoke.MultiModuleIncrementalCompilationIT] + * + * [CommonCodeWithPlatformSymbolsIT] should be removed with the IC option when the underlying compiler issue is fixed + */ + +open class CommonCodeWithPlatformSymbolsITBase(val platformSourceSet: String, val taskToExecute: String) : KGPBaseTest() { + override val defaultBuildOptions: BuildOptions + get() = super.defaultBuildOptions.copy( + logLevel = LogLevel.DEBUG, + languageVersion = "2.0", + enableUnsafeIncrementalCompilationForMultiplatform = false + ) + + @GradleTest + @DisplayName("Baseline - compilation failure without hotfix") + @TestMetadata("kt-62686-mpp-source-set-boundary") + fun testBaselineForCommonSourceSetIC(gradleVersion: GradleVersion) { + project( + "kt-62686-mpp-source-set-boundary", + gradleVersion, + buildOptions = defaultBuildOptions.copy(enableUnsafeIncrementalCompilationForMultiplatform = true) + ) { + // initial build is good + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + } + + projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useMemberFunctionFromExpectClass") + + // common source file is recompiled, and the incorrect overload resolution happens: + buildAndFail(taskToExecute) { + assertTasksFailed(taskToExecute) + assertOutputContains("Return type mismatch: expected 'kotlin.String', actual '${platformSourceSet}OnlyType'.") + } + + // but changed code is valid for full compilation: + build("clean", taskToExecute) { + assertTasksExecuted(taskToExecute) + } + } + } + + @GradleTest + @DisplayName("Use member function of an expect class that is overloaded in the actual class") + @TestMetadata("kt-62686-mpp-source-set-boundary") + fun testIncrementalBuildWithOverloadedMemberFunction(gradleVersion: GradleVersion) { + project( + "kt-62686-mpp-source-set-boundary", gradleVersion + ) { + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + } + + projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useMemberFunctionFromExpectClass") + + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + assertNonIncrementalCompilation(BuildAttribute.UNSAFE_INCREMENTAL_CHANGE_KT_62686) + } + } + } + + @GradleTest + @DisplayName("Use top level function that is overloaded in platform sourceSet") + @TestMetadata("kt-62686-mpp-source-set-boundary") + fun testIncrementalBuildWithOverloadedTopLevelFunction(gradleVersion: GradleVersion) { + project( + "kt-62686-mpp-source-set-boundary", gradleVersion + ) { + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + } + + projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useTopLevelFunction") + + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + assertNonIncrementalCompilation(BuildAttribute.UNSAFE_INCREMENTAL_CHANGE_KT_62686) + } + } + } + + @GradleTest + @DisplayName("Touch risky code by change in common sourceSet") + @TestMetadata("kt-62686-mpp-source-set-boundary") + fun testIncrementalBuildWhereRiskyCodeIsTouchedIndirectly(gradleVersion: GradleVersion) { + project( + "kt-62686-mpp-source-set-boundary", gradleVersion + ) { + projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useMemberFunctionFromExpectClass") + + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + } + + projectPath.resolve("src/commonMain/kotlin/dependedOnByRiskyCode.kt") + .replaceWithVersion("changeType") + + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + assertNonIncrementalCompilation(BuildAttribute.UNSAFE_INCREMENTAL_CHANGE_KT_62686) + } + } + } + + @GradleTest + @DisplayName("Touch risky code by change in platform sourceSet") + @TestMetadata("kt-62686-mpp-source-set-boundary") + fun testIncrementalBuildWhereRiskyCodeIsTouchedByChangeInPlatformSourceSet(gradleVersion: GradleVersion) { + project( + "kt-62686-mpp-source-set-boundary", gradleVersion + ) { + projectPath.resolve("src/commonMain/kotlin/riskyCode.kt").replaceWithVersion("useMemberFunctionFromExpectClass") + + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + } + + projectPath.resolve("src/$platformSourceSet/kotlin/dependedOnByActualDeclaration.kt") + .replaceWithVersion("changeType") + + build(taskToExecute) { + assertTasksExecuted(taskToExecute) + assertNonIncrementalCompilation(BuildAttribute.UNSAFE_INCREMENTAL_CHANGE_KT_62686) + } + } + } +} + +@MppGradlePluginTests +@DisplayName("Tests for IC with compatible overloads in common and platform sourceSets - Jvm") +class CommonCodeWithPlatformSymbolsJvmIT() : CommonCodeWithPlatformSymbolsITBase( + platformSourceSet = "jvmMain", taskToExecute = ":compileKotlinJvm" +) + +@MppGradlePluginTests +@DisplayName("Tests for IC with compatible overloads in common and platform sourceSets - Js") +class CommonCodeWithPlatformSymbolsJsIT() : CommonCodeWithPlatformSymbolsITBase( + platformSourceSet = "jsMain", taskToExecute = ":compileKotlinJs" +) diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/KmpIncrementalITBase.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/KmpIncrementalITBase.kt index e6de7e98dba..3211b31f158 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/KmpIncrementalITBase.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/KmpIncrementalITBase.kt @@ -20,7 +20,12 @@ import java.nio.file.Path abstract class KmpIncrementalITBase : KGPBaseTest() { override val defaultBuildOptions: BuildOptions - get() = super.defaultBuildOptions.copyEnsuringK2() + get() = super.defaultBuildOptions.copyEnsuringK2().copy( + /** + * disable IC-breaking feature; it's tested separately in [org.jetbrains.kotlin.gradle.mpp.CommonCodeWithPlatformSymbolsITBase] + */ + enableUnsafeIncrementalCompilationForMultiplatform = true + ) protected open val gradleTask = "assemble" protected open val projectName = "generic-kmp-app-plus-lib-with-tests" diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/smoke/ExpectActualIncrementalCompilationIT.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/smoke/ExpectActualIncrementalCompilationIT.kt index 6db4ccfbd8f..8fc8f16daf9 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/smoke/ExpectActualIncrementalCompilationIT.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/mpp/smoke/ExpectActualIncrementalCompilationIT.kt @@ -22,7 +22,10 @@ import org.junit.jupiter.api.DisplayName @MppGradlePluginTests open class ExpectActualIncrementalCompilationIT : KGPBaseTest() { override val defaultBuildOptions: BuildOptions - get() = super.defaultBuildOptions.copyEnsuringK2() + get() = super.defaultBuildOptions.copyEnsuringK2().copy( + // disable IC-breaking feature; it's tested separately in [org.jetbrains.kotlin.gradle.mpp.CommonCodeWithPlatformSymbolsITBase] + enableUnsafeIncrementalCompilationForMultiplatform = true + ) @DisplayName("File with actual declaration needs recompiling") @GradleTest diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/testbase/BuildOptions.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/testbase/BuildOptions.kt index 8770086ebad..0b2e436c22c 100644 --- a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/testbase/BuildOptions.kt +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/kotlin/org/jetbrains/kotlin/gradle/testbase/BuildOptions.kt @@ -46,6 +46,7 @@ data class BuildOptions( val statisticsForceValidation: Boolean = true, val usePreciseOutputsBackup: Boolean? = null, val keepIncrementalCompilationCachesInMemory: Boolean? = null, + val enableUnsafeIncrementalCompilationForMultiplatform: Boolean? = null, val useDaemonFallbackStrategy: Boolean = false, val useParsableDiagnosticsFormatting: Boolean = true, val showDiagnosticsStacktrace: Boolean? = false, // false by default to not clutter the testdata + stacktraces change often @@ -195,6 +196,10 @@ data class BuildOptions( arguments.add("-Pkotlin.compiler.keepIncrementalCompilationCachesInMemory=$keepIncrementalCompilationCachesInMemory") } + if (enableUnsafeIncrementalCompilationForMultiplatform != null) { + arguments.add("-Pkotlin.internal.incremental.enableUnsafeOptimizationsForMultiplatform=$enableUnsafeIncrementalCompilationForMultiplatform") + } + arguments.add("-Pkotlin.daemon.useFallbackStrategy=$useDaemonFallbackStrategy") if (useParsableDiagnosticsFormatting) { diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/build.gradle.kts b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/build.gradle.kts new file mode 100644 index 00000000000..081d42edfa0 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/build.gradle.kts @@ -0,0 +1,18 @@ +plugins { + kotlin("multiplatform") +} + +repositories { + mavenLocal() + mavenCentral() +} + +kotlin { + jvm().compilations.all { + compilerOptions.configure { + // log level isn't properly used to set `verbose` in the default configuration, fix is WIP in KT-64698 + verbose.convention(true) + } + } + js(IR) +} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/dependedOnByRiskyCode.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/dependedOnByRiskyCode.kt new file mode 100644 index 00000000000..d19999e3953 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/dependedOnByRiskyCode.kt @@ -0,0 +1 @@ +val dependedOnByRiskyCode = 1 diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/dependedOnByRiskyCode.kt.changeType b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/dependedOnByRiskyCode.kt.changeType new file mode 100644 index 00000000000..153c4ae29de --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/dependedOnByRiskyCode.kt.changeType @@ -0,0 +1 @@ +val dependedOnByRiskyCode = "2" diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt new file mode 100644 index 00000000000..21b9038199b --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt @@ -0,0 +1,6 @@ + +fun test(): String = "!" //A().foo(Child()) + +fun test2(): String = "?" //foo(Child()) + +private val dependency = dependedOnByRiskyCode diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt.useMemberFunctionFromExpectClass b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt.useMemberFunctionFromExpectClass new file mode 100644 index 00000000000..c48fc7add44 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt.useMemberFunctionFromExpectClass @@ -0,0 +1,6 @@ + +fun test(): String = A().foo(Child()) + +fun test2(): String = "?" //foo(Child()) + +private val dependency = dependedOnByRiskyCode diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt.useTopLevelFunction b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt.useTopLevelFunction new file mode 100644 index 00000000000..bfe46dd3899 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/riskyCode.kt.useTopLevelFunction @@ -0,0 +1,6 @@ + +fun test(): String = "!" //A().foo(Child()) + +fun test2(): String = foo(Child()) + +private val dependency = dependedOnByRiskyCode diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/theseDeclarationsShouldBeUsedInCommonCode.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/theseDeclarationsShouldBeUsedInCommonCode.kt new file mode 100644 index 00000000000..6aa7df64e55 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/commonMain/kotlin/theseDeclarationsShouldBeUsedInCommonCode.kt @@ -0,0 +1,9 @@ + +open class Base +class Child: Base() + +fun foo(x: Base): String = "" + +expect class A() { + fun foo(x: Base): String +} diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/actualClassAndFunWithExtraOverloads.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/actualClassAndFunWithExtraOverloads.kt new file mode 100644 index 00000000000..e73a8403017 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/actualClassAndFunWithExtraOverloads.kt @@ -0,0 +1,18 @@ +class jsMainOnlyType() + +actual class A { + actual fun foo(x: Base) = "aloha" + fun foo(x: Child) = jsMainOnlyType() +} + +fun foo(x: Child) = jsMainOnlyType() + +fun testPlatform() = + A().foo(Child()) + +fun main() { + println(test()) + println(testPlatform()) +} + +private val dependency = dependedOnByActualDeclaration diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/dependedOnByActualDeclaration.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/dependedOnByActualDeclaration.kt new file mode 100644 index 00000000000..7758ce01136 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/dependedOnByActualDeclaration.kt @@ -0,0 +1 @@ +val dependedOnByActualDeclaration = 1 diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/dependedOnByActualDeclaration.kt.changeType b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/dependedOnByActualDeclaration.kt.changeType new file mode 100644 index 00000000000..23634365a9b --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jsMain/kotlin/dependedOnByActualDeclaration.kt.changeType @@ -0,0 +1 @@ +val dependedOnByActualDeclaration = "2" diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/actualClassAndFunWithExtraOverloads.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/actualClassAndFunWithExtraOverloads.kt new file mode 100644 index 00000000000..b124980a3e1 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/actualClassAndFunWithExtraOverloads.kt @@ -0,0 +1,18 @@ +class jvmMainOnlyType + +actual class A { + actual fun foo(x: Base) = "aloha" + fun foo(x: Child) = jvmMainOnlyType() +} + +fun foo(x: Child) = jvmMainOnlyType() + +fun testPlatform() = + A().foo(Child()) + +fun main() { + println(test()) + println(testPlatform()) +} + +private val dependency = dependedOnByActualDeclaration diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/dependedOnByActualDeclaration.kt b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/dependedOnByActualDeclaration.kt new file mode 100644 index 00000000000..7758ce01136 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/dependedOnByActualDeclaration.kt @@ -0,0 +1 @@ +val dependedOnByActualDeclaration = 1 diff --git a/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/dependedOnByActualDeclaration.kt.changeType b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/dependedOnByActualDeclaration.kt.changeType new file mode 100644 index 00000000000..23634365a9b --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin-integration-tests/src/test/resources/testProject/kt-62686-mpp-source-set-boundary/src/jvmMain/kotlin/dependedOnByActualDeclaration.kt.changeType @@ -0,0 +1 @@ +val dependedOnByActualDeclaration = "2" diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesProvider.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesProvider.kt index 6f37e1604d6..b2edefd8914 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesProvider.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/PropertiesProvider.kt @@ -540,6 +540,13 @@ internal class PropertiesProvider private constructor(private val project: Proje .map { KotlinCompilerArgumentsLogLevel.fromPropertyValue(it) } .orElse(KotlinCompilerArgumentsLogLevel.DEFAULT) + /** + * Without unsafe optimization: in k2, if common source is dirty, module will be rebuilt. + * With unsafe optimization: regular IC logic is used. Common sources might see declarations from platform sources. See KT-62686 + */ + val enableUnsafeOptimizationsForMultiplatform: Boolean + get() = booleanProperty(PropertyNames.KOTLIN_UNSAFE_MULTIPLATFORM_INCREMENTAL_COMPILATION) ?: false + /** * Retrieves a comma-separated list of browsers to use when running karma tests for [target] * @see KOTLIN_JS_KARMA_BROWSERS @@ -648,6 +655,7 @@ internal class PropertiesProvider private constructor(private val project: Proje val KOTLIN_CREATE_ARCHIVE_TASKS_FOR_CUSTOM_COMPILATIONS = property("$KOTLIN_INTERNAL_NAMESPACE.mpp.createArchiveTasksForCustomCompilations") val KOTLIN_COMPILER_ARGUMENTS_LOG_LEVEL = property("$KOTLIN_INTERNAL_NAMESPACE.compiler.arguments.log.level") + val KOTLIN_UNSAFE_MULTIPLATFORM_INCREMENTAL_COMPILATION = property("$KOTLIN_INTERNAL_NAMESPACE.incremental.enableUnsafeOptimizationsForMultiplatform") } companion object { diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/AbstractKotlinCompile.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/AbstractKotlinCompile.kt index 641a3867c5e..e6e18d73b16 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/AbstractKotlinCompile.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/AbstractKotlinCompile.kt @@ -218,6 +218,10 @@ abstract class AbstractKotlinCompile @Inject constr @get:Internal internal abstract val keepIncrementalCompilationCachesInMemory: Property + /** See [org.jetbrains.kotlin.incremental.IncrementalCompilationFeatures.enableUnsafeIncrementalCompilationForMultiplatform] */ + @get:Internal + internal abstract val enableUnsafeIncrementalCompilationForMultiplatform: Property + /** Task outputs that we don't want to include in [TaskOutputsBackup] (see [TaskOutputsBackup.outputsToRestore] for more info). */ @get:Internal internal abstract val taskOutputsBackupExcludes: SetProperty @@ -302,6 +306,7 @@ abstract class AbstractKotlinCompile @Inject constr return IncrementalCompilationFeatures( preciseCompilationResultsBackup = preciseCompilationResultsBackup.get(), keepIncrementalCompilationCachesInMemory = keepIncrementalCompilationCachesInMemory.get(), + enableUnsafeIncrementalCompilationForMultiplatform = enableUnsafeIncrementalCompilationForMultiplatform.get(), ) } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/configuration/AbstractKotlinCompileConfig.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/configuration/AbstractKotlinCompileConfig.kt index a1971bd702d..16f94f26cc8 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/configuration/AbstractKotlinCompileConfig.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/tasks/configuration/AbstractKotlinCompileConfig.kt @@ -80,6 +80,9 @@ internal abstract class AbstractKotlinCompileConfig