stage2/ml9: Fix merging of provided services into KFF #31
Conversation
With kotlinx-metadata-jvm being promoted to stable in Kotlin 2.0, the kotlin-reflect library now relies on a new service ([1]), the services file for which wasn't yet present in older KFF versions. And with the Java module system, which ModLauncher uses, merely merging the service file from the newer Kotlin isn't enough, we also need to merge them into KFF's module descriptor. [1]: https://github.com/JetBrains/kotlin/blob/4c61b7118f23a090a764a3aa07e4291e50210771/libraries/kotlinx-metadata/jvm/resources/META-INF/services/kotlin.metadata.internal.extensions.MetadataExtensions
| Map<String, List<String>> newProvides = newPkgsMeta.descriptor().provides() | ||
| .stream().collect(Collectors.toMap(ModuleDescriptor.Provides::service, ModuleDescriptor.Provides::providers)); | ||
| Map<String, List<String>> mergedProvides = new HashMap<>(orgProvides); | ||
| mergedProvides.putAll(newProvides); |
There was a problem hiding this comment.
Question: I assume we merge to clear duplicate keys, is there any scenario in which we would want to keep the entries in the "value" (List) of the replaced entry?
i.e. would putAll ever need to merge the preexisting list with the new putted list, for duplicate keys
There was a problem hiding this comment.
No, the opposite is true actually, it shouldn't do that because if e.g. the Kotlin which shipped with KFF had a service A with provider A1, which in a later Kotlin version was replaced with provider A2, then it'd be wrong for our list to contain A1 and A2 since A1 won't work any more.
(One might think that there might be a case where KFF itself provides an impl for a Kotlin service, then if Kotlin itself also provides an impl, we'd be hiding KFF's impl. But that's not the case, and except for really old KFF versions, which don't seem to receive any updates any more, KFF's own code is jar-in-jar'd, so would be in a separate module anyway).
With kotlinx-metadata-jvm being promoted to stable in Kotlin 2.0, the kotlin-reflect library now relies on a new service (1), the services file for which wasn't yet present in older KFF versions. And with the Java module system, which ModLauncher uses, merely merging the service file from the newer Kotlin isn't enough, we also need to merge them into KFF's module descriptor.