From 162cf5ec55f4b189a97eaffe15442ea7afef9294 Mon Sep 17 00:00:00 2001 From: Nikolay Chashnikov Date: Mon, 5 Feb 2024 12:41:19 +0100 Subject: [PATCH] [runtime module repository] include descriptors for test parts of modules to runtime module repository (IDEA-345102) Code which adds transitive dependencies for tests is modified to include only those dependencies which aren't already available. This change reduces the size of module-descriptors.jar from 4.2Mb to 2.2Mb for the intellij ultimate project. GitOrigin-RevId: f1ac6cd72b082afac5a8d3100c8664dbbe80fbe0 --- .../RuntimeModuleRepositoryBuildConstants.kt | 2 +- .../build/RuntimeModuleRepositoryBuilder.kt | 114 ++++++++++++------ ...dModuleRepositoryForIntelliJProjectTest.kt | 3 +- .../RuntimeModuleRepositoryBuilderTest.kt | 80 +++++++++++- 4 files changed, 153 insertions(+), 46 deletions(-) diff --git a/plugins/devkit/runtimeModuleRepository/jps/src/build/RuntimeModuleRepositoryBuildConstants.kt b/plugins/devkit/runtimeModuleRepository/jps/src/build/RuntimeModuleRepositoryBuildConstants.kt index 6c634e53bbd2..f3ddf7d52800 100644 --- a/plugins/devkit/runtimeModuleRepository/jps/src/build/RuntimeModuleRepositoryBuildConstants.kt +++ b/plugins/devkit/runtimeModuleRepository/jps/src/build/RuntimeModuleRepositoryBuildConstants.kt @@ -3,7 +3,7 @@ package com.intellij.devkit.runtimeModuleRepository.jps.build object RuntimeModuleRepositoryBuildConstants { const val JAR_REPOSITORY_FILE_NAME: String = "module-descriptors.jar" - const val GENERATOR_VERSION: Int = 1 + const val GENERATOR_VERSION: Int = 2 /** * Must be equal to the [org.jetbrains.idea.devkit.build.IntelliJModuleRepositoryBuildScopeProvider.TARGET_TYPE_ID] diff --git a/plugins/devkit/runtimeModuleRepository/jps/src/build/RuntimeModuleRepositoryBuilder.kt b/plugins/devkit/runtimeModuleRepository/jps/src/build/RuntimeModuleRepositoryBuilder.kt index 74a957e70dc1..91aec74741df 100644 --- a/plugins/devkit/runtimeModuleRepository/jps/src/build/RuntimeModuleRepositoryBuilder.kt +++ b/plugins/devkit/runtimeModuleRepository/jps/src/build/RuntimeModuleRepositoryBuilder.kt @@ -27,6 +27,7 @@ import org.jetbrains.jps.model.java.JpsJavaExtensionService import org.jetbrains.jps.model.library.JpsLibrary import org.jetbrains.jps.model.library.JpsOrderRootType import org.jetbrains.jps.model.module.JpsModule +import org.jetbrains.jps.model.module.JpsModuleDependency import org.jetbrains.jps.util.JpsPathUtil import java.io.IOException import java.nio.file.Path @@ -41,7 +42,7 @@ internal class RuntimeModuleRepositoryBuilder /** * Specifies whether descriptors for 'tests' parts of modules should be generated. */ - const val GENERATE_DESCRIPTORS_FOR_TEST_MODULES = false + const val GENERATE_DESCRIPTORS_FOR_TEST_MODULES = true private val LOG = logger() internal fun enumerateRuntimeDependencies(module: JpsModule): JpsJavaDependenciesEnumerator { @@ -143,14 +144,14 @@ internal class RuntimeModuleRepositoryBuilder for (module in project.modules) { //if a module doesn't have production sources, it still makes sense to generate a descriptor for it, because it may be used from code if (!module.isTestOnly) { - descriptors.add(createModuleDescriptor(module, false, ::getRuntimeModuleName)) + descriptors.add(createProductionPartDescriptor(module, ::getRuntimeModuleName)) } if (GENERATE_DESCRIPTORS_FOR_TEST_MODULES && module.hasTestSources) { - descriptors.add(createModuleDescriptor(module, true, ::getRuntimeModuleName)) + descriptors.add(createTestPartDescriptor(module, ::getRuntimeModuleName)) } } } - + private val JpsModule.isTestOnly get() = name.endsWith(RuntimeModuleId.TESTS_NAME_SUFFIX) || //todo align module names to get rid of these conditions @@ -166,51 +167,88 @@ internal class RuntimeModuleRepositoryBuilder private val JpsModule.hasProductionSources get() = sourceRoots.any { it.rootType in JavaModuleSourceRootTypes.PRODUCTION } - private fun createModuleDescriptor(module: JpsModule, test: Boolean, runtimeModuleNameGenerator: (JpsModule, Boolean) -> String): RawRuntimeModuleDescriptor { + private fun createProductionPartDescriptor(module: JpsModule, runtimeModuleNameGenerator: (JpsModule, Boolean) -> String): RawRuntimeModuleDescriptor { + val dependencies = LinkedHashSet() + enumerateRuntimeDependencies(module).productionOnly().processModuleAndLibraries( + { dependencies.add(runtimeModuleNameGenerator(it, false)) }, + { dependencies.add(getLibraryId(it).stringId) } + ) + val resourcePaths = if (module.hasProductionSources) listOf("production/${module.name}") else emptyList() + return RawRuntimeModuleDescriptor(runtimeModuleNameGenerator(module, false), resourcePaths, dependencies.toList()) + } + + /** + * Generates a descriptor for [module]'s tests. + * In JPS, tests are added to classpath transitively. For example, if module 'a' depends on 'b', and 'b' depends on 'c', then tests of + * module 'c' will be added to test classpath of module 'a', even if module 'b' has no test sources. + * If we generate synthetic descriptors for tests of each module, even if it doesn't have test sources, the size of the module repository + * will increase a lot. So here we add such transitive test dependencies directly to the module descriptors. To avoid adding too many + * dependencies, we add only those which aren't already available as transitive dependencies of explicitly added dependencies. + */ + private fun createTestPartDescriptor(module: JpsModule, runtimeModuleNameGenerator: (JpsModule, Boolean) -> String): RawRuntimeModuleDescriptor { + val addedTransitiveModuleDependencies = HashSet() + val addedTransitiveLibraryDependencies = HashSet() + + fun JpsJavaDependenciesEnumerator.collectTransitiveDependencies() { + recursively().satisfying { dependency -> + (dependency as? JpsModuleDependency)?.module !in addedTransitiveModuleDependencies + }.processModuleAndLibraries( + { addedTransitiveModuleDependencies.add(it) }, + { addedTransitiveLibraryDependencies.add(it) } + ) + } + + JpsJavaExtensionService.dependencies(module).runtimeOnly().processModules { directDependency -> + if (directDependency.hasTestSources) { + JpsJavaExtensionService.dependencies(module).withoutSdk().runtimeOnly().collectTransitiveDependencies() + } + } + JpsJavaExtensionService.dependencies(module).withoutSdk().runtimeOnly().productionOnly().collectTransitiveDependencies() + addedTransitiveModuleDependencies.remove(module) + val dependencies = LinkedHashSet() val processedDummyTestDependencies = HashSet() - collectDependencies(module, test, dependencies, processedDummyTestDependencies, runtimeModuleNameGenerator) - val sourceRootTypes = if (test) JavaModuleSourceRootTypes.TESTS else JavaModuleSourceRootTypes.PRODUCTION - val resourcePaths = if (module.sourceRoots.any { it.rootType in sourceRootTypes }) { - listOf("${if (test) "test" else "production"}/${module.name}") + if (module.hasProductionSources) { + dependencies.add(runtimeModuleNameGenerator(module, false)) } - else { - emptyList() - } - return RawRuntimeModuleDescriptor(runtimeModuleNameGenerator(module, test), resourcePaths, dependencies.toList()) - } - - private fun collectDependencies(module: JpsModule, - test: Boolean, - result: MutableCollection, - processedDummyTestDependencies: HashSet, - runtimeModuleNameGenerator: (JpsModule, Boolean) -> String) { - val enumerator = enumerateRuntimeDependencies(module) - if (!test) { - enumerator.productionOnly() - } - else if (module.hasProductionSources) { - result.add(runtimeModuleNameGenerator(module, false)) - } - enumerator.processModuleAndLibraries( - { addDependency(result, it, test, processedDummyTestDependencies, runtimeModuleNameGenerator) }, - { result.add(getLibraryId(it).stringId) } + enumerateRuntimeDependencies(module).processModuleAndLibraries( + { dependency -> + if (dependency.hasProductionSources) { + dependencies.add(runtimeModuleNameGenerator(dependency, false)) + } + addTestDependency(dependencies, dependency, processedDummyTestDependencies, addedTransitiveModuleDependencies, + addedTransitiveLibraryDependencies, runtimeModuleNameGenerator) + }, + { dependencies.add(getLibraryId(it).stringId) } ) + val resourcePaths = if (module.hasTestSources) listOf("test/${module.name}") else emptyList() + return RawRuntimeModuleDescriptor(runtimeModuleNameGenerator(module, true), resourcePaths, dependencies.toList()) } - private fun addDependency(result: MutableCollection, - module: JpsModule, - test: Boolean, - processedDummyTestDependencies: HashSet, - runtimeModuleNameGenerator: (JpsModule, Boolean) -> String) { - if (!test || module.hasTestSources) { - result.add(runtimeModuleNameGenerator(module, test)) + private fun addTestDependency(result: MutableCollection, + module: JpsModule, + processedDummyTestDependencies: HashSet, + addedTransitiveModuleDependencies: MutableSet, + addedTransitiveLibraryDependencies: MutableSet, + runtimeModuleNameGenerator: (JpsModule, Boolean) -> String) { + if (module.hasTestSources) { + result.add(runtimeModuleNameGenerator(module, true)) return } if (!processedDummyTestDependencies.add(module.name)) { return } - collectDependencies(module, true, result, processedDummyTestDependencies, runtimeModuleNameGenerator) + enumerateRuntimeDependencies(module).processModuleAndLibraries( + { + addTestDependency(result, it, processedDummyTestDependencies, addedTransitiveModuleDependencies, + addedTransitiveLibraryDependencies, runtimeModuleNameGenerator) + }, + { + if (addedTransitiveLibraryDependencies.add(it)) { + result.add(getLibraryId(it).stringId) + } + } + ) } private fun getLibraryId(library: JpsLibrary): RuntimeModuleId { diff --git a/plugins/devkit/runtimeModuleRepository/jps/testSrc/build/BuildModuleRepositoryForIntelliJProjectTest.kt b/plugins/devkit/runtimeModuleRepository/jps/testSrc/build/BuildModuleRepositoryForIntelliJProjectTest.kt index a51b44084acf..dae2c908fa74 100644 --- a/plugins/devkit/runtimeModuleRepository/jps/testSrc/build/BuildModuleRepositoryForIntelliJProjectTest.kt +++ b/plugins/devkit/runtimeModuleRepository/jps/testSrc/build/BuildModuleRepositoryForIntelliJProjectTest.kt @@ -21,6 +21,7 @@ class BuildModuleRepositoryForIntelliJProjectTest : JpsBuildTestCase() { if (!Files.exists(ultimateRoot.resolve(".idea"))) return loadProject(ultimateRoot.pathString) JpsJavaExtensionService.getInstance().getOrCreateProjectExtension(myProject).outputUrl = getUrl("out") - doBuild(CompileScopeTestBuilder.make().targetTypes(RuntimeModuleRepositoryTarget)).assertSuccessful() + val buildResult = doBuild(CompileScopeTestBuilder.make().targetTypes(RuntimeModuleRepositoryTarget)) + buildResult.assertSuccessful() } } \ No newline at end of file diff --git a/plugins/devkit/runtimeModuleRepository/jps/testSrc/build/RuntimeModuleRepositoryBuilderTest.kt b/plugins/devkit/runtimeModuleRepository/jps/testSrc/build/RuntimeModuleRepositoryBuilderTest.kt index 7e7eb0a8e93c..1b97fcf9faf6 100644 --- a/plugins/devkit/runtimeModuleRepository/jps/testSrc/build/RuntimeModuleRepositoryBuilderTest.kt +++ b/plugins/devkit/runtimeModuleRepository/jps/testSrc/build/RuntimeModuleRepositoryBuilderTest.kt @@ -4,6 +4,9 @@ package com.intellij.devkit.runtimeModuleRepository.jps.build import org.jetbrains.jps.model.java.JavaResourceRootType import org.jetbrains.jps.model.java.JpsJavaDependencyScope import org.jetbrains.jps.model.java.JpsJavaExtensionService +import org.jetbrains.jps.model.java.JpsJavaLibraryType +import org.jetbrains.jps.model.library.JpsOrderRootType +import org.jetbrains.jps.util.JpsPathUtil class RuntimeModuleRepositoryBuilderTest : RuntimeModuleRepositoryTestCase() { fun `test module with tests`() { @@ -56,7 +59,19 @@ class RuntimeModuleRepositoryBuilderTest : RuntimeModuleRepositoryTestCase() { descriptor("a") testDescriptor("a.tests", "a") descriptor("b", "a") - testDescriptor("b.tests", "b", "a.tests") + testDescriptor("b.tests", "b", "a", "a.tests") + } + } + + fun `test dependency on test only module with non-standard name`() { + val aTests = addModule("a.test", withSources = false, withTests = true) + val b = addModule("b", withTests = true) + val dependency = b.dependenciesList.addModuleDependency(aTests) + JpsJavaExtensionService.getInstance().getOrCreateDependencyExtension(dependency).scope = JpsJavaDependencyScope.TEST + buildAndCheck { + testDescriptor("a.test.tests", resourceDirName = "a.test") + descriptor("b") + testDescriptor("b.tests", "b", "a.test.tests") } } @@ -72,6 +87,21 @@ class RuntimeModuleRepositoryBuilderTest : RuntimeModuleRepositoryTestCase() { testDescriptor("c.tests", "c", "b", "a.tests") } } + + fun `test do not add unnecessary transitive dependencies via module without tests`() { + val a = addModule("a", withTests = true) + val b = addModule("b", withTests = false) + val c = addModule("c", a, b, withTests = false) + addModule("d", c, withTests = true) + buildAndCheck { + descriptor("a") + descriptor("b") + descriptor("c", "a", "b") + descriptor("d", "c") + testDescriptor("a.tests", "a") + testDescriptor("d.tests", "d", "c", "a.tests") + } + } fun `test circular dependency with tests`() { val a = addModule("a", withTests = true) @@ -80,9 +110,9 @@ class RuntimeModuleRepositoryBuilderTest : RuntimeModuleRepositoryTestCase() { JpsJavaExtensionService.getInstance().getOrCreateDependencyExtension(dependency).scope = JpsJavaDependencyScope.RUNTIME buildAndCheck { descriptor("a", "b") - testDescriptor("a.tests", "a", "b.tests") + testDescriptor("a.tests", "a", "b", "b.tests") descriptor("b", "a") - testDescriptor("b.tests", "b", "a.tests") + testDescriptor("b.tests", "b", "a", "a.tests") } } @@ -96,17 +126,55 @@ class RuntimeModuleRepositoryBuilderTest : RuntimeModuleRepositoryTestCase() { descriptor("a", "b") descriptor("b", "a") descriptor("c", "b") - testDescriptor("c.tests", "c", "b", "a") + testDescriptor("c.tests", "c", "b") } } fun `test separate module for tests`() { - addModule("a", withTests = false) - addModule("a.tests", withTests = true, withSources = false) + val a = addModule("a", withTests = false) + addModule("a.tests", a, withTests = true, withSources = false) buildAndCheck { descriptor("a") testDescriptor("a.tests", "a", resourceDirName = "a.tests") } } + + fun `test module library`() { + val a = addModule("a", withTests = false) + val lib = a.libraryCollection.addLibrary("lib", JpsJavaLibraryType.INSTANCE) + a.dependenciesList.addLibraryDependency(lib) + val libraryRoot = getUrl("lib") + lib.addRoot(libraryRoot, JpsOrderRootType.COMPILED) + buildAndCheck { + descriptor("a", "lib.a.lib") + descriptor("lib.a.lib", listOf(JpsPathUtil.urlToPath(libraryRoot)), emptyList()) + } + } + + fun `test project library`() { + val a = addModule("a", withTests = false) + val lib = myProject.libraryCollection.addLibrary("lib", JpsJavaLibraryType.INSTANCE) + a.dependenciesList.addLibraryDependency(lib) + val libraryRoot = getUrl("lib") + lib.addRoot(libraryRoot, JpsOrderRootType.COMPILED) + buildAndCheck { + descriptor("a", "lib.lib") + descriptor("lib.lib", listOf(JpsPathUtil.urlToPath(libraryRoot)), emptyList()) + } + } + + fun `test library with test scope`() { + val a = addModule("a", withTests = true) + val lib = myProject.libraryCollection.addLibrary("lib", JpsJavaLibraryType.INSTANCE) + val dependency = a.dependenciesList.addLibraryDependency(lib) + JpsJavaExtensionService.getInstance().getOrCreateDependencyExtension(dependency).scope = JpsJavaDependencyScope.TEST + val libraryRoot = getUrl("lib") + lib.addRoot(libraryRoot, JpsOrderRootType.COMPILED) + buildAndCheck { + descriptor("a") + testDescriptor("a.tests", "a", "lib.lib") + descriptor("lib.lib", listOf(JpsPathUtil.urlToPath(libraryRoot)), emptyList()) + } + } }