Skip to content

Commit

Permalink
More logger cleanups (#4658)
Browse files Browse the repository at this point in the history
Another part of #3603

More narrowing of the `Logger` interface so we can figure out what
really needs to be there

- Moved `colored`, `infoColor`, `errorColor` to the `prompt` object
since it's not typically overriden
- Inlined the `subLogger` calls at its one call-site
  • Loading branch information
lihaoyi authored Mar 6, 2025
1 parent 44fd0a8 commit 0fbf041
Show file tree
Hide file tree
Showing 15 changed files with 54 additions and 66 deletions.
1 change: 0 additions & 1 deletion bsp/src/mill/bsp/BspContext.scala
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ private[mill] class BspContext(
canReload: Boolean
): Result[BspServerHandle] = {
val log: Logger = new Logger {
override def colored: Boolean = false
override def streams: SystemStreams = new SystemStreams(
out = streams0.out,
err = streams0.err,
Expand Down
2 changes: 0 additions & 2 deletions bsp/worker/src/mill/bsp/worker/MillBspLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import mill.internal.ProxyLogger
private class MillBspLogger(client: BuildClient, taskId: Int, logger: Logger)
extends ProxyLogger(logger)
with Logger {
override def infoColor = fansi.Color.Blue
override def errorColor = fansi.Color.Red

override def ticker(s: String): Unit = {
try {
Expand Down
17 changes: 10 additions & 7 deletions core/api/src/mill/api/Logger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ import java.io.PrintStream
* used to display the final `show` output for easy piping.
*/
trait Logger {
def infoColor: fansi.Attrs = fansi.Attrs.Empty
def errorColor: fansi.Attrs = fansi.Attrs.Empty
def colored: Boolean

private[mill] def unprefixedStreams: SystemStreams = streams
def streams: SystemStreams
Expand All @@ -39,10 +36,7 @@ trait Logger {

private[mill] def prompt: Logger.Prompt

private[mill] def subLogger(path: os.Path, keySuffix: String, message: String): Logger =
this

private[mill] def withPromptLine[T](t: => T): T = {
private[mill] final def withPromptLine[T](t: => T): T = {
prompt.setPromptLine(logPrefixKey, keySuffix, message)
try t
finally prompt.removePromptLine(logPrefixKey)
Expand All @@ -63,6 +57,7 @@ object Logger {
* to logger unchanged without any customization.
*/
trait Prompt {

private[mill] def setPromptDetail(key: Seq[String], s: String): Unit
private[mill] def reportKey(key: Seq[String]): Unit
private[mill] def setPromptLine(key: Seq[String], keySuffix: String, message: String): Unit
Expand All @@ -75,6 +70,11 @@ object Logger {
def debugEnabled: Boolean

def enableTicker: Boolean

def infoColor: fansi.Attrs

def errorColor: fansi.Attrs
def colored: Boolean
}
object Prompt {
class NoOp extends Prompt {
Expand All @@ -91,6 +91,9 @@ object Logger {
def debugEnabled: Boolean = false

def enableTicker: Boolean = false
def infoColor: fansi.Attrs = fansi.Attrs.Empty
def errorColor: fansi.Attrs = fansi.Attrs.Empty
def colored: Boolean = false
}
}
}
7 changes: 6 additions & 1 deletion core/exec/src/mill/exec/ExecutionContexts.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package mill.exec

import mill.internal.{FileLogger, MultiLogger, PrefixLogger}
import os.Path

import scala.concurrent.{Await, Future}
Expand Down Expand Up @@ -84,7 +85,11 @@ private object ExecutionContexts {
def async[T](dest: Path, key: String, message: String)(t: => T)(implicit
ctx: mill.api.Ctx
): Future[T] = {
val logger = ctx.log.subLogger(dest / os.up / s"${dest.last}.log", key, message)
val logger = new MultiLogger(
new PrefixLogger(ctx.log, Seq(key), ctx.log.keySuffix, message),
new FileLogger(dest / os.up / s"${dest.last}.log", false),
ctx.log.streams.in
)

var destInitialized: Boolean = false
def makeDest() = synchronized {
Expand Down
3 changes: 1 addition & 2 deletions core/exec/src/mill/exec/GroupExecution.scala
Original file line number Diff line number Diff line change
Expand Up @@ -377,9 +377,8 @@ private trait GroupExecution {
logPath match {
case None => (logger, None)
case Some(path) =>
val fileLogger = new FileLogger(logger.colored, path)
val fileLogger = new FileLogger(path)
val multiLogger = new MultiLogger(
logger.colored,
logger,
fileLogger,
logger.streams.in
Expand Down
5 changes: 0 additions & 5 deletions core/internal/src/mill/internal/FileLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import java.io.{OutputStream, PrintStream}
import java.nio.file.{Files, StandardOpenOption}

private[mill] class FileLogger(
override val colored: Boolean,
file: os.Path,
append: Boolean = false
) extends Logger with AutoCloseable {
Expand Down Expand Up @@ -52,9 +51,5 @@ private[mill] class FileLogger(
if (outputStreamUsed)
streams.out.close()
}
def enableTicker = false
def prompt = new Logger.Prompt.NoOp
override def subLogger(path: os.Path, keySuffix: String, message: String): Logger = {
new FileLogger(colored, path, append)
}
}
18 changes: 5 additions & 13 deletions core/internal/src/mill/internal/MultiLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import mill.api.{Logger, SystemStreams}
import java.io.{InputStream, PrintStream}

private[mill] class MultiLogger(
val colored: Boolean,
val logger1: Logger,
val logger2: Logger,
val inStream0: InputStream
Expand Down Expand Up @@ -84,28 +83,21 @@ private[mill] class MultiLogger(
override def enableTicker: Boolean = logger1.prompt.enableTicker || logger2.prompt.enableTicker

override def debugEnabled: Boolean = logger1.prompt.debugEnabled || logger2.prompt.debugEnabled

override def infoColor: Attrs = logger1.prompt.infoColor ++ logger2.prompt.infoColor

override def errorColor: Attrs = logger1.prompt.errorColor ++ logger2.prompt.errorColor
override def colored: Boolean = logger1.prompt.colored || logger2.prompt.colored
}
def debug(s: String): Unit = {
logger1.debug(s)
logger2.debug(s)
}

private[mill] override def subLogger(path: os.Path, key: String, message: String): Logger = {
new MultiLogger(
colored,
logger1.subLogger(path, key, message),
logger2.subLogger(path, key, message),
inStream0
)
}

override def infoColor: Attrs = logger1.infoColor ++ logger2.infoColor
override def errorColor: Attrs = logger1.errorColor ++ logger2.errorColor
private[mill] override def logPrefixKey = logger1.logPrefixKey ++ logger2.logPrefixKey

override def withOutStream(outStream: PrintStream): Logger = {
new MultiLogger(
colored,
logger1.withOutStream(outStream),
logger2.withOutStream(outStream),
inStream0
Expand Down
27 changes: 9 additions & 18 deletions core/internal/src/mill/internal/PrefixLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import java.io.PrintStream
*
* Generates log lines of the form
*
* [$logPrefixKey/$keySuffix] $message
* [$logPrefixKey] ...logs...
* [$logPrefixKey] ...logs...
* [$logPrefixKey] ...logs...
* [$parentKeys-$key0/$keySuffix] $message
* [$parentKeys-$key0] ...logs...
* [$parentKeys-$key0] ...logs...
* [$parentKeys-$key0] ...logs...
*
* And a prompt line of the form
*
* [$logPrefixKey] $message
* [$parentKeys-$key0] $message
*/
private[mill] class PrefixLogger(
val logger0: Logger,
Expand All @@ -36,14 +36,9 @@ private[mill] class PrefixLogger(
override def toString: String =
s"PrefixLogger($logger0, $key0)"

override def colored = logger0.colored

override def infoColor = logger0.infoColor
override def errorColor = logger0.errorColor

def prefixPrintStream(stream: java.io.OutputStream) = {
new PrintStream(new LinePrefixOutputStream(
infoColor(linePrefix).render,
prompt.infoColor(linePrefix).render,
stream,
() => prompt.reportKey(logPrefixKey)
))
Expand All @@ -62,19 +57,19 @@ private[mill] class PrefixLogger(

override def info(s: String): Unit = {
prompt.reportKey(logPrefixKey)
logger0.info("" + infoColor(linePrefix) + s)
logger0.info("" + prompt.infoColor(linePrefix) + s)
}
override def error(s: String): Unit = {
prompt.reportKey(logPrefixKey)
logger0.error("" + infoColor(linePrefix) + s)
logger0.error("" + prompt.infoColor(linePrefix) + s)
}
override def ticker(s: String): Unit = prompt.setPromptDetail(logPrefixKey, s)

def prompt = logger0.prompt

override def debug(s: String): Unit = {
if (prompt.debugEnabled) prompt.reportKey(logPrefixKey)
logger0.debug("" + infoColor(linePrefix) + s)
logger0.debug("" + prompt.infoColor(linePrefix) + s)
}
override def withOutStream(outStream: PrintStream): Logger = new ProxyLogger(this) with Logger {
override lazy val unprefixedStreams = new SystemStreams(
Expand All @@ -89,8 +84,4 @@ private[mill] class PrefixLogger(
PrefixLogger.this.streams.in
)
}

private[mill] override def subLogger(path: os.Path, subKey: String, message: String): Logger = {
new PrefixLogger(this, Seq(subKey), keySuffix, message)
}
}
11 changes: 8 additions & 3 deletions core/internal/src/mill/internal/PrintLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import mill.api.{Logger, SystemStreams}
import java.io.*

private[mill] class PrintLogger(
override val colored: Boolean,
colored: Boolean,
enableTicker: Boolean,
override val infoColor: fansi.Attrs,
override val errorColor: fansi.Attrs,
infoColor: fansi.Attrs,
errorColor: fansi.Attrs,
val streams: SystemStreams,
debugEnabled: Boolean,
val context: String,
Expand All @@ -32,6 +32,11 @@ private[mill] class PrintLogger(
}

override def enableTicker: Boolean = PrintLogger.this.enableTicker

override def infoColor: fansi.Attrs = PrintLogger.this.infoColor

override def errorColor: fansi.Attrs = PrintLogger.this.errorColor
override def colored: Boolean = PrintLogger.this.colored
}
def ticker(s: String): Unit = synchronized {
if (enableTicker) {
Expand Down
10 changes: 7 additions & 3 deletions core/internal/src/mill/internal/PromptLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ import java.io.*
* buffer to be read out and handled asynchronously.
*/
private[mill] class PromptLogger(
override val colored: Boolean,
colored: Boolean,
enableTicker: Boolean,
override val infoColor: fansi.Attrs,
override val errorColor: fansi.Attrs,
infoColor: fansi.Attrs,
errorColor: fansi.Attrs,
systemStreams0: SystemStreams,
debugEnabled: Boolean,
titleText: String,
Expand Down Expand Up @@ -145,6 +145,10 @@ private[mill] class PromptLogger(

def enableTicker = PromptLogger.this.enableTicker
def debugEnabled = PromptLogger.this.debugEnabled

def infoColor: fansi.Attrs = PromptLogger.this.infoColor
def errorColor: fansi.Attrs = PromptLogger.this.errorColor
def colored: Boolean = PromptLogger.this.colored
}
def ticker(s: String): Unit = ()

Expand Down
3 changes: 0 additions & 3 deletions core/internal/src/mill/internal/ProxyLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import mill.api.{Logger, SystemStreams}
*/
private[mill] class ProxyLogger(logger: Logger) extends Logger {
override def toString: String = s"ProxyLogger($logger)"
def colored = logger.colored

lazy val streams = logger.streams

Expand All @@ -19,8 +18,6 @@ private[mill] class ProxyLogger(logger: Logger) extends Logger {

def prompt = logger.prompt

override def infoColor: fansi.Attrs = logger.infoColor
override def errorColor: fansi.Attrs = logger.errorColor
private[mill] override def logPrefixKey: Seq[String] = logger.logPrefixKey
private[mill] override def unprefixedStreams: SystemStreams = logger.unprefixedStreams
}
2 changes: 1 addition & 1 deletion pythonlib/src/mill/pythonlib/PythonModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ trait PythonModule extends PipModule with TaskModule { outer =>
Map(
"PYTHONPATH" -> transitivePythonPath().map(_.path).mkString(java.io.File.pathSeparator),
"PYTHONPYCACHEPREFIX" -> (Task.dest / "cache").toString,
if (Task.log.colored) { "FORCE_COLOR" -> "1" }
if (Task.log.prompt.colored) { "FORCE_COLOR" -> "1" }
else { "NO_COLOR" -> "1" }
)
}
Expand Down
2 changes: 1 addition & 1 deletion scalalib/src/mill/scalalib/TestModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ trait TestModule
arguments = args(),
sysProps = Map.empty,
outputPath = outputPath,
colored = Task.log.colored,
colored = Task.log.prompt.colored,
testCp = testClasspath().map(_.path),
globSelectors = selectors
)
Expand Down
2 changes: 1 addition & 1 deletion scalalib/src/mill/scalalib/TestModuleUtil.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ private[scalalib] object TestModuleUtil {
arguments = args,
sysProps = props,
outputPath = outputPath,
colored = Task.log.colored,
colored = Task.log.prompt.colored,
testCp = testClasspath.map(_.path),
globSelectors = selectors2
)
Expand Down
10 changes: 5 additions & 5 deletions scalalib/worker/src/mill/scalalib/worker/ZincWorkerImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,8 @@ class ZincWorkerImpl(
val consoleAppender = ConsoleAppender(
"ZincLogAppender",
ConsoleOut.printStreamOut(ctx.log.streams.err),
ctx.log.colored,
ctx.log.colored,
ctx.log.prompt.colored,
ctx.log.prompt.colored,
_ => None
)
val loggerId = Thread.currentThread().getId.toString
Expand All @@ -548,12 +548,12 @@ class ZincWorkerImpl(
def mkNewReporter(mapper: (xsbti.Position => xsbti.Position) | Null) = reporter match {
case None =>
new ManagedLoggedReporter(10, logger) with RecordingReporter
with TransformingReporter(ctx.log.colored, mapper) {}
with TransformingReporter(ctx.log.prompt.colored, mapper) {}
case Some(forwarder) =>
new ManagedLoggedReporter(10, logger)
with ForwardingReporter(forwarder)
with RecordingReporter
with TransformingReporter(ctx.log.colored, mapper) {}
with TransformingReporter(ctx.log.prompt.colored, mapper) {}
}
val analysisMap0 = upstreamCompileOutput.map(c => c.classes.path -> c.analysisFile).toMap

Expand Down Expand Up @@ -645,7 +645,7 @@ class ZincWorkerImpl(
val scalaColorProp = "scala.color"
val previousScalaColor = sys.props(scalaColorProp)
try {
sys.props(scalaColorProp) = if (ctx.log.colored) "true" else "false"
sys.props(scalaColorProp) = if (ctx.log.prompt.colored) "true" else "false"
val newResult = ic.compile(
in = inputs,
logger = logger
Expand Down

0 comments on commit 0fbf041

Please sign in to comment.