From 1c285de55ead322a07d17bfb70cc11f618c827c0 Mon Sep 17 00:00:00 2001 From: Dmitriy Dolovov Date: Thu, 7 Dec 2023 16:52:13 +0100 Subject: [PATCH] [KLIB Resolver] Prettier KLIB resolver messages - Make the messages that are reported by KLIB resolver prettier - For those messages that affect the resolve process add prefix "KLIB resolver: " - Don't log warning on duplicated libraries on the classpath. This does not make any sense. ^KT-63573 --- .../testData/cli/js/libraryDirNotFound.out | 2 +- .../testData/cli/js/notValidLibraryDir.out | 2 +- .../impl/KotlinLibraryResolverImpl.kt | 41 ++++++++++--------- .../kotlin/library/SearchPathResolver.kt | 14 +++---- .../kotlin/library/ToolingResolve.kt | 7 ++-- .../backend.native/tests/build.gradle | 2 +- .../konan/library/SearchPathResolver.kt | 2 +- 7 files changed, 36 insertions(+), 34 deletions(-) diff --git a/compiler/testData/cli/js/libraryDirNotFound.out b/compiler/testData/cli/js/libraryDirNotFound.out index 36b11e5f7ec..1b2a2dc9876 100644 --- a/compiler/testData/cli/js/libraryDirNotFound.out +++ b/compiler/testData/cli/js/libraryDirNotFound.out @@ -1,2 +1,2 @@ -error: could not find "not/existing/path" in [$USER_DIR$] +error: KLIB resolver: Could not find "not/existing/path" in [$USER_DIR$] COMPILATION_ERROR diff --git a/compiler/testData/cli/js/notValidLibraryDir.out b/compiler/testData/cli/js/notValidLibraryDir.out index bc9c7acc419..c7fce9d2550 100644 --- a/compiler/testData/cli/js/notValidLibraryDir.out +++ b/compiler/testData/cli/js/notValidLibraryDir.out @@ -1,2 +1,2 @@ -error: could not find "compiler/testData/integration/ant/js/simpleWithJsFileAsAnotherLib" in [$USER_DIR$] +error: KLIB resolver: Could not find "compiler/testData/integration/ant/js/simpleWithJsFileAsAnotherLib" in [$USER_DIR$] COMPILATION_ERROR diff --git a/compiler/util-klib-metadata/src/org/jetbrains/kotlin/library/metadata/resolver/impl/KotlinLibraryResolverImpl.kt b/compiler/util-klib-metadata/src/org/jetbrains/kotlin/library/metadata/resolver/impl/KotlinLibraryResolverImpl.kt index f486518f6a5..bc264dabf53 100644 --- a/compiler/util-klib-metadata/src/org/jetbrains/kotlin/library/metadata/resolver/impl/KotlinLibraryResolverImpl.kt +++ b/compiler/util-klib-metadata/src/org/jetbrains/kotlin/library/metadata/resolver/impl/KotlinLibraryResolverImpl.kt @@ -65,33 +65,34 @@ class KotlinLibraryResolverImpl internal constructor( } /** - * Leaves only distinct libraries (by absolute path), warns on duplicated paths. + * Leaves only distinct libraries (by absolute path). */ - private fun List.leaveDistinct() = - this.groupBy { it.libraryFile.absolutePath }.let { groupedByAbsolutePath -> - warnOnLibraryDuplicates(groupedByAbsolutePath.filter { it.value.size > 1 }.keys) - groupedByAbsolutePath.map { it.value.first() } - } + private fun List.leaveDistinct(): List { + if (size <= 1) return this + + val deduplicatedLibraries: Map> = groupByTo(linkedMapOf()) { it.libraryFile.absolutePath } + return deduplicatedLibraries.values.map { it.first() } + } /** - * Having two libraries with the same uniqName we only keep the first one. - * TODO: The old JS plugin passes libraries with the same uniqName twice, - * so make it a warning for now. + * Having two libraries with the same `unique_name` we only keep the first one. + * + * TODO: This is actually undesirable behavior. + * - In certain situations it harms, e.g. KT-63573 + * - But sometimes it is really necessary, e.g. KT-64115 + * - Overall, we should not do any resolve inside the compiler (such as skipping KLIBs that happen to have repeated `unique_name`). + * This is an opaque process which better should be performed by the build system (e.g. Gradle). To be fixed in KT-64169 */ private fun List.omitDuplicateNames() = - this.groupBy { it.uniqueName }.let { groupedByUniqName -> - warnOnLibraryDuplicateNames(groupedByUniqName.filter { it.value.size > 1 }.keys) - groupedByUniqName.map { it.value.first() } + groupBy { it.uniqueName }.let { groupedByUniqName -> + val librariesWithDuplicatedUniqueNames = groupedByUniqName.filterValues { it.size > 1 } + librariesWithDuplicatedUniqueNames.entries.sortedBy { it.key }.forEach { (uniqueName, libraries) -> + val libraryPaths = libraries.map { it.libraryFile.absolutePath }.sorted().joinToString() + logger.warning("KLIB resolver: The same 'unique_name=$uniqueName' found in more than one library: $libraryPaths") + } + groupedByUniqName.map { it.value.first() } // This line is the reason of such issues as KT-63573. } - private fun warnOnLibraryDuplicates(duplicatedPaths: Iterable) { - duplicatedPaths.forEach { logger.warning("library included more than once: $it") } - } - - private fun warnOnLibraryDuplicateNames(duplicatedPaths: Iterable) { - duplicatedPaths.forEach { logger.warning("duplicate library name: $it") } - } - /** * Given the list of root libraries does the following: * diff --git a/compiler/util-klib/src/org/jetbrains/kotlin/library/SearchPathResolver.kt b/compiler/util-klib/src/org/jetbrains/kotlin/library/SearchPathResolver.kt index 6b2024be6b3..85620d0ca5c 100644 --- a/compiler/util-klib/src/org/jetbrains/kotlin/library/SearchPathResolver.kt +++ b/compiler/util-klib/src/org/jetbrains/kotlin/library/SearchPathResolver.kt @@ -56,7 +56,7 @@ interface SearchPathResolver : WithLogger { return if (isDeprecated) LookupResult.FoundWithWarning( library = resolvedLibrary, - warningText = "Library '${libraryPath.path}' was found in a custom library repository '${searchRootPath.path}'. " + + warningText = "KLIB resolver: Library '${libraryPath.path}' was found in a custom library repository '${searchRootPath.path}'. " + "Note, that custom library repositories are deprecated and will be removed in one of the future Kotlin releases. " + "Please, avoid using '-repo' ('-r') compiler option and specify full paths to libraries in compiler CLI arguments." ) @@ -231,7 +231,7 @@ abstract class KotlinLibrarySearchPathResolver( private fun Sequence.filterOutPre_1_4_libraries(): Sequence = this.filter { if (it.isPre_1_4_Library) { - logger.warning("Skipping \"$it\" as it is a pre 1.4 library") + logger.warning("KLIB resolver: Skipping '$it'. This is a pre 1.4 library.") false } else { true @@ -256,7 +256,7 @@ abstract class KotlinLibrarySearchPathResolver( .firstOrNull() .let(::ResolvedLibrary) } catch (e: Throwable) { - logger.error("Failed to resolve Kotlin library: $givenPath") + logger.error("KLIB resolver: Failed to resolve Kotlin library: $givenPath") throw e } }.library @@ -268,7 +268,7 @@ abstract class KotlinLibrarySearchPathResolver( override fun resolve(unresolved: RequiredUnresolvedLibrary, isDefaultLink: Boolean): L { return resolveOrNull(unresolved, isDefaultLink) - ?: logger.fatal("Could not find \"${unresolved.path}\" in ${searchRoots.map { it.searchRootPath.absolutePath }}") + ?: logger.fatal("KLIB resolver: Could not find \"${unresolved.path}\" in ${searchRoots.map { it.searchRootPath.absolutePath }}") } override fun libraryMatch(candidate: L, unresolved: UnresolvedLibrary): Boolean = true @@ -342,7 +342,7 @@ abstract class KotlinLibraryProperResolverWithAttributes( // Please, don't add checks for other versions here. For example, check for the metadata version should be // implemented in KlibDeserializedContainerSource.incompatibility if (candidateAbiVersion?.isCompatible() != true) { - logger.warning("skipping $candidatePath. Incompatible abi version. The current default is '${KotlinAbiVersion.CURRENT}', found '${candidateAbiVersion}'. The library produced by ${candidateCompilerVersion} compiler") + logger.warning("KLIB resolver: Skipping '$candidatePath'. Incompatible ABI version. The current default is '${KotlinAbiVersion.CURRENT}', found '${candidateAbiVersion}'. The library was produced by '$candidateCompilerVersion' compiler.") return false } @@ -350,13 +350,13 @@ abstract class KotlinLibraryProperResolverWithAttributes( candidateLibraryVersion != null && unresolved.libraryVersion != null ) { - logger.warning("skipping $candidatePath. The library versions don't match. Expected '${unresolved.libraryVersion}', found '${candidateLibraryVersion}'") + logger.warning("KLIB resolver: Skipping '$candidatePath'. Library versions don't match. Expected '${unresolved.libraryVersion}', found '${candidateLibraryVersion}'.") return false } candidate.irProviderName?.let { if (it !in knownIrProviders) { - logger.warning("skipping $candidatePath. The library requires unknown IR provider $it.") + logger.warning("KLIB resolver: Skipping '$candidatePath'. The library requires unknown IR provider: $it") return false } } diff --git a/compiler/util-klib/src/org/jetbrains/kotlin/library/ToolingResolve.kt b/compiler/util-klib/src/org/jetbrains/kotlin/library/ToolingResolve.kt index 67ae0e9fc80..f77af44f82a 100644 --- a/compiler/util-klib/src/org/jetbrains/kotlin/library/ToolingResolve.kt +++ b/compiler/util-klib/src/org/jetbrains/kotlin/library/ToolingResolve.kt @@ -45,9 +45,10 @@ object ToolingSingleFileKlibResolveStrategy : SingleFileKlibResolveStrategy { } else -> { // TODO: choose the best fit among all available candidates // mimic as old style library and warn - logger.warning("Library $libraryFile can not be read. Multiple components found: ${components.map { - it.path.substringAfter(localRoot.path) - }}") + logger.warning( + "KLIB resolver: Library '$libraryFile' can not be read." + + " Multiple components found: ${components.map { it.path.substringAfter(localRoot.path) }}" + ) null } diff --git a/kotlin-native/backend.native/tests/build.gradle b/kotlin-native/backend.native/tests/build.gradle index 5d8f3643edb..ea971fc3011 100644 --- a/kotlin-native/backend.native/tests/build.gradle +++ b/kotlin-native/backend.native/tests/build.gradle @@ -3937,7 +3937,7 @@ standaloneTest("library_ir_provider_mismatch") { String exceptionText = t.message exceptionText = PlatformInfo.isWindows() ? exceptionText.replace("\\", "/") : exceptionText - String expectedText = "warning: skipping $outputDir/unsupported_ir_provider/empty.klib. The library requires unknown IR provider UNSUPPORTED." + String expectedText = "warning: KLIB resolver: Skipping '$outputDir/unsupported_ir_provider/empty.klib'. The library requires unknown IR provider: UNSUPPORTED" expectedText = PlatformInfo.isWindows() ? expectedText.replace("\\", "/") : expectedText if (t instanceof GradleException && exceptionText.contains(expectedText)) { diff --git a/native/utils/src/org/jetbrains/kotlin/konan/library/SearchPathResolver.kt b/native/utils/src/org/jetbrains/kotlin/konan/library/SearchPathResolver.kt index 6d57a26a489..c49a7309a90 100644 --- a/native/utils/src/org/jetbrains/kotlin/konan/library/SearchPathResolver.kt +++ b/native/utils/src/org/jetbrains/kotlin/konan/library/SearchPathResolver.kt @@ -75,7 +75,7 @@ internal class KonanLibraryProperResolver( val candidatePath = candidate.libraryFile.absolutePath if (!candidate.targetList.contains(resolverTarget.visibleName)) { - logger.warning("skipping $candidatePath. The target doesn't match. Expected '$resolverTarget', found ${candidate.targetList}") + logger.warning("KLIB resolver: Skipping '$candidatePath'. The target doesn't match. Expected '$resolverTarget', found ${candidate.targetList}.") return false }