[KLIB Resolver] Deprecate Logger.fatal()

Invocation of Logger.fatal() may cause severe side effects such as
throwing an exception or even terminating the current JVM process
(check various implementations of this function for details).

The code that uses Logger.fatal() sometimes expects a particular kind
of side effect. This is totally a design flaw. And it's definitely not
a responsibility of Logger to influence the execution flow of
the program.
This commit is contained in:
Dmitriy Dolovov
2023-11-29 22:07:14 +01:00
committed by Space Team
parent 50dd94502b
commit e92017f64e
11 changed files with 60 additions and 16 deletions
@@ -34,6 +34,7 @@ import java.nio.file.Path
import kotlin.io.path.absolutePathString
import kotlin.io.path.extension
import kotlin.io.path.isDirectory
import org.jetbrains.kotlin.util.Logger as KLogger
class LLFirStandaloneLibrarySymbolProviderFactory(private val project: Project) : LLFirLibrarySymbolProviderFactory() {
override fun createJvmLibrarySymbolProvider(
@@ -154,7 +155,7 @@ class LLFirStandaloneLibrarySymbolProviderFactory(private val project: Project)
private val LOG = Logger.getInstance(LLFirStandaloneLibrarySymbolProviderFactory::class.java)
}
private object IntellijLogBasedLogger : org.jetbrains.kotlin.util.Logger {
private object IntellijLogBasedLogger : KLogger {
override fun log(message: String) {
LOG.info(message)
}
@@ -167,6 +168,7 @@ class LLFirStandaloneLibrarySymbolProviderFactory(private val project: Project)
LOG.warn(message)
}
@Deprecated(KLogger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(KLogger.FATAL_REPLACEMENT))
override fun fatal(message: String): Nothing {
throw IllegalStateException(message)
}
@@ -25,6 +25,7 @@ private class CompilerLoggerAdapter(
override fun warning(message: String) = messageCollector.report(if (treatWarningsAsErrors) ERROR else STRONG_WARNING, message, null)
override fun error(message: String) = messageCollector.report(ERROR, message, null)
@Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT))
override fun fatal(message: String): Nothing {
error(message)
(messageCollector as? GroupingMessageCollector)?.flush()
@@ -19,6 +19,7 @@ private class IrMessageLoggerAdapter(private val irMessageLogger: IrMessageLogge
override fun warning(message: String) = irMessageLogger.report(WARNING, message, null)
override fun error(message: String) = irMessageLogger.report(ERROR, message, null)
@Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT))
override fun fatal(message: String): Nothing {
error(message)
throw CompilationErrorException()
@@ -4,9 +4,20 @@ import kotlin.system.exitProcess
interface Logger {
fun log(message: String)
fun error(message: String)
fun warning(message: String)
fun error(message: String)
@Deprecated(FATAL_DEPRECATION_MESSAGE, ReplaceWith(FATAL_REPLACEMENT))
fun fatal(message: String): Nothing
companion object {
const val FATAL_DEPRECATION_MESSAGE = "Invocation of fatal() may cause severe side effects such as throwing an exception or " +
"even terminating the current JVM process (check various implementations of this function for details). " +
"The code that uses Logger.fatal() sometimes expects a particular kind of side effect. " +
"This is an undesirable design. And it's definitely not a responsibility of Logger to influence " +
"the execution flow of the program."
const val FATAL_REPLACEMENT = "error(message)"
}
}
interface WithLogger {
@@ -27,10 +38,12 @@ interface WithLogger {
*/
object DummyLogger : Logger {
override fun log(message: String) = println(message)
override fun error(message: String) = println("e: $message")
override fun warning(message: String) = println("w: $message")
override fun error(message: String) = println("e: $message")
@Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT))
override fun fatal(message: String): Nothing {
println("e: $message")
exitProcess(1)
error(message)
exitProcess(1) // WARNING: This would stop the JVM process!
}
}
@@ -268,7 +268,18 @@ abstract class KotlinLibrarySearchPathResolver<L : KotlinLibrary>(
override fun resolve(unresolved: RequiredUnresolvedLibrary, isDefaultLink: Boolean): L {
return resolveOrNull(unresolved, isDefaultLink)
?: logger.fatal("KLIB resolver: Could not find \"${unresolved.path}\" in ${searchRoots.map { it.searchRootPath.absolutePath }}")
?: run {
// It does not make sense to replace logger.fatal() by logger.error() here, because:
// 1. We don't know which exactly side effect (throwing an exception, exiting JVM process, etc) is performed
// by logger.fatal() as we don't know the concrete Logger class. So, we can't use logger.error()
// and emulate the proper side effect here.
// 2. The contract of resolve(RequiredUnresolvedLibrary, Boolean) has a design flaw: It assumes that either
// the library is resolved or the side effect takes place. The latter (side effect) affects the execution
// flow of the program and should not be a responsibility of SearchPathResolver.
// 3. Finally, we are going to drop SearchPathResolver which is a part of KLIB resolver.
@Suppress("DEPRECATION")
logger.fatal("KLIB resolver: Could not find \"${unresolved.path}\" in ${searchRoots.map { it.searchRootPath.absolutePath }}")
}
}
override fun libraryMatch(candidate: L, unresolved: UnresolvedLibrary): Boolean = true
@@ -371,6 +371,8 @@ fun isKotlinLibrary(libraryFile: File): Boolean = try {
override fun log(message: String) = Unit // don't log
override fun error(message: String) = Unit // don't log
override fun warning(message: String) = Unit // don't log
@Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT))
override fun fatal(message: String): Nothing = kotlin.error("This function should not be called")
},
knownIrProviders = emptyList()
@@ -155,10 +155,13 @@ internal fun logError(text: String, withStacktrace: Boolean = false): Nothing {
}
object KlibToolLogger : Logger, IrMessageLogger {
override fun log(message: String) = println(message)
override fun warning(message: String) = logWarning(message)
override fun error(message: String) = logWarning(message)
@Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT))
override fun fatal(message: String) = logError(message, withStacktrace = true)
override fun log(message: String) = println(message)
override fun report(severity: IrMessageLogger.Severity, message: String, location: IrMessageLogger.Location?) {
when (severity) {
IrMessageLogger.Severity.INFO -> log(message)
@@ -14,6 +14,7 @@ import org.jetbrains.kotlin.library.uniqueName
import java.io.File
import java.nio.charset.StandardCharsets
import java.security.MessageDigest
import org.jetbrains.kotlin.util.Logger as KLogger
internal fun getCacheDirectory(
rootCacheDirectory: File,
@@ -112,11 +113,13 @@ internal fun getAllDependencies(dependency: ResolvedDependencyResult): Set<Resol
return allDependencies
}
internal class GradleLoggerAdapter(private val gradleLogger: Logger) : org.jetbrains.kotlin.util.Logger {
internal class GradleLoggerAdapter(private val gradleLogger: Logger) : KLogger {
override fun log(message: String) = gradleLogger.info(message)
override fun warning(message: String) = gradleLogger.warn(message)
override fun error(message: String) = kotlin.error(message)
override fun fatal(message: String): Nothing = kotlin.error(message)
override fun error(message: String) = gradleLogger.error(message)
@Deprecated(KLogger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(KLogger.FATAL_REPLACEMENT))
override fun fatal(message: String): Nothing = kotlin.error(message) // WARNING: This would crash Gradle daemon!
}
private fun libraryFilter(artifact: ResolvedArtifactResult): Boolean = artifact.file.absolutePath.endsWith(".klib")
@@ -5,6 +5,7 @@
package org.jetbrains.kotlin.commonizer
import org.jetbrains.kotlin.commonizer.cli.errorAndExitJvmProcess
import org.jetbrains.kotlin.commonizer.konan.NativeLibrary
import org.jetbrains.kotlin.library.ToolingSingleFileKlibResolveStrategy
import org.jetbrains.kotlin.library.metadata.KlibMetadataVersion
@@ -29,11 +30,11 @@ internal class DefaultNativeLibraryLoader(
)
if (library.versions.metadataVersion == null)
logger.fatal("Library does not have metadata version specified in manifest: $file")
logger.errorAndExitJvmProcess("Library does not have metadata version specified in manifest: $file")
val metadataVersion = library.metadataVersion
if (metadataVersion?.isCompatibleWithCurrentCompilerVersion() != true)
logger.fatal(
logger.errorAndExitJvmProcess(
"""
Library has incompatible metadata version ${metadataVersion ?: "\"unknown\""}: $file
Please make sure that all libraries passed to commonizer compatible metadata version ${KlibMetadataVersion.INSTANCE}
@@ -20,10 +20,11 @@ internal class CliLoggerAdapter(
override fun warning(message: String) = printlnIndented("Warning: $message", *CommonizerLogLevel.values())
override fun error(message: String) = fatal(message)
override fun error(message: String) = printlnIndented("Error: $message\n", *CommonizerLogLevel.values())
@Deprecated(Logger.FATAL_DEPRECATION_MESSAGE, ReplaceWith(Logger.FATAL_REPLACEMENT))
override fun fatal(message: String): Nothing {
printlnIndented("Error: $message\n", *CommonizerLogLevel.values())
error(message)
exitProcess(1)
}
@@ -36,3 +37,8 @@ internal class CliLoggerAdapter(
}
}
}
internal fun Logger.errorAndExitJvmProcess(message: String): Nothing {
error(message)
exitProcess(1)
}
@@ -6,6 +6,7 @@
package org.jetbrains.kotlin.commonizer.konan
import org.jetbrains.kotlin.commonizer.*
import org.jetbrains.kotlin.commonizer.cli.errorAndExitJvmProcess
import org.jetbrains.kotlin.commonizer.repository.Repository
import org.jetbrains.kotlin.commonizer.stats.StatsCollector
import org.jetbrains.kotlin.commonizer.utils.progress
@@ -94,8 +95,8 @@ internal class LibraryCommonizer internal constructor(
private fun checkPreconditions() {
outputTargets.forEach { outputTarget ->
when (outputTarget.allLeaves().size) {
0 -> logger.fatal("No targets specified")
1 -> logger.fatal("Too few targets specified: $outputTarget")
0 -> logger.errorAndExitJvmProcess("No targets specified")
1 -> logger.errorAndExitJvmProcess("Too few targets specified: $outputTarget")
}
}
}