[Commonizer] Calculate 'commonModuleNames' in hierarchical context

^KT-46330 Verification Pending

native/commonizer/src/org/jetbrains/kotlin/commonizer/CommonizerParameters.kt

@@ -20,17 +20,6 @@ class CommonizerParameters(
     val progressLogger: ((String) -> Unit)? = null,
 )
 
-fun CommonizerParameters.getCommonModuleNames(): Set<String> {
-    val supportedTargets = targetProviders.filterNonNull()
-    if (supportedTargets.size == 0) return emptySet() // Nothing to do
-
-    val allModuleNames: List<Set<String>> = supportedTargets.toList().map { targetProvider ->
-        targetProvider.modulesProvider.loadModuleInfos().mapTo(HashSet()) { it.name }
-    }
-
-    return allModuleNames.reduce { a, b -> a intersect b } // there are modules that are present in every target
-}
-
 internal fun CommonizerParameters.dependencyClassifiers(target: CommonizerTarget): CirProvidedClassifiers {
     val modules = outputTarget.withAllAncestors()
         .sortedBy { it.level }

native/commonizer/src/org/jetbrains/kotlin/commonizer/commonModuleNames.kt

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2010-2021 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.commonizer
+
+/**
+ * @return Set of module names that is available across all children targets
+ */
+internal fun CommonizerParameters.commonModuleNames(target: CommonizerTarget): Set<String> {
+    val supportedTargets = target.withAllAncestors().mapNotNull(targetProviders::getOrNull)
+    if (supportedTargets.isEmpty()) return emptySet() // Nothing to do
+
+    val allModuleNames: List<Set<String>> = supportedTargets.toList().map { targetProvider ->
+        targetProvider.modulesProvider.loadModuleInfos().mapTo(HashSet()) { it.name }
+    }
+
+    return allModuleNames.reduce { a, b -> a intersect b } // there are modules that are present in every target
+}
+
+/**
+ * @return Set of module names that this [targetProvider] shares with *at least* one other target
+ */
+internal fun CommonizerParameters.commonModuleNames(targetProvider: TargetProvider): Set<String> {
+    return outputTarget.withAllAncestors()
+        .filter { target -> target isAncestorOf targetProvider.target }
+        .map { target -> commonModuleNames(target) }
+        .fold(emptySet()) { acc, names -> acc + names }
+}
+
+
+internal fun CommonizerParameters.containsCommonModuleNames(): Boolean {
+    return targetProviders.filterNonNull().any { targetProvider -> commonModuleNames(targetProvider).isNotEmpty() }
+}

native/commonizer/src/org/jetbrains/kotlin/commonizer/facade.kt

@@ -25,7 +25,7 @@ import org.jetbrains.kotlin.storage.LockBasedStorageManager
 import org.jetbrains.kotlin.storage.StorageManager
 
 fun runCommonization(parameters: CommonizerParameters) {
-    if (parameters.getCommonModuleNames().isEmpty()) {
+    if (!parameters.containsCommonModuleNames()) {
         parameters.resultsConsumer.allConsumed(Status.NOTHING_TO_DO)
         return
     }
@@ -109,11 +109,13 @@ private fun serialize(parameters: CommonizerParameters, mergedTree: CirRootNode,
 }
 
 private fun serializeMissingModules(parameters: CommonizerParameters, requestedTarget: CommonizerTarget) {
-    if (requestedTarget in parameters.targetProviders.targets) {
+    val targetProvider = parameters.targetProviders.getOrNull(requestedTarget) ?: return
-        parameters.targetProviders[requestedTarget]?.modulesProvider?.loadModuleInfos().orEmpty()
+    val commonModuleNames = parameters.commonModuleNames(targetProvider)
-            .filter { it.name !in parameters.getCommonModuleNames() }
+
-            .forEach { missingModule -> parameters.resultsConsumer.consume(requestedTarget, Missing(missingModule.originalLocation)) }
+    targetProvider.modulesProvider.loadModuleInfos()
-    }
+        .filter { it.name !in commonModuleNames }
+        .forEach { missingModule -> parameters.resultsConsumer.consume(requestedTarget, Missing(missingModule.originalLocation)) }
+
 }
 
 private val KLIB_FRAGMENT_WRITE_STRATEGY = ChunkedKlibModuleFragmentWriteStrategy()

native/commonizer/src/org/jetbrains/kotlin/commonizer/tree/deserializer/RootCirTreeDeserializer.kt

@@ -5,10 +5,7 @@
 
 package org.jetbrains.kotlin.commonizer.tree.deserializer
 
-import org.jetbrains.kotlin.commonizer.CommonizerParameters
+import org.jetbrains.kotlin.commonizer.*
-import org.jetbrains.kotlin.commonizer.TargetProvider
-import org.jetbrains.kotlin.commonizer.dependencyClassifiers
-import org.jetbrains.kotlin.commonizer.getCommonModuleNames
 import org.jetbrains.kotlin.commonizer.mergedtree.CirProvidedClassifiers
 import org.jetbrains.kotlin.commonizer.metadata.CirTypeResolver
 import org.jetbrains.kotlin.commonizer.tree.CirTreeRoot
@@ -18,8 +15,10 @@ internal class RootCirTreeDeserializer(
 ) {
     operator fun invoke(parameters: CommonizerParameters, targetProvider: TargetProvider): CirTreeRoot {
 
+        val commonModuleNames = parameters.commonModuleNames(targetProvider)
+
         val commonModuleInfos = targetProvider.modulesProvider.loadModuleInfos()
-            .filter { moduleInfo -> moduleInfo.name in parameters.getCommonModuleNames() }
+            .filter { moduleInfo -> moduleInfo.name in commonModuleNames }
 
         val typeResolver = CirTypeResolver.create(
             providedClassifiers = CirProvidedClassifiers.of(
@@ -36,3 +35,4 @@ internal class RootCirTreeDeserializer(
         )
     }
 }
+

native/commonizer/tests/org/jetbrains/kotlin/commonizer/AbstractInlineSourcesCommonizationTest.kt

@@ -13,6 +13,7 @@ import org.jetbrains.kotlin.commonizer.ResultsConsumer.ModuleResult.Commonized
 import org.jetbrains.kotlin.commonizer.konan.NativeManifestDataProvider
 import org.jetbrains.kotlin.commonizer.utils.*
 import kotlin.test.assertIs
+import kotlin.test.fail
 
 data class HierarchicalCommonizationResult(
     val inlineSourceTestFactory: DependencyAwareInlineSourceTestFactory,
@@ -131,12 +132,15 @@ abstract class AbstractInlineSourcesCommonizationTest : KtInlineSourceCommonizer
         fun build(): Target = Target(target, modules = modules)
     }
 
-    fun commonize(builder: ParametersBuilder.() -> Unit): HierarchicalCommonizationResult {
+    fun commonize(
+        expectedStatus: ResultsConsumer.Status = ResultsConsumer.Status.DONE,
+        builder: ParametersBuilder.() -> Unit
+    ): HierarchicalCommonizationResult {
         val consumer = MockResultsConsumer()
         val testParameters = ParametersBuilder(this).also(builder).build()
         val commonizerParameters = testParameters.toCommonizerParameters(consumer)
         runCommonization(commonizerParameters)
-        assertEquals(ResultsConsumer.Status.DONE, consumer.status)
+        assertEquals(expectedStatus, consumer.status)
         return HierarchicalCommonizationResult(
             inlineSourceTestFactory = DependencyAwareInlineSourceTestFactory(inlineSourceBuilder, testParameters.dependencies),
             testParameters = testParameters,
@@ -176,7 +180,7 @@ abstract class AbstractInlineSourcesCommonizationTest : KtInlineSourceCommonizer
 /* ASSERTIONS */
 
 fun HierarchicalCommonizationResult.getTarget(target: CommonizerTarget): List<ResultsConsumer.ModuleResult> {
-    return this.results[target] ?: kotlin.test.fail("Missing target $target in results ${this.results.keys}")
+    return this.results[target] ?: fail("Missing target $target in results ${this.results.keys}")
 }
 
 fun HierarchicalCommonizationResult.getTarget(target: String): List<ResultsConsumer.ModuleResult> {
@@ -188,10 +192,16 @@ fun HierarchicalCommonizationResult.assertCommonized(
     moduleBuilder: InlineSourceBuilder.ModuleBuilder.() -> Unit
 ) {
     val inlineSourceTest = inlineSourceTestFactory[target]
-    val commonizedModule = getTarget(target).assertSingleCommonizedModule()
+
     val referenceModule = inlineSourceTest.createModule {
         moduleBuilder()
     }
+
+    val module = getTarget(target).firstOrNull { moduleResult -> moduleResult.libraryName == referenceModule.name }
+        ?: fail("Missing ${referenceModule.name} in target $target")
+
+    val commonizedModule = assertIs<Commonized>(module, "Expected ${module.libraryName} to be 'Commonized'")
+
     assertModulesAreEqual(
         inlineSourceTest.createMetadata(referenceModule), commonizedModule.metadata, target
     )
@@ -206,8 +216,10 @@ fun HierarchicalCommonizationResult.assertCommonized(target: CommonizerTarget, @
 fun HierarchicalCommonizationResult.assertCommonized(target: String, @Language("kotlin") sourceContent: String) =
     assertCommonized(parseCommonizerTarget(target), sourceContent)
 
-fun HierarchicalCommonizationResult.assertCommonized(target: String, moduleBuilder: InlineSourceBuilder.ModuleBuilder.() -> Unit) =
+fun HierarchicalCommonizationResult.assertCommonized(
-    assertCommonized(parseCommonizerTarget(target), moduleBuilder)
+    target: String,
+    moduleBuilder: InlineSourceBuilder.ModuleBuilder.() -> Unit
+) = assertCommonized(parseCommonizerTarget(target), moduleBuilder)
 
 fun Collection<ResultsConsumer.ModuleResult>.assertSingleCommonizedModule(): Commonized {
     kotlin.test.assertEquals(1, size, "Expected exactly one module. Found: ${this.map { it.libraryName }}")

native/commonizer/tests/org/jetbrains/kotlin/commonizer/CommonizerFacadeTest.kt

@@ -89,14 +89,14 @@ class CommonizerFacadeTest {
         private fun doTestNothingToCommonize(originalModules: Map<String, List<String>>) {
             val results = MockResultsConsumer()
             runCommonization(originalModules.toCommonizerParameters(results))
-            assertEquals(results.status, Status.NOTHING_TO_DO)
+            assertEquals(Status.NOTHING_TO_DO, results.status)
             assertTrue(results.modulesByTargets.isEmpty())
         }
 
         private fun doTestSuccessfulCommonization(originalModules: Map<String, List<String>>) {
             val results = MockResultsConsumer()
             runCommonization(originalModules.toCommonizerParameters(results))
-            assertEquals(results.status, Status.DONE)
+            assertEquals(Status.DONE, results.status)
 
             val expectedCommonModuleNames = mutableSetOf<String>()
             originalModules.values.forEachIndexed { index, moduleNames ->
@@ -128,7 +128,6 @@ class CommonizerFacadeTest {
             expectedMissingModuleNames: Set<String>,
             actualModuleResults: Collection<ModuleResult>
         ) {
-            assertEquals(expectedCommonizedModuleNames.size + expectedMissingModuleNames.size, actualModuleResults.size)
 
             val actualCommonizedModuleNames = mutableSetOf<String>()
             val actualMissingModuleNames = mutableSetOf<String>()
@@ -140,6 +139,7 @@ class CommonizerFacadeTest {
                 }
             }
 
+            assertEquals(expectedCommonizedModuleNames.size + expectedMissingModuleNames.size, actualModuleResults.size)
             assertEquals(expectedCommonizedModuleNames, actualCommonizedModuleNames)
             assertEquals(expectedMissingModuleNames, actualMissingModuleNames)
         }

native/commonizer/tests/org/jetbrains/kotlin/commonizer/hierarchical/HierarchicalModuleCommonizationTest.kt

@@ -0,0 +1,247 @@
+/*
+ * Copyright 2010-2021 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.commonizer.hierarchical
+
+import org.jetbrains.kotlin.commonizer.AbstractInlineSourcesCommonizationTest
+import org.jetbrains.kotlin.commonizer.ResultsConsumer.ModuleResult.Missing
+import org.jetbrains.kotlin.commonizer.ResultsConsumer.Status
+import org.jetbrains.kotlin.commonizer.assertCommonized
+import org.jetbrains.kotlin.commonizer.parseCommonizerTarget
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
+
+class HierarchicalModuleCommonizationTest : AbstractInlineSourcesCommonizationTest() {
+
+    fun `test common modules hierarchically`() {
+        val result = commonize {
+            outputTarget("((a, b), c)")
+
+            target("a") {
+                module {
+                    name = "foo"
+                    source("val foo: Int = 1")
+                }
+
+                module {
+                    name = "bar"
+                    source("val bar: Int = 1")
+                }
+            }
+
+            target("b") {
+                module {
+                    name = "foo"
+                    source("val foo: Int = 1")
+                }
+
+                module {
+                    name = "bar"
+                    source("val bar: Int = 1")
+                }
+            }
+
+            target("c") {
+                module {
+                    name = "foo"
+                    source("val foo: Int = 1")
+                }
+
+                module {
+                    name = "not-bar"
+                    source("val bar: Int = 1")
+                }
+            }
+        }
+
+        result.assertCommonized("(a, b)") {
+            name = "foo"
+            source("expect val foo: Int")
+        }
+
+        result.assertCommonized("(a, b)") {
+            name = "bar"
+            source("expect val bar: Int")
+        }
+
+
+        result.assertCommonized("((a, b), c)") {
+            name = "foo"
+            source("expect val foo: Int")
+        }
+
+        assertEquals(
+            1, result.results[parseCommonizerTarget("((a, b), c)")].orEmpty().size,
+            "Expected only a single module"
+        )
+    }
+
+    fun `test module commonization with empty root not sharing any module`() {
+        val result = commonize {
+            outputTarget("((a, b), c)")
+
+            target("a") {
+                module {
+                    name = "foo"
+                    source("val foo: Int = 1")
+                }
+            }
+
+            target("b") {
+                module {
+                    name = "foo"
+                    source("val foo: Int = 1")
+                }
+            }
+
+            target("c") {
+                module {
+                    name = "bar"
+                    source("val bar: Int = 1")
+                }
+            }
+        }
+
+        result.assertCommonized("(a, b)") {
+            name = "foo"
+            source("expect val foo: Int")
+        }
+
+        assertTrue(
+            result.results[parseCommonizerTarget("((a, b), c)")].orEmpty().isEmpty(),
+            "Expected empty result for ((a, b), c)"
+        )
+    }
+
+    fun `test no common modules`() {
+        val result = commonize(Status.NOTHING_TO_DO) {
+            outputTarget("((a, b), c)")
+
+            target("a") {
+                module {
+                    name = "a"
+                    source("val foo: Int = 1")
+                }
+            }
+
+            target("b") {
+                module {
+                    name = "b"
+                    source("val foo: Int = 1")
+                }
+            }
+
+            target("c") {
+                module {
+                    name = "c"
+                    source("val foo: Int = 1")
+                }
+            }
+        }
+
+        assertTrue(result.results.isEmpty(), "Expected no results")
+    }
+
+    fun `test propagation`() {
+        val result = commonize {
+            outputTarget("(((a, b), c), d)")
+
+            target("a") {
+                module {
+                    name = "foo"
+                    source("val foo: Int = 1")
+                }
+            }
+
+            target("b") {
+                module {
+                    name = "foo"
+                    source("val foo: Int = 1")
+                }
+            }
+
+            target("d") {
+                module {
+                    name = "foo"
+                    source("val foo: Int = 1")
+                }
+            }
+        }
+
+        result.assertCommonized("((a, b), c)") {
+            name = "foo"
+            source("expect val foo: Int")
+        }
+
+        result.assertCommonized("(((a, b), c), d)") {
+            name = "foo"
+            source("expect val foo: Int")
+        }
+    }
+
+    fun `test missing modules on two targets`() {
+        val result = commonize {
+            outputTarget("(a, b)")
+
+            target("a") {
+                module {
+                    name = "shared"
+                    source("class Shared")
+                }
+
+                module {
+                    name = "onlyInA"
+                    source("class A")
+                }
+            }
+
+            target("b") {
+                module {
+                    name = "shared"
+                    source("class Shared")
+                }
+
+                module {
+                    name = "onlyInB"
+                    source("class B")
+                }
+            }
+        }
+
+        assertEquals(1, result.results[parseCommonizerTarget("(a, b)")]?.size, "Expected only one commonized module")
+
+
+        result.assertCommonized("(a, b)") {
+            name = "shared"
+            source("expect class Shared expect constructor()")
+        }
+
+        result.assertCommonized("a") {
+            name = "shared"
+            source("class Shared constructor()")
+        }
+
+        result.assertCommonized("b") {
+            name = "shared"
+            source("class Shared constructor()")
+        }
+
+        val targetAResults = result.results[parseCommonizerTarget("a")].orEmpty()
+        assertEquals(2, targetAResults.size, "Expected 'Missing' and 'Commonized' results for target a")
+        val targetAMissingModule = kotlin.test.assertNotNull(
+            targetAResults.filterIsInstance<Missing>().singleOrNull(),
+            "Expected 'Missing' result for target a"
+        )
+        assertEquals("onlyInA", targetAMissingModule.libraryName)
+
+        val targetBResults = result.results[parseCommonizerTarget("b")].orEmpty()
+        assertEquals(2, targetBResults.size, "Expected 'Missing' and 'Commonized' results for target b")
+        val targetBMissingModule = kotlin.test.assertNotNull(
+            targetBResults.filterIsInstance<Missing>().singleOrNull(),
+            "Expected 'Missing' result for target b"
+        )
+        assertEquals("onlyInB", targetBMissingModule.libraryName)
+    }
+}

native/commonizer/tests/org/jetbrains/kotlin/commonizer/utils/mocks.kt

@@ -156,7 +156,6 @@ internal class MockResultsConsumer : ResultsConsumer {
 
     override fun targetConsumed(target: CommonizerTarget) {
         check(!this::status.isInitialized)
-        check(target in _modulesByTargets.keys)
         check(target !in finishedTargets)
         finishedTargets += target
     }