From d978640c4226d9b74a4da2f22fa7f466ee719f7d Mon Sep 17 00:00:00 2001 From: Sebastian Sellmair Date: Fri, 4 Aug 2023 15:51:35 +0200 Subject: [PATCH] [Gradle] Remove KotlinOutputDependency The IdeBinaryDependencyResolver will filter respective files by using a simpler check: testing if the files do reside within the build dir of the current project. ^KT-60937 Verification Pending --- .../gradle/plugin/KotlinOutputDependency.kt | 109 ------------------ .../IdeBinaryDependencyResolver.kt | 13 +-- ...bstractKotlinCompilationToRunnableFiles.kt | 7 +- .../plugin/mpp/InternalKotlinCompilation.kt | 3 + .../KotlinCompilationAssociator.kt | 55 +++++---- .../compilationImpl/KotlinCompilationImpl.kt | 28 +++-- .../native/KotlinNativeTargetConfigurator.kt | 8 +- .../unitTests/AssociateCompilationsTest.kt | 64 ++++++++++ 8 files changed, 126 insertions(+), 161 deletions(-) delete mode 100644 libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinOutputDependency.kt create mode 100644 libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/AssociateCompilationsTest.kt diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinOutputDependency.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinOutputDependency.kt deleted file mode 100644 index ae7d3d73dfa..00000000000 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/KotlinOutputDependency.kt +++ /dev/null @@ -1,109 +0,0 @@ -/* - * 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.plugin - -import org.gradle.api.artifacts.Dependency -import org.gradle.api.artifacts.FileCollectionDependency -import org.gradle.api.artifacts.component.ComponentIdentifier -import org.gradle.api.file.FileCollection -import org.gradle.api.internal.artifacts.dependencies.SelfResolvingDependencyInternal -import org.gradle.api.tasks.TaskDependency -import org.jetbrains.kotlin.gradle.InternalKotlinGradlePluginApi -import org.jetbrains.kotlin.gradle.targets.native.internal.CInteropIdentifier -import java.io.File - -/** - * Identifier used for [KotlinOutputDependency]. - * All use cases where Kotlin needs to add 'file collection based' dependencies shall be modelled - * using [KotlinOutputDependency] where this identifier is able to provide - * further information about 'what kind of dependency' this is used. - * - * This identifier basically contains - * 1) The information that this dependency was added by the Kotlin Gradle Plugin - * 2) The information that the files are locally built - * 3) The information about the semantics of the dependency - */ -internal sealed class KotlinOutputDependencyIdentifier : ComponentIdentifier { - - /** - * Indicates that the dependency is representing the compilation under - * the specified coordinates and that the reason for the dependency is a 'compilation association' - * - * e.g. 'test compilations' are associated with 'main compilations' - */ - internal class AssociateCompilation( - val projectPath: String, val target: String, val compilation: String, - ) : KotlinOutputDependencyIdentifier() { - constructor(compilation: KotlinCompilation<*>) : this( - projectPath = compilation.project.path, - target = compilation.target.targetName, - compilation = compilation.compilationName - ) - } - - /** - * Indicates that the dependency represents the output of the 'cinterop tool' for a single cinterop, - * identified by the specified [identifier] - */ - internal class CInterop( - val identifier: CInteropIdentifier, - ) : KotlinOutputDependencyIdentifier() - - override fun getDisplayName(): String = when (this) { - is AssociateCompilation -> "Associate Compilation: '$projectPath/$target/$compilation'" - is CInterop -> "CInterop ${identifier.uniqueName}" - } - - override fun toString(): String = displayName -} - -/** - * Special 'Dependency' implementation that shall be used to add 'file collection' based dependencies - * to Kotlin entities (such as compilations). This dependency implementation retains information about - * the semantics of the file collection underneath (by providing a [KotlinOutputDependencyIdentifier]) - */ -internal class KotlinOutputDependency internal constructor( - private val identifier: KotlinOutputDependencyIdentifier, - private val files: FileCollection, -) : FileCollectionDependency, SelfResolvingDependencyInternal { - private var reason: String? = null - - override fun getTargetComponentId(): ComponentIdentifier = identifier - override fun getGroup(): String? = null - override fun getName(): String = "unspecified" - override fun getVersion(): String? = null - - override fun contentEquals(dependency: Dependency): Boolean { - if (dependency !is KotlinOutputDependency) return false - return files == dependency.files - } - - override fun copy(): KotlinOutputDependency { - return KotlinOutputDependency(identifier, files) - } - - override fun getReason(): String? = reason - - override fun because(reason: String?) { - this.reason = reason - } - - override fun getBuildDependencies(): TaskDependency { - return files.buildDependencies - } - - override fun resolve(): MutableSet { - return files.files - } - - override fun resolve(transitive: Boolean): MutableSet { - return files.files - } - - override fun getFiles(): FileCollection { - return files - } -} diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/ide/dependencyResolvers/IdeBinaryDependencyResolver.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/ide/dependencyResolvers/IdeBinaryDependencyResolver.kt index 4f575edf4bf..a6f6033a9a1 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/ide/dependencyResolvers/IdeBinaryDependencyResolver.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/ide/dependencyResolvers/IdeBinaryDependencyResolver.kt @@ -17,7 +17,6 @@ import org.jetbrains.kotlin.gradle.idea.tcs.* import org.jetbrains.kotlin.gradle.idea.tcs.extras.artifactsClasspath import org.jetbrains.kotlin.gradle.idea.tcs.extras.isOpaqueFileDependency import org.jetbrains.kotlin.gradle.plugin.KotlinCompilation -import org.jetbrains.kotlin.gradle.plugin.KotlinOutputDependencyIdentifier import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType import org.jetbrains.kotlin.gradle.plugin.KotlinSourceSet import org.jetbrains.kotlin.gradle.plugin.ide.* @@ -172,11 +171,13 @@ class IdeBinaryDependencyResolver @JvmOverloads constructor( } is OpaqueComponentArtifactIdentifier -> { - /* Such dependencies *would* require implementing a resolver */ + /* Files within the build directory still require a custom resolver */ + if (artifact.file.absoluteFile.startsWith(sourceSet.project.buildDir.absoluteFile)) return@mapNotNull null + IdeaKotlinResolvedBinaryDependency( binaryType = binaryType, coordinates = IdeaKotlinBinaryCoordinates( group = "", - module = componentId.file.relativeOrAbsolute(sourceSet.project.rootDir), + module = artifact.file.relativeOrAbsolute(sourceSet.project.rootDir), version = null, sourceSetName = null ), @@ -186,12 +187,6 @@ class IdeBinaryDependencyResolver @JvmOverloads constructor( } } - /* - Such dependencies (e.g. associate compilations, cinterops, ...) - Will have dedicated resolvers implemented - */ - is KotlinOutputDependencyIdentifier -> null - else -> { logger.warn("Unhandled componentId: ${componentId.javaClass}") null diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/AbstractKotlinCompilationToRunnableFiles.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/AbstractKotlinCompilationToRunnableFiles.kt index 9ff59ce598f..8c11d6a93fb 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/AbstractKotlinCompilationToRunnableFiles.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/AbstractKotlinCompilationToRunnableFiles.kt @@ -19,6 +19,9 @@ internal constructor(compilation: KotlinCompilationImpl) : AbstractKotlinCompila final override val runtimeDependencyConfigurationName: String get() = compilation.runtimeDependencyConfigurationName ?: error("$compilation: Missing 'runtimeDependencyConfigurationName'") - final override var runtimeDependencyFiles: FileCollection = - compilation.runtimeDependencyFiles ?: error("$compilation: Missing 'runtimeDependencyFiles'") + final override var runtimeDependencyFiles: FileCollection + get() = compilation.runtimeDependencyFiles ?: error("$compilation: Missing 'runtimeDependencyFiles'") + set(value) { + compilation.runtimeDependencyFiles = value + } } \ No newline at end of file diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/InternalKotlinCompilation.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/InternalKotlinCompilation.kt index 0fd9d42ff9f..e2035d86938 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/InternalKotlinCompilation.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/InternalKotlinCompilation.kt @@ -21,6 +21,9 @@ internal interface InternalKotlinCompilation : Kotl override val kotlinSourceSets: ObservableSet override val allKotlinSourceSets: ObservableSet + val associatedCompilations: ObservableSet> + val allAssociatedCompilations: ObservableSet> + val configurations: KotlinCompilationConfigurationsContainer val friendPaths: Iterable val processResourcesTaskName: String? diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/compilationImpl/KotlinCompilationAssociator.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/compilationImpl/KotlinCompilationAssociator.kt index 25309e67221..b2b79b67f41 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/compilationImpl/KotlinCompilationAssociator.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/compilationImpl/KotlinCompilationAssociator.kt @@ -7,16 +7,17 @@ package org.jetbrains.kotlin.gradle.plugin.mpp.compilationImpl import org.gradle.api.Project import org.gradle.api.artifacts.Dependency -import org.jetbrains.kotlin.gradle.plugin.KotlinOutputDependency -import org.jetbrains.kotlin.gradle.plugin.KotlinOutputDependencyIdentifier +import org.gradle.api.file.FileCollection +import org.gradle.api.tasks.SourceSet import org.jetbrains.kotlin.gradle.plugin.KotlinPlatformType.jvm import org.jetbrains.kotlin.gradle.plugin.KotlinTarget -import org.jetbrains.kotlin.gradle.plugin.mpp.InternalKotlinCompilation -import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinWithJavaTarget -import org.jetbrains.kotlin.gradle.plugin.mpp.isMain -import org.jetbrains.kotlin.gradle.plugin.mpp.isTest +import org.jetbrains.kotlin.gradle.plugin.launch +import org.jetbrains.kotlin.gradle.plugin.mpp.* import org.jetbrains.kotlin.gradle.targets.jvm.KotlinJvmTarget +import org.jetbrains.kotlin.gradle.utils.Future import org.jetbrains.kotlin.gradle.utils.filesProvider +import org.jetbrains.kotlin.gradle.utils.future +import org.jetbrains.kotlin.gradle.utils.listProperty internal fun interface KotlinCompilationAssociator { @@ -28,36 +29,35 @@ internal object DefaultKotlinCompilationAssociator : KotlinCompilationAssociator val project = target.project /* - we add dependencies to compileDependencyConfiguration ('compileClasspath' usually) and runtimeDependency - ('runtimeClasspath') instead of modifying respective api/implementation/compileOnly/runtimeOnly configs + This associator has two jobs + 1) It will add the output of the 'main' compilation as compile & runtime dependency to the 'auxiliary' compilation + 2) It will add all 'declared dependencies' present on 'main' to 'auxiliary' - This is needed because api/implementation/compileOnly/runtimeOnly are used in IDE Import and will leak - to dependencies of IDE modules. But they are not needed here, because IDE resolution works inherently - transitively and symbols from associated compilation will be resolved from source sets of associated - compilation itself (moreover, direct dependencies are not equivalent to transitive ones because of - resolution order - e.g. in case of FQNs clash, so it's even harmful) + For 1) + This is necessary so that all symbols that are declared/produced in 'main' are available in 'auxiliary'. + We use the 'compileOnlyConfiguration' and 'runtimeOnlyConfigurationName' to add the respective classes. + Note (a): This 'associate' function will be called for 'all' associated compilations (full transitive closure) + Note (b): It is important that the compiled output of 'main' is prioritised in the compile path order: + We therefore ensure that the files are added to the front of the compile path. + + This is necessary as other binaries might leak into the compile path which contain the same symbols but + are not marked as 'friend'. We ensure that associate dependencies are resolved first + + For 2) + This is an agreed upon convention: 'test' is able to see all dependencies declared for 'main' + As described in 1b: It needs to be taken care of, that the dependencies are ordered after the output of 'main' */ - project.dependencies.add( - auxiliary.compileOnlyConfigurationName, - KotlinOutputDependency(KotlinOutputDependencyIdentifier.AssociateCompilation(main), main.output.classesDirs) - ) - - project.dependencies.add( - auxiliary.runtimeOnlyConfigurationName, - KotlinOutputDependency(KotlinOutputDependencyIdentifier.AssociateCompilation(main), main.output.allOutputs) - ) - + project.dependencies.add(auxiliary.compileOnlyConfigurationName, main.output.classesDirs) + project.dependencies.add(auxiliary.runtimeOnlyConfigurationName, main.output.allOutputs) // Adding classes that could be produced into non-default destination for JVM target // Check KotlinSourceSetProcessor for details project.dependencies.add( auxiliary.implementationConfigurationName, - KotlinOutputDependency( - KotlinOutputDependencyIdentifier.AssociateCompilation(main), - project.filesProvider { main.defaultSourceSet.kotlin.classesDirectory.orNull?.asFile } - ) + project.filesProvider { main.defaultSourceSet.kotlin.classesDirectory.orNull?.asFile } ) + // Adding declared dependencies auxiliary.compileDependencyConfigurationName.addAllDependenciesFromOtherConfigurations( project, main.apiConfigurationName, @@ -76,7 +76,6 @@ internal object DefaultKotlinCompilationAssociator : KotlinCompilationAssociator internal object KotlinNativeCompilationAssociator : KotlinCompilationAssociator { override fun associate(target: KotlinTarget, auxiliary: InternalKotlinCompilation<*>, main: InternalKotlinCompilation<*>) { - auxiliary.compileDependencyFiles += main.output.classesDirs + target.project.filesProvider { main.compileDependencyFiles } diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/compilationImpl/KotlinCompilationImpl.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/compilationImpl/KotlinCompilationImpl.kt index 29a87098774..10bf4392a6d 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/compilationImpl/KotlinCompilationImpl.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/plugin/mpp/compilationImpl/KotlinCompilationImpl.kt @@ -22,12 +22,13 @@ import org.jetbrains.kotlin.gradle.plugin.mpp.InternalKotlinCompilation import org.jetbrains.kotlin.gradle.plugin.mpp.internal import org.jetbrains.kotlin.gradle.tasks.KotlinCompilationTask import org.jetbrains.kotlin.gradle.tasks.locateTask +import org.jetbrains.kotlin.gradle.utils.MutableObservableSetImpl import org.jetbrains.kotlin.gradle.utils.ObservableSet import org.jetbrains.kotlin.tooling.core.MutableExtras import org.jetbrains.kotlin.tooling.core.mutableExtrasOf internal class KotlinCompilationImpl constructor( - private val params: Params + private val params: Params, ) : InternalKotlinCompilation { //region Params @@ -44,7 +45,7 @@ internal class KotlinCompilationImpl constructor( val kotlinOptions: KotlinCommonOptions, val compilationAssociator: KotlinCompilationAssociator, val compilationFriendPathsResolver: KotlinCompilationFriendPathsResolver, - val compilationSourceSetInclusion: KotlinCompilationSourceSetInclusion + val compilationSourceSetInclusion: KotlinCompilationSourceSetInclusion, ) //endregion @@ -182,24 +183,37 @@ internal class KotlinCompilationImpl constructor( // endregion - private val associateWithImpl = mutableSetOf>() + + private val associatedCompilationsImpl = MutableObservableSetImpl>() + + private val allAssociatedCompilationsImpl = MutableObservableSetImpl>() + + override val associatedCompilations: ObservableSet> + get() = associatedCompilationsImpl + + override val allAssociatedCompilations: ObservableSet> + get() = allAssociatedCompilationsImpl override val associateWith: List> - get() = associateWithImpl.toList() + get() = associatedCompilationsImpl.toList() override fun associateWith(other: KotlinCompilation<*>) { require(other.target == target) { "Only associations between compilations of a single target are supported" } - if (!associateWithImpl.add(other)) return - params.compilationAssociator.associate(target, this, other.internal) + if (!associatedCompilationsImpl.add(other)) return + if (!allAssociatedCompilationsImpl.add(other)) return + other.internal.allAssociatedCompilations.forAll { compilation -> allAssociatedCompilationsImpl.add(compilation) } } - //region final init init { sourceSets.allKotlinSourceSets.forAll { sourceSet -> params.compilationSourceSetInclusion.include(this, sourceSet) } + + allAssociatedCompilations.forAll { compilation -> + params.compilationAssociator.associate(target, this, compilation.internal) + } } //endregion diff --git a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/native/KotlinNativeTargetConfigurator.kt b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/native/KotlinNativeTargetConfigurator.kt index a2c78c71d66..b56a75958fa 100644 --- a/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/native/KotlinNativeTargetConfigurator.kt +++ b/libraries/tools/kotlin-gradle-plugin/src/common/kotlin/org/jetbrains/kotlin/gradle/targets/native/KotlinNativeTargetConfigurator.kt @@ -158,11 +158,7 @@ open class KotlinNativeTargetConfigurator : AbstractKotl val interopOutput = project.files(interopTask.map { it.outputFileProvider }) with(compilation) { - // Register the interop library as a dependency of the compilation to make IDE happy. - project.dependencies.add( - compileDependencyConfigurationName, - KotlinOutputDependency(KotlinOutputDependencyIdentifier.CInterop(interop.identifier), interopOutput) - ) + compileDependencyFiles += interopOutput if (isMain()) { // Add interop library to special CInteropApiElements configuration createCInteropApiElementsKlibArtifact(compilation.target, interop, interopTask) @@ -180,7 +176,7 @@ open class KotlinNativeTargetConfigurator : AbstractKotl // IDE doesn't see this library in module dependencies. So we have to manually add // main interop libraries in dependencies of the default test compilation. target.compilations.findByName(TEST_COMPILATION_NAME)?.let { testCompilation -> - project.dependencies.add(testCompilation.compileDependencyConfigurationName, interopOutput) + testCompilation.compileDependencyFiles += interopOutput testCompilation.cinterops.all { it.dependencyFiles += interopOutput } diff --git a/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/AssociateCompilationsTest.kt b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/AssociateCompilationsTest.kt new file mode 100644 index 00000000000..5f662e23727 --- /dev/null +++ b/libraries/tools/kotlin-gradle-plugin/src/functionalTest/kotlin/org/jetbrains/kotlin/gradle/unitTests/AssociateCompilationsTest.kt @@ -0,0 +1,64 @@ +/* + * 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. + */ + +@file:Suppress("FunctionName") + +package org.jetbrains.kotlin.gradle.unitTests + +import org.jetbrains.kotlin.gradle.dsl.multiplatformExtension +import org.jetbrains.kotlin.gradle.plugin.mpp.internal +import org.jetbrains.kotlin.gradle.util.* +import kotlin.test.Test +import kotlin.test.assertEquals + + +class AssociateCompilationsTest { + + @Test + fun `test - associatedCompilations and allAssociatedCompilations sets`() { + val project = buildProjectWithMPP() + val kotlin = project.multiplatformExtension + kotlin.jvm() + + val main = kotlin.jvm().compilations.main + val test = kotlin.jvm().compilations.test + val bar = kotlin.jvm().compilations.create("bar") + val foo = kotlin.jvm().compilations.create("foo") + + // Check: foo is not associated with anything + assertEquals(emptySet(), foo.internal.associatedCompilations.toSet()) + assertEquals(emptySet(), foo.internal.allAssociatedCompilations.toSet()) + + // Check: foo associates with test (which is already associated with main by default) + foo.associateWith(test) + assertEquals(setOf(test), foo.internal.associatedCompilations.toSet()) + assertEquals(setOf(main, test), foo.internal.allAssociatedCompilations.toSet()) + + // Check: associate main with bar (should be reflected in foo transitively) + main.associateWith(bar) + assertEquals(setOf(test), foo.internal.associatedCompilations.toSet()) + assertEquals(setOf(main, test, bar), foo.internal.allAssociatedCompilations.toSet()) + } + + @Test + fun `test - associate compilation dependency files are first in compile path`() { + val project = buildProject { enableDefaultStdlibDependency(false) } + val kotlin = project.applyMultiplatformPlugin() + val jvm = kotlin.jvm() + val foo = jvm.compilations.create("foo") + + foo.defaultSourceSet.dependencies { + implementation(project.files("bar.jar")) + } + + foo.associateWith(jvm.compilations.main) + + assertEquals( + listOf(jvm.compilations.main.output.classesDirs, project.files("bar.jar")).flatten(), + foo.compileDependencyFiles.files.toList(), + "Expected 'main classesDirs' to be listed before file dependency 'bar.jar'" + ) + } +} \ No newline at end of file