Skip to content

Commit

Permalink
Remove PrintLogger (#4661)
Browse files Browse the repository at this point in the history
Part of #3603

We've been using the new multi-line `PromptLogger` for a while since
0.12.0 was released back in Aug 2024, and it's worked great. So we can
remove the old implementation for 0.13.0 due in mid 2025, and deprecate
and hide the `--remove-prompt` flag that was introduced to flip back to
the old implementation.

It turns out that we were still using the old `PrintLogger` in a bunch
of places - unit tests, testrunner, etc. - and I had to replace those
use sites either with `PromptLogger` or with just `println`

Also made some adjustments to `enableTicker` handling to keep it
internal within `Logger` so it is more consistently applied, and fixed
the logging of skipped tasks in `--keep-going`
  • Loading branch information
lihaoyi authored Mar 6, 2025
1 parent 0fbf041 commit ee1d32b
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 237 deletions.
2 changes: 1 addition & 1 deletion core/api/src/mill/api/Logger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ object Logger {

def debugEnabled: Boolean

def enableTicker: Boolean
private[mill] def enableTicker: Boolean

def infoColor: fansi.Attrs

Expand Down
4 changes: 2 additions & 2 deletions core/exec/src/mill/exec/GroupExecution.scala
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,9 @@ private trait GroupExecution {
fileLoggerOpt.foreach(_.close())

if (!failFast) maybeTargetLabel.foreach { targetLabel =>
val taskFailed = newResults.exists(task => !task._2.isInstanceOf[Success[?]])
val taskFailed = newResults.exists(task => task._2.isInstanceOf[ExecResult.Failing[?]])
if (taskFailed) {
logger.error(s"[$counterMsg] $targetLabel failed")
logger.error(s"$targetLabel failed")
}
}

Expand Down
3 changes: 2 additions & 1 deletion core/internal/src/mill/internal/PrefixLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ private[mill] class PrefixLogger(

assert(key0.forall(_.nonEmpty))
val linePrefix: String =
if (noPrefix || logPrefixKey.isEmpty) "" else s"[${logPrefixKey.mkString("-")}] "
if (noPrefix || logPrefixKey.isEmpty || !prompt.enableTicker) ""
else s"[${logPrefixKey.mkString("-")}] "
override def toString: String =
s"PrefixLogger($logger0, $key0)"

Expand Down
95 changes: 0 additions & 95 deletions core/internal/src/mill/internal/PrintLogger.scala

This file was deleted.

22 changes: 13 additions & 9 deletions core/internal/src/mill/internal/PromptLogger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -305,15 +305,19 @@ private[mill] object PromptLogger {
promptShown = false
}

// Clear each line as they are drawn, rather than relying on clearing
// the entire screen before each batch of writes, to try and reduce the
// amount of terminal flickering in slow terminals (e.g. windows)
// https://stackoverflow.com/questions/71452837/how-to-reduce-flicker-in-terminal-re-drawing
dest.write(
new String(buf, 0, end)
.replaceAll("(\r\n|\n|\t)", AnsiNav.clearLine(0) + "$1")
.getBytes
)
if (enableTicker) {
// Clear each line as they are drawn, rather than relying on clearing
// the entire screen before each batch of writes, to try and reduce the
// amount of terminal flickering in slow terminals (e.g. windows)
// https://stackoverflow.com/questions/71452837/how-to-reduce-flicker-in-terminal-re-drawing
dest.write(
new String(buf, 0, end)
.replaceAll("(\r\n|\n|\t)", AnsiNav.clearLine(0) + "$1")
.getBytes
)
} else {
dest.write(new String(buf, 0, end).getBytes)
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions example/fundamentals/tasks/7-forking-futures/build.mill
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ def taskSpawningFutures = Task {
/** Usage

> ./mill show taskSpawningFutures
[1] Running First Future inside .../out/taskSpawningFutures.dest/future-1
[2] Running Second Future inside .../out/taskSpawningFutures.dest/future-2
[1] Finished First Future
[2] Finished Second Future
Running First Future inside .../out/taskSpawningFutures.dest/future-1
Running Second Future inside .../out/taskSpawningFutures.dest/future-2
Finished First Future
Finished Second Future
3

*/
Expand Down
88 changes: 33 additions & 55 deletions integration/feature/full-run-logs/src/FullRunLogsTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ object FullRunLogsTests extends UtestIntegrationTestSuite {
import tester._

val res = eval(("--ticker", "false", "run", "--text", "hello"))

res.isSuccess ==> true
assert(res.out == "<h1>hello</h1>")
assert(
res.err.replace('\\', '/').replaceAll("(\r\n)|\r", "\n") ==
s"""[build.mill] [info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ...
|[build.mill] [info] done compiling
s"""[info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ...
|[info] done compiling
|[info] compiling 1 Java source to ${tester.workspacePath}/out/compile.dest/classes ...
|[info] done compiling""".stripMargin.replace('\\', '/').replaceAll("(\r\n)|\r", "\n")
)
Expand Down Expand Up @@ -52,6 +53,36 @@ object FullRunLogsTests extends UtestIntegrationTestSuite {
val normErr = res.err.replace('\\', '/').replaceAll("(\r\n)|\r", "\n")
assert(expectedErrorRegex.r.matches(normErr))
}
test("keepGoingFailure") - integrationTest { tester =>
import tester._

modifyFile(workspacePath / "src/foo/Foo.java", _ + "class Bar")
val res = eval(("--ticker", "true", "--keep-going", "jar"))
res.isSuccess ==> false

val expectedErrorRegex = java.util.regex.Pattern
.quote(
s"""<dashes> jar <dashes>
|[build.mill-<digits>/<digits>] compile
|[build.mill-<digits>] [info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ...
|[build.mill-<digits>] [info] done compiling
|[<digits>/<digits>] compile
|[<digits>] [info] compiling 1 Java source to ${tester.workspacePath}/out/compile.dest/classes ...
|[<digits>] [error] ${tester.workspacePath}/src/foo/Foo.java:36:10: reached end of file while parsing
|[<digits>] compile failed
|[<digits>/<digits>, 1 failed] <dashes> jar <dashes> <digits>s
|1 tasks failed
|compile javac returned non-zero exit code"""
.stripMargin
.replaceAll("(\r\n)|\r", "\n")
.replace('\\', '/')
)
.replace("<digits>", "\\E\\d+\\Q")
.replace("<dashes>", "\\E=+\\Q")

val normErr = res.err.replace('\\', '/').replaceAll("(\r\n)|\r", "\n")
assert(expectedErrorRegex.r.matches(normErr))
}
test("show") - integrationTest { tester =>
import tester._
// Make sure when we have nested evaluations, e.g. due to usage of evaluator commands
Expand All @@ -74,58 +105,5 @@ object FullRunLogsTests extends UtestIntegrationTestSuite {
assert(millProfile.exists(_.obj("label").str == "show"))
assert(millChromeProfile.exists(_.obj("name").str == "show"))
}
test("failedTasksCounter") - integrationTest { tester =>
import tester._

val javaSource = os.Path(workspacePath, os.pwd)
val fooJava = javaSource / "src" / "foo" / "Foo.java"
val originalContent = os.read(fooJava)

// Introduce a compilation error by adding an unclosed brace
modifyFile(fooJava, _.replace("class Foo", "class Foo {"))

// Run with ticker to see the failed tasks count for compile
val compileRes = eval(("--ticker", "true", "compile"))
compileRes.isSuccess ==> false

// Verify the output shows failed tasks count in the progress indicator
val failedPattern = "\\[(\\d+)/(\\d+),\\s*(\\d+)\\s*failed\\]".r // Matches [X/Y, N failed]
val failedMatch = failedPattern.findFirstIn(compileRes.err)
failedMatch.isDefined ==> true

// Extract and verify the number of failed tasks
val failedCount =
failedPattern.findFirstMatchIn(compileRes.err).map(_.group(3).toInt).getOrElse(0)
failedCount ==> 1 // Expecting 1 failed task for compile

// Run jar task with --keep-going to see upstream failures
val jarKeepGoingRes = eval(("--ticker", "true", "--keep-going", "jar"))
jarKeepGoingRes.isSuccess ==> false

// Verify the output shows failed tasks count in the progress indicator
val jarKeepGoingFailedMatch = failedPattern.findFirstIn(jarKeepGoingRes.err)
jarKeepGoingFailedMatch.isDefined ==> true

// Extract and verify the number of failed tasks
val jarKeepGoingFailedCount =
failedPattern.findFirstMatchIn(jarKeepGoingRes.err).map(_.group(3).toInt).getOrElse(0)
jarKeepGoingFailedCount ==> 1 // Expecting 1 failed task for jar with --keep-going

// Run jar task without --keep-going to see it fail immediately
val jarRes = eval(("--ticker", "true", "jar"))
jarRes.isSuccess ==> false

// Verify the output shows failed tasks count in the progress indicator
val jarFailedMatch = failedPattern.findFirstIn(jarRes.err)
jarFailedMatch.isDefined ==> true

// Extract and verify the number of failed tasks
val jarFailedCount =
failedPattern.findFirstMatchIn(jarRes.err).map(_.group(3).toInt).getOrElse(0)
jarFailedCount ==> 1 // Expecting 1 failed task for jar without --keep-going

// Restore the original content
modifyFile(fooJava, _ => originalContent)
}
}
}
4 changes: 3 additions & 1 deletion runner/src/mill/runner/MillCliConfig.scala
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,13 @@ case class MillCliConfig(
metaLevel: Option[Int] = None,
@arg(doc = "Allows command args to be passed positionally without `--arg` by default")
allowPositional: Flag = Flag(),
@deprecated("No longer used", "Mill 0.13.0")
@arg(
doc = """
Disables the new multi-line status prompt used for showing thread
status at the command line and falls back to the legacy ticker
"""
""",
hidden = true
)
disablePrompt: Flag = Flag(),
@arg(
Expand Down
45 changes: 12 additions & 33 deletions runner/src/mill/runner/MillMain.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import mill.constants.{OutFiles, ServerFiles, Util}
import mill.client.lock.Lock
import mill.main.BuildInfo
import mill.runner.worker.ScalaCompilerWorker
import mill.internal.{Colors, PrintLogger, PromptLogger}
import mill.internal.{Colors, PromptLogger}

import java.lang.reflect.InvocationTargetException
import scala.util.control.NonFatal
Expand Down Expand Up @@ -121,7 +121,6 @@ object MillMain {
systemExit: Int => Nothing,
serverDir: os.Path
): (Boolean, RunnerState) = {
val printLoggerState = new PrintLogger.State()
val streams = streams0
SystemStreams.withStreams(streams) {
os.SubProcess.env.withValue(env) {
Expand Down Expand Up @@ -261,12 +260,10 @@ object MillMain {
Using.resource(getLogger(
streams,
config,
mainInteractive,
enableTicker =
config.ticker
.orElse(config.enableTicker)
.orElse(Option.when(config.disableTicker.value)(false)),
printLoggerState,
serverDir,
colored = colored,
colors = colors
Expand Down Expand Up @@ -366,40 +363,22 @@ object MillMain {
def getLogger(
streams: SystemStreams,
config: MillCliConfig,
mainInteractive: Boolean,
enableTicker: Option[Boolean],
printLoggerState: PrintLogger.State,
serverDir: os.Path,
colored: Boolean,
colors: Colors
): Logger with AutoCloseable = {

val logger = if (config.disablePrompt.value) {
new mill.internal.PrintLogger(
colored = colored,
enableTicker = enableTicker.getOrElse(mainInteractive),
infoColor = colors.info,
errorColor = colors.error,
streams = streams,
debugEnabled = config.debugLog.value,
context = "",
printLoggerState
)
} else {
new PromptLogger(
colored = colored,
enableTicker = enableTicker.getOrElse(true),
infoColor = colors.info,
errorColor = colors.error,
systemStreams0 = streams,
debugEnabled = config.debugLog.value,
titleText = config.leftoverArgs.value.mkString(" "),
terminfoPath = serverDir / ServerFiles.terminfo,
currentTimeMillis = () => System.currentTimeMillis()
)
}

logger
new PromptLogger(
colored = colored,
enableTicker = enableTicker.getOrElse(true),
infoColor = colors.info,
errorColor = colors.error,
systemStreams0 = streams,
debugEnabled = config.debugLog.value,
titleText = config.leftoverArgs.value.mkString(" "),
terminfoPath = serverDir / ServerFiles.terminfo,
currentTimeMillis = () => System.currentTimeMillis()
)
}

/**
Expand Down
Loading

0 comments on commit ee1d32b

Please sign in to comment.