From ee1d32bf88a5160584ed5bbce7034737da878b27 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Thu, 6 Mar 2025 13:48:30 +0800 Subject: [PATCH] Remove PrintLogger (#4661) Part of https://github.com/com-lihaoyi/mill/issues/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` --- core/api/src/mill/api/Logger.scala | 2 +- core/exec/src/mill/exec/GroupExecution.scala | 4 +- .../src/mill/internal/PrefixLogger.scala | 3 +- .../src/mill/internal/PrintLogger.scala | 95 ------------------- .../src/mill/internal/PromptLogger.scala | 22 +++-- .../tasks/7-forking-futures/build.mill | 8 +- .../full-run-logs/src/FullRunLogsTests.scala | 88 +++++++---------- runner/src/mill/runner/MillCliConfig.scala | 4 +- runner/src/mill/runner/MillMain.scala | 45 +++------ testkit/src/mill/testkit/UnitTester.scala | 14 +-- .../src/mill/testrunner/TestRunnerMain0.scala | 20 +--- .../src/mill/testrunner/TestRunnerUtils.scala | 27 +++--- 12 files changed, 95 insertions(+), 237 deletions(-) delete mode 100644 core/internal/src/mill/internal/PrintLogger.scala diff --git a/core/api/src/mill/api/Logger.scala b/core/api/src/mill/api/Logger.scala index b24200e6e71..dd393198947 100644 --- a/core/api/src/mill/api/Logger.scala +++ b/core/api/src/mill/api/Logger.scala @@ -69,7 +69,7 @@ object Logger { def debugEnabled: Boolean - def enableTicker: Boolean + private[mill] def enableTicker: Boolean def infoColor: fansi.Attrs diff --git a/core/exec/src/mill/exec/GroupExecution.scala b/core/exec/src/mill/exec/GroupExecution.scala index d551a56055e..81b106cbc58 100644 --- a/core/exec/src/mill/exec/GroupExecution.scala +++ b/core/exec/src/mill/exec/GroupExecution.scala @@ -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") } } diff --git a/core/internal/src/mill/internal/PrefixLogger.scala b/core/internal/src/mill/internal/PrefixLogger.scala index b71536fe083..fba0a3752f5 100644 --- a/core/internal/src/mill/internal/PrefixLogger.scala +++ b/core/internal/src/mill/internal/PrefixLogger.scala @@ -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)" diff --git a/core/internal/src/mill/internal/PrintLogger.scala b/core/internal/src/mill/internal/PrintLogger.scala deleted file mode 100644 index 5cb2162d19b..00000000000 --- a/core/internal/src/mill/internal/PrintLogger.scala +++ /dev/null @@ -1,95 +0,0 @@ -package mill.internal - -import mill.api.{Logger, SystemStreams} - -import java.io.* - -private[mill] class PrintLogger( - colored: Boolean, - enableTicker: Boolean, - infoColor: fansi.Attrs, - errorColor: fansi.Attrs, - val streams: SystemStreams, - debugEnabled: Boolean, - val context: String, - printLoggerState: PrintLogger.State -) extends Logger with AutoCloseable { - def close() = () // do nothing - override def toString: String = s"PrintLogger($colored, $enableTicker)" - def info(s: String): Unit = synchronized { - printLoggerState.value = PrintLogger.State.Newline - streams.err.println(infoColor(context + s)) - } - - def error(s: String): Unit = synchronized { - printLoggerState.value = PrintLogger.State.Newline - streams.err.println((infoColor(context) ++ errorColor(s)).render) - } - - def prompt = new Logger.Prompt.NoOp { - override def setPromptDetail(key: Seq[String], s: String): Unit = synchronized { - ticker(s) - } - - 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) { - printLoggerState.value match { - case PrintLogger.State.Newline => - streams.err.println(infoColor(s)) - case PrintLogger.State.Middle => - streams.err.println() - streams.err.println(infoColor(s)) - case PrintLogger.State.Ticker => - val p = new PrintWriter(streams.err) - // Need to make this more "atomic" - val nav = new AnsiNav(p) - nav.up(1) - nav.clearLine(2) - nav.left(9999) - p.flush() - - streams.err.println(infoColor(s)) - } - printLoggerState.value = PrintLogger.State.Ticker - } - } - - override def withOutStream(outStream: PrintStream): PrintLogger = new PrintLogger( - colored, - enableTicker, - infoColor, - errorColor, - new SystemStreams(outStream, streams.err, streams.in), - debugEnabled, - context, - printLoggerState - ) - - def debug(s: String): Unit = synchronized { - if (debugEnabled) { - printLoggerState.value = PrintLogger.State.Newline - streams.err.println(context + s) - } - } -} - -private[mill] object PrintLogger { - - class State { - var value: State.Value = State.Newline - } - - object State { - sealed trait Value - case object Ticker extends Value - case object Newline extends Value - case object Middle extends Value - } -} diff --git a/core/internal/src/mill/internal/PromptLogger.scala b/core/internal/src/mill/internal/PromptLogger.scala index 3161ec2745a..5c0ef03ca6a 100644 --- a/core/internal/src/mill/internal/PromptLogger.scala +++ b/core/internal/src/mill/internal/PromptLogger.scala @@ -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) + } } } diff --git a/example/fundamentals/tasks/7-forking-futures/build.mill b/example/fundamentals/tasks/7-forking-futures/build.mill index 2d2d133990a..5668dd67f3f 100644 --- a/example/fundamentals/tasks/7-forking-futures/build.mill +++ b/example/fundamentals/tasks/7-forking-futures/build.mill @@ -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 */ diff --git a/integration/feature/full-run-logs/src/FullRunLogsTests.scala b/integration/feature/full-run-logs/src/FullRunLogsTests.scala index 0f16bdf462c..9940d5b0646 100644 --- a/integration/feature/full-run-logs/src/FullRunLogsTests.scala +++ b/integration/feature/full-run-logs/src/FullRunLogsTests.scala @@ -14,12 +14,13 @@ object FullRunLogsTests extends UtestIntegrationTestSuite { import tester._ val res = eval(("--ticker", "false", "run", "--text", "hello")) + res.isSuccess ==> true assert(res.out == "

hello

") 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") ) @@ -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""" jar + |[build.mill-/] compile + |[build.mill-] [info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ... + |[build.mill-] [info] done compiling + |[/] compile + |[] [info] compiling 1 Java source to ${tester.workspacePath}/out/compile.dest/classes ... + |[] [error] ${tester.workspacePath}/src/foo/Foo.java:36:10: reached end of file while parsing + |[] compile failed + |[/, 1 failed] jar s + |1 tasks failed + |compile javac returned non-zero exit code""" + .stripMargin + .replaceAll("(\r\n)|\r", "\n") + .replace('\\', '/') + ) + .replace("", "\\E\\d+\\Q") + .replace("", "\\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 @@ -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) - } } } diff --git a/runner/src/mill/runner/MillCliConfig.scala b/runner/src/mill/runner/MillCliConfig.scala index e1b7a1d337b..3530bb1ae35 100644 --- a/runner/src/mill/runner/MillCliConfig.scala +++ b/runner/src/mill/runner/MillCliConfig.scala @@ -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( diff --git a/runner/src/mill/runner/MillMain.scala b/runner/src/mill/runner/MillMain.scala index fbc58a503c5..a3110d8428f 100644 --- a/runner/src/mill/runner/MillMain.scala +++ b/runner/src/mill/runner/MillMain.scala @@ -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 @@ -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) { @@ -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 @@ -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() + ) } /** diff --git a/testkit/src/mill/testkit/UnitTester.scala b/testkit/src/mill/testkit/UnitTester.scala index 9febf0235ba..627c80ba683 100644 --- a/testkit/src/mill/testkit/UnitTester.scala +++ b/testkit/src/mill/testkit/UnitTester.scala @@ -5,7 +5,6 @@ import mill.api.ExecResult.OuterStack import mill.api.{DummyInputStream, ExecResult, Result, SystemStreams, Val} import mill.define.{Evaluator, InputImpl, SelectMode, TargetImpl} import mill.resolve.Resolve -import mill.internal.PrintLogger import mill.exec.JsonArrayLogger import mill.constants.OutFiles.{millChromeProfile, millProfile} @@ -66,15 +65,16 @@ class UnitTester( } } - object logger extends mill.internal.PrintLogger( + object logger extends mill.internal.PromptLogger( colored = true, enableTicker = false, - mill.internal.Colors.Default.info, - mill.internal.Colors.Default.error, - new SystemStreams(out = outStream, err = errStream, in = inStream), + infoColor = mill.internal.Colors.Default.info, + errorColor = mill.internal.Colors.Default.error, + systemStreams0 = new SystemStreams(out = outStream, err = errStream, in = inStream), debugEnabled = debugEnabled, - context = "", - new PrintLogger.State() + titleText = "", + terminfoPath = os.temp(), + currentTimeMillis = () => System.currentTimeMillis() ) { val prefix: String = { val idx = fullName.value.lastIndexOf(".") diff --git a/testrunner/src/mill/testrunner/TestRunnerMain0.scala b/testrunner/src/mill/testrunner/TestRunnerMain0.scala index f1bdde3309e..7315929202d 100644 --- a/testrunner/src/mill/testrunner/TestRunnerMain0.scala +++ b/testrunner/src/mill/testrunner/TestRunnerMain0.scala @@ -1,27 +1,11 @@ package mill.testrunner -import mill.api.{Ctx, DummyTestReporter, SystemStreams, internal} -import mill.internal.PrintLogger +import mill.api.{DummyTestReporter, internal} @internal object TestRunnerMain0 { def main0(args: Array[String], classLoader: ClassLoader): Unit = { try { val testArgs = upickle.default.read[mill.testrunner.TestArgs](os.read(os.Path(args(1)))) - val ctx = new Ctx.Log { - val log = new PrintLogger( - testArgs.colored, - true, - if (testArgs.colored) fansi.Color.Blue - else fansi.Attrs.Empty, - if (testArgs.colored) fansi.Color.Red - else fansi.Attrs.Empty, - new SystemStreams(System.out, System.err, System.in), - debugEnabled = false, - context = "", - new PrintLogger.State() - ) - } - ctx.log.debug(s"Setting ${testArgs.sysProps.size} system properties") testArgs.sysProps.foreach { case (k, v) => System.setProperty(k, v) } val filter = TestRunnerUtils.globFilter(testArgs.globSelectors) @@ -33,7 +17,7 @@ import mill.internal.PrintLogger classFilter = cls => filter(cls.getName), cl = classLoader, testReporter = DummyTestReporter - )(ctx) + ) // Clear interrupted state in case some badly-behaved test suite // dirtied the thread-interrupted flag and forgot to clean up. Otherwise, diff --git a/testrunner/src/mill/testrunner/TestRunnerUtils.scala b/testrunner/src/mill/testrunner/TestRunnerUtils.scala index 0c02cb132f0..c9852f36e6b 100644 --- a/testrunner/src/mill/testrunner/TestRunnerUtils.scala +++ b/testrunner/src/mill/testrunner/TestRunnerUtils.scala @@ -1,6 +1,6 @@ package mill.testrunner -import mill.api.{Ctx, TestReporter, internal} +import mill.api.{TestReporter, internal} import os.Path import sbt.testing._ @@ -129,9 +129,14 @@ import scala.jdk.CollectionConverters.IteratorHasAsScala (runner, tasks) } - def runTasks(tasks: Seq[Task], testReporter: TestReporter, runner: Runner)(implicit - ctx: Ctx.Log + def runTasks( + tasks: Seq[Task], + testReporter: TestReporter, + runner: Runner ): (String, Iterator[TestResult]) = { + // Capture this value outside of the task event handler so it + // isn't affected by a test framework's stream redirects + val systemOut = System.out val events = new ConcurrentLinkedQueue[Event]() val doneMessage = { @@ -146,12 +151,12 @@ import scala.jdk.CollectionConverters.IteratorHasAsScala } }, Array(new Logger { - def debug(msg: String) = ctx.log.streams.out.println(msg) - def error(msg: String) = ctx.log.streams.out.println(msg) + def debug(msg: String) = systemOut.println(msg) + def error(msg: String) = systemOut.println(msg) def ansiCodesSupported() = true - def warn(msg: String) = ctx.log.streams.out.println(msg) - def trace(t: Throwable) = t.printStackTrace(ctx.log.streams.out) - def info(msg: String) = ctx.log.streams.out.println(msg) + def warn(msg: String) = systemOut.println(msg) + def trace(t: Throwable) = t.printStackTrace(systemOut) + def info(msg: String) = systemOut.println(msg) }) ) @@ -162,9 +167,9 @@ import scala.jdk.CollectionConverters.IteratorHasAsScala if (doneMessage != null && doneMessage.nonEmpty) { if (doneMessage.endsWith("\n")) - ctx.log.streams.out.print(doneMessage) + println(doneMessage.stripSuffix("\n")) else - ctx.log.streams.out.println(doneMessage) + println(doneMessage) } val results = for (e <- events.iterator().asScala) yield { @@ -197,7 +202,7 @@ import scala.jdk.CollectionConverters.IteratorHasAsScala classFilter: Class[?] => Boolean, cl: ClassLoader, testReporter: TestReporter - )(implicit ctx: Ctx.Log): (String, Seq[TestResult]) = { + ): (String, Seq[TestResult]) = { val framework = frameworkInstances(cl)