From 469b11e0aa244e41c431ce67f674516c4e263aee Mon Sep 17 00:00:00 2001 From: Alexey Sedunov Date: Tue, 11 Sep 2018 16:28:32 +0300 Subject: [PATCH] MPP: Drop duplicating library produced by file collection dependency #KT-26675 Fixed --- .../NewMultiplatformProjectImportingTest.kt | 64 ++++++++++++- .../kotlin/gradle/projectStructureDSL.kt | 89 +++++++++++++------ .../src/KotlinMPPGradleModelBuilder.kt | 21 +++-- .../src/KotlinMPPGradleModelBuilder.kt.173 | 21 +++-- .../src/KotlinMPPGradleModelBuilder.kt.181 | 21 +++-- .../src/KotlinMPPGradleModelBuilder.kt.as31 | 21 +++-- 6 files changed, 182 insertions(+), 55 deletions(-) diff --git a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/NewMultiplatformProjectImportingTest.kt b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/NewMultiplatformProjectImportingTest.kt index 5186a77fc3d..39d3df92aa8 100644 --- a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/NewMultiplatformProjectImportingTest.kt +++ b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/NewMultiplatformProjectImportingTest.kt @@ -311,8 +311,68 @@ class NewMultiplatformProjectImportingTest : GradleImportingTestCase() { } } - private fun checkProjectStructure(body: ProjectInfo.() -> Unit = {}) { - checkProjectStructure(myProject, projectPath, body) + @Test + fun testFileCollectionDependency() { + createProjectSubFile( + "build.gradle", + """ + buildscript { + repositories { + mavenLocal() + jcenter() + maven { url 'https://dl.bintray.com/kotlin/kotlin-dev' } + } + dependencies { + classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion" + } + } + + allprojects { + repositories { + mavenLocal() + jcenter() + maven { url 'https://dl.bintray.com/kotlin/kotlin-dev' } + } + } + + apply plugin: 'kotlin-multiplatform' + + kotlin { + sourceSets { + jvmMain { + dependencies { + implementation files("a", "b") + } + } + } + targets { + fromPreset(presets.jvmWithJava, 'jvm') + } + } + """.trimIndent() + ) + + importProject() + + checkProjectStructure( + exhaustiveModuleList = false, + exhaustiveSourceSourceRootList = false + ) { + module("project_jvmMain") { + libraryDependencyByUrl("file://$projectPath/a", DependencyScope.COMPILE) + libraryDependencyByUrl("file://$projectPath/b", DependencyScope.COMPILE) + moduleDependency("project_commonMain", DependencyScope.COMPILE) + moduleDependency("project_main", DependencyScope.COMPILE) + } + } + } + + private fun checkProjectStructure( + exhaustiveModuleList: Boolean = true, + exhaustiveSourceSourceRootList: Boolean = true, + body: ProjectInfo.() -> Unit = {} + ) { + checkProjectStructure(myProject, projectPath, exhaustiveModuleList, exhaustiveSourceSourceRootList, body) } override fun importProject() { diff --git a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/projectStructureDSL.kt b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/projectStructureDSL.kt index f0db3ffdcf6..2c01373844d 100644 --- a/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/projectStructureDSL.kt +++ b/idea/idea-gradle/tests/org/jetbrains/kotlin/gradle/projectStructureDSL.kt @@ -33,8 +33,13 @@ class MessageCollector { } } -class ProjectInfo(project: Project, private val projectPath: String) { - private val messageCollector = MessageCollector() +class ProjectInfo( + project: Project, + internal val projectPath: String, + internal val exhaustiveModuleList: Boolean, + internal val exhaustiveSourceSourceRootList: Boolean +) { + internal val messageCollector = MessageCollector() private val moduleManager = ModuleManager.getInstance(project) private val expectedModuleNames = HashSet() private var allModulesAsserter: (ModuleInfo.() -> Unit)? = null @@ -50,7 +55,7 @@ class ProjectInfo(project: Project, private val projectPath: String) { messageCollector.report("No module found: '$name'") return } - val moduleInfo = ModuleInfo(module, messageCollector, projectPath) + val moduleInfo = ModuleInfo(module, this) allModulesAsserter?.let { moduleInfo.it() } moduleInfo.run(body) expectedModuleNames += name @@ -59,10 +64,12 @@ class ProjectInfo(project: Project, private val projectPath: String) { fun run(body: ProjectInfo.() -> Unit = {}) { body() - val actualNames = moduleManager.modules.map { it.name }.sorted() - val expectedNames = expectedModuleNames.sorted() - if (actualNames != expectedNames) { - messageCollector.report("Expected module list $expectedNames doesn't match the actual one: $actualNames") + if (exhaustiveModuleList) { + val actualNames = moduleManager.modules.map { it.name }.sorted() + val expectedNames = expectedModuleNames.sorted() + if (actualNames != expectedNames) { + messageCollector.report("Expected module list $expectedNames doesn't match the actual one: $actualNames") + } } messageCollector.check() @@ -71,8 +78,7 @@ class ProjectInfo(project: Project, private val projectPath: String) { class ModuleInfo( val module: Module, - private val messageCollector: MessageCollector, - private val projectPath: String + val projectInfo: ProjectInfo ) { private val rootModel = module.rootManager private val expectedDependencyNames = HashSet() @@ -82,7 +88,7 @@ class ModuleInfo( .flatMap { it.sourceFolders.asSequence() } .mapNotNull { val path = it.file?.path ?: return@mapNotNull null - FileUtil.getRelativePath(projectPath, path, '/')!! to it + FileUtil.getRelativePath(projectInfo.projectPath, path, '/')!! to it } .toMap() } @@ -90,21 +96,21 @@ class ModuleInfo( fun languageVersion(version: String) { val actualVersion = module.languageVersionSettings.languageVersion.versionString if (actualVersion != version) { - messageCollector.report("Module '${module.name}': expected language version '$version' but found '$actualVersion'") + projectInfo.messageCollector.report("Module '${module.name}': expected language version '$version' but found '$actualVersion'") } } fun apiVersion(version: String) { val actualVersion = module.languageVersionSettings.apiVersion.versionString if (actualVersion != version) { - messageCollector.report("Module '${module.name}': expected API version '$version' but found '$actualVersion'") + projectInfo.messageCollector.report("Module '${module.name}': expected API version '$version' but found '$actualVersion'") } } fun platform(platform: IdePlatform<*, *>) { val actualPlatform = module.platform if (actualPlatform != platform) { - messageCollector.report( + projectInfo.messageCollector.report( "Module '${module.name}': expected platform '${platform.description}' but found '${actualPlatform?.description}'" ) } @@ -113,7 +119,7 @@ class ModuleInfo( fun additionalArguments(arguments: String?) { val actualArguments = KotlinFacet.get(module)?.configuration?.settings?.compilerSettings?.additionalArguments if (actualArguments != arguments) { - messageCollector.report( + projectInfo.messageCollector.report( "Module '${module.name}': expected additional arguments '$arguments' but found '$actualArguments'" ) } @@ -121,8 +127,19 @@ class ModuleInfo( fun libraryDependency(libraryName: String, scope: DependencyScope) { val libraryEntry = rootModel.orderEntries.filterIsInstance().singleOrNull { it.libraryName == libraryName } + checkLibrary(libraryEntry, libraryName, scope) + } + + fun libraryDependencyByUrl(classesUrl: String, scope: DependencyScope) { + val libraryEntry = rootModel.orderEntries.filterIsInstance().singleOrNull { entry -> + entry.library?.getUrls(OrderRootType.CLASSES)?.any { it == classesUrl } ?: false + } + checkLibrary(libraryEntry, classesUrl, scope) + } + + private fun checkLibrary(libraryEntry: LibraryOrderEntry?, id: String, scope: DependencyScope) { if (libraryEntry == null) { - messageCollector.report("Module '${module.name}': No library dependency found: '$libraryName'") + projectInfo.messageCollector.report("Module '${module.name}': No library dependency found: '$id'") return } checkDependencyScope(libraryEntry, scope) @@ -132,7 +149,7 @@ class ModuleInfo( fun moduleDependency(moduleName: String, scope: DependencyScope) { val moduleEntry = rootModel.orderEntries.filterIsInstance().singleOrNull { it.moduleName == moduleName } if (moduleEntry == null) { - messageCollector.report("Module '${module.name}': No module dependency found: '$moduleName'") + projectInfo.messageCollector.report("Module '${module.name}': No module dependency found: '$moduleName'") return } checkDependencyScope(moduleEntry, scope) @@ -142,13 +159,13 @@ class ModuleInfo( fun sourceFolder(pathInProject: String, rootType: JpsModuleSourceRootType<*>) { val sourceFolder = sourceFolderByPath[pathInProject] if (sourceFolder == null) { - messageCollector.report("Module '${module.name}': No source folder found: '$pathInProject'") + projectInfo.messageCollector.report("Module '${module.name}': No source folder found: '$pathInProject'") return } expectedSourceRoots += pathInProject val actualRootType = sourceFolder.rootType if (actualRootType != rootType) { - messageCollector.report( + projectInfo.messageCollector.report( "Module '${module.name}', source root '$pathInProject': Expected root type $rootType doesn't match the actual one: $actualRootType" ) return @@ -158,7 +175,7 @@ class ModuleInfo( fun inheritProjectOutput() { val isInherited = CompilerModuleExtension.getInstance(module)?.isCompilerOutputPathInherited ?: true if (!isInherited) { - messageCollector.report("Module '${module.name}': project output is not inherited") + projectInfo.messageCollector.report("Module '${module.name}': project output is not inherited") } } @@ -167,7 +184,7 @@ class ModuleInfo( val url = if (isProduction) compilerModuleExtension?.compilerOutputUrl else compilerModuleExtension?.compilerOutputUrlForTests val actualPathInProject = url?.let { FileUtil.getRelativePath( - projectPath, + projectInfo.projectPath, JpsPathUtil.urlToPath( it ), @@ -175,7 +192,7 @@ class ModuleInfo( ) } if (actualPathInProject != pathInProject) { - messageCollector.report( + projectInfo.messageCollector.report( "Module '${module.name}': Expected output path $pathInProject doesn't match the actual one: $actualPathInProject" ) return @@ -192,24 +209,38 @@ class ModuleInfo( .sorted() val expectedDependencyNames = expectedDependencyNames.sorted() if (actualDependencyNames != expectedDependencyNames) { - messageCollector.report("Module '${module.name}': Expected dependency list $expectedDependencyNames doesn't match the actual one: $actualDependencyNames") + projectInfo.messageCollector.report( + "Module '${module.name}': Expected dependency list $expectedDependencyNames doesn't match the actual one: $actualDependencyNames" + ) } - val actualSourceRoots = sourceFolderByPath.keys.sorted() - val expectedSourceRoots = expectedSourceRoots.sorted() - if (actualSourceRoots != expectedSourceRoots) { - messageCollector.report("Module '${module.name}': Expected source root list $expectedSourceRoots doesn't match the actual one: $actualSourceRoots") + if (projectInfo.exhaustiveSourceSourceRootList) { + val actualSourceRoots = sourceFolderByPath.keys.sorted() + val expectedSourceRoots = expectedSourceRoots.sorted() + if (actualSourceRoots != expectedSourceRoots) { + projectInfo.messageCollector.report( + "Module '${module.name}': Expected source root list $expectedSourceRoots doesn't match the actual one: $actualSourceRoots" + ) + } } } private fun checkDependencyScope(library: ExportableOrderEntry, scope: DependencyScope) { val actualScope = library.scope if (actualScope != scope) { - messageCollector.report("Module '${module.name}': Dependency '${library.presentableName}': expected scope '$scope' but found '$actualScope'") + projectInfo.messageCollector.report( + "Module '${module.name}': Dependency '${library.presentableName}': expected scope '$scope' but found '$actualScope'" + ) } } } -fun checkProjectStructure(project: Project, projectPath: String, body: ProjectInfo.() -> Unit = {}) { - ProjectInfo(project, projectPath).run(body) +fun checkProjectStructure( + project: Project, + projectPath: String, + exhaustiveModuleList: Boolean, + exhaustiveSourceSourceRootList: Boolean, + body: ProjectInfo.() -> Unit = {} +) { + ProjectInfo(project, projectPath, exhaustiveModuleList, exhaustiveSourceSourceRootList).run(body) } \ No newline at end of file diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt index 257ea08e760..dfedc0bfa36 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt @@ -14,10 +14,7 @@ import org.gradle.api.artifacts.component.ProjectComponentIdentifier import org.gradle.api.file.FileCollection import org.gradle.api.file.SourceDirectorySet import org.gradle.api.provider.Property -import org.jetbrains.plugins.gradle.model.AbstractExternalDependency -import org.jetbrains.plugins.gradle.model.DefaultExternalProjectDependency -import org.jetbrains.plugins.gradle.model.ExternalDependency -import org.jetbrains.plugins.gradle.model.ExternalProjectDependency +import org.jetbrains.plugins.gradle.model.* import org.jetbrains.plugins.gradle.tooling.ErrorMessageBuilder import org.jetbrains.plugins.gradle.tooling.ModelBuilderService import org.jetbrains.plugins.gradle.tooling.util.DependencyResolver @@ -135,13 +132,14 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { dependency to artifact } .groupBy { (it.second.id.componentIdentifier as ProjectComponentIdentifier).projectPath } - return dependencyResolver.resolveDependencies(configuration) + val resolvedDependencies = dependencyResolver.resolveDependencies(configuration) .apply { forEach { (it as? AbstractExternalDependency)?.scope = scope } } .map { dependency -> if (dependency !is ExternalProjectDependency - || dependency.configurationName != Dependency.DEFAULT_CONFIGURATION) return@map dependency + || dependency.configurationName != Dependency.DEFAULT_CONFIGURATION + ) return@map dependency val artifacts = dependenciesByProjectPath[dependency.projectPath] ?: return@map dependency val artifactConfiguration = artifacts.mapTo(LinkedHashSet()) { it.first.configuration @@ -162,6 +160,17 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { this.classifier = classifier } } + val singleDependencyFiles = resolvedDependencies.mapNotNullTo(LinkedHashSet()) { + (it as? FileCollectionDependency)?.files?.singleOrNull() + } + // Workaround for duplicated dependencies specified as a file collection (KT-26675) + // Drop this code when the issue is fixed in the platform + return resolvedDependencies.filter { dependency -> + if (dependency !is FileCollectionDependency) return@filter true + val files = dependency.files + if (files.size <= 1) return@filter true + (files.any { it !in singleDependencyFiles }) + } } private fun buildTargets( diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.173 b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.173 index 703d3312cc3..138444b648f 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.173 +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.173 @@ -13,10 +13,7 @@ import org.gradle.api.artifacts.Dependency import org.gradle.api.artifacts.component.ProjectComponentIdentifier import org.gradle.api.file.FileCollection import org.gradle.api.file.SourceDirectorySet -import org.jetbrains.plugins.gradle.model.AbstractExternalDependency -import org.jetbrains.plugins.gradle.model.DefaultExternalProjectDependency -import org.jetbrains.plugins.gradle.model.ExternalDependency -import org.jetbrains.plugins.gradle.model.ExternalProjectDependency +import org.jetbrains.plugins.gradle.model.* import org.jetbrains.plugins.gradle.tooling.ErrorMessageBuilder import org.jetbrains.plugins.gradle.tooling.ModelBuilderService import org.jetbrains.plugins.gradle.tooling.util.DependencyResolver @@ -134,13 +131,14 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { dependency to artifact } .groupBy { (it.second.id.componentIdentifier as ProjectComponentIdentifier).projectPath } - return dependencyResolver.resolveDependencies(configuration) + val resolvedDependencies = dependencyResolver.resolveDependencies(configuration) .apply { forEach { (it as? AbstractExternalDependency)?.scope = scope } } .map { dependency -> if (dependency !is ExternalProjectDependency - || dependency.configurationName != Dependency.DEFAULT_CONFIGURATION) return@map dependency + || dependency.configurationName != Dependency.DEFAULT_CONFIGURATION + ) return@map dependency val artifacts = dependenciesByProjectPath[dependency.projectPath] ?: return@map dependency val artifactConfiguration = artifacts.mapTo(LinkedHashSet()) { it.first.configuration @@ -161,6 +159,17 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { this.classifier = classifier } } + val singleDependencyFiles = resolvedDependencies.mapNotNullTo(LinkedHashSet()) { + (it as? FileCollectionDependency)?.files?.singleOrNull() + } + // Workaround for duplicated dependencies specified as a file collection (KT-26675) + // Drop this code when the issue is fixed in the platform + return resolvedDependencies.filter { dependency -> + if (dependency !is FileCollectionDependency) return@filter true + val files = dependency.files + if (files.size <= 1) return@filter true + (files.any { it !in singleDependencyFiles }) + } } private fun buildTargets( diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.181 b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.181 index 1b4df9ac9da..2fb80934c6a 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.181 +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.181 @@ -14,10 +14,7 @@ import org.gradle.api.artifacts.component.ProjectComponentIdentifier import org.gradle.api.file.FileCollection import org.gradle.api.file.SourceDirectorySet import org.gradle.api.provider.Property -import org.jetbrains.plugins.gradle.model.AbstractExternalDependency -import org.jetbrains.plugins.gradle.model.DefaultExternalProjectDependency -import org.jetbrains.plugins.gradle.model.ExternalDependency -import org.jetbrains.plugins.gradle.model.ExternalProjectDependency +import org.jetbrains.plugins.gradle.model.* import org.jetbrains.plugins.gradle.tooling.ErrorMessageBuilder import org.jetbrains.plugins.gradle.tooling.ModelBuilderService import org.jetbrains.plugins.gradle.tooling.util.DependencyResolver @@ -135,13 +132,14 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { dependency to artifact } .groupBy { (it.second.id.componentIdentifier as ProjectComponentIdentifier).projectPath } - return dependencyResolver.resolveDependencies(configuration) + val resolvedDependencies = dependencyResolver.resolveDependencies(configuration) .apply { forEach { (it as? AbstractExternalDependency)?.scope = scope } } .map { dependency -> if (dependency !is ExternalProjectDependency - || dependency.configurationName != Dependency.DEFAULT_CONFIGURATION) return@map dependency + || dependency.configurationName != Dependency.DEFAULT_CONFIGURATION + ) return@map dependency val artifacts = dependenciesByProjectPath[dependency.projectPath] ?: return@map dependency val artifactConfiguration = artifacts.mapTo(LinkedHashSet()) { it.first.configuration @@ -162,6 +160,17 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { this.classifier = classifier } } + val singleDependencyFiles = resolvedDependencies.mapNotNullTo(LinkedHashSet()) { + (it as? FileCollectionDependency)?.files?.singleOrNull() + } + // Workaround for duplicated dependencies specified as a file collection (KT-26675) + // Drop this code when the issue is fixed in the platform + return resolvedDependencies.filter { dependency -> + if (dependency !is FileCollectionDependency) return@filter true + val files = dependency.files + if (files.size <= 1) return@filter true + (files.any { it !in singleDependencyFiles }) + } } private fun buildTargets( diff --git a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.as31 b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.as31 index 703d3312cc3..138444b648f 100644 --- a/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.as31 +++ b/idea/kotlin-gradle-tooling/src/KotlinMPPGradleModelBuilder.kt.as31 @@ -13,10 +13,7 @@ import org.gradle.api.artifacts.Dependency import org.gradle.api.artifacts.component.ProjectComponentIdentifier import org.gradle.api.file.FileCollection import org.gradle.api.file.SourceDirectorySet -import org.jetbrains.plugins.gradle.model.AbstractExternalDependency -import org.jetbrains.plugins.gradle.model.DefaultExternalProjectDependency -import org.jetbrains.plugins.gradle.model.ExternalDependency -import org.jetbrains.plugins.gradle.model.ExternalProjectDependency +import org.jetbrains.plugins.gradle.model.* import org.jetbrains.plugins.gradle.tooling.ErrorMessageBuilder import org.jetbrains.plugins.gradle.tooling.ModelBuilderService import org.jetbrains.plugins.gradle.tooling.util.DependencyResolver @@ -134,13 +131,14 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { dependency to artifact } .groupBy { (it.second.id.componentIdentifier as ProjectComponentIdentifier).projectPath } - return dependencyResolver.resolveDependencies(configuration) + val resolvedDependencies = dependencyResolver.resolveDependencies(configuration) .apply { forEach { (it as? AbstractExternalDependency)?.scope = scope } } .map { dependency -> if (dependency !is ExternalProjectDependency - || dependency.configurationName != Dependency.DEFAULT_CONFIGURATION) return@map dependency + || dependency.configurationName != Dependency.DEFAULT_CONFIGURATION + ) return@map dependency val artifacts = dependenciesByProjectPath[dependency.projectPath] ?: return@map dependency val artifactConfiguration = artifacts.mapTo(LinkedHashSet()) { it.first.configuration @@ -161,6 +159,17 @@ class KotlinMPPGradleModelBuilder : ModelBuilderService { this.classifier = classifier } } + val singleDependencyFiles = resolvedDependencies.mapNotNullTo(LinkedHashSet()) { + (it as? FileCollectionDependency)?.files?.singleOrNull() + } + // Workaround for duplicated dependencies specified as a file collection (KT-26675) + // Drop this code when the issue is fixed in the platform + return resolvedDependencies.filter { dependency -> + if (dependency !is FileCollectionDependency) return@filter true + val files = dependency.files + if (files.size <= 1) return@filter true + (files.any { it !in singleDependencyFiles }) + } } private fun buildTargets(