From a94ef4417893adb7dfddf012053866b98cbf26c4 Mon Sep 17 00:00:00 2001 From: eugene yokota Date: Fri, 14 Aug 2026 08:51:51 -0400 Subject: [PATCH] [2.x] fix: Forward build.sbt compilation errors to logger (#9599) **Problem** 1. build.sbt was compiled with console reporter, so we couldn't capture the error. 2. Another problem was that reload deleted global log so we couldn't get to the previous load failure. **Solution** 1. This forwards build.sbt compilation errors to the logger. 2. This retains the failed loading log. 3. Using the facility above, this adds negative test for loading. --- .../scala/sbt/internal/EvalReporter.scala | 22 ++++++++++ .../internal/LoggingEvalReporterTest.scala | 34 ++++++++++++++ main/src/main/scala/sbt/Main.scala | 10 ++++- main/src/main/scala/sbt/MainLoop.scala | 7 ++- main/src/main/scala/sbt/internal/Load.scala | 6 ++- .../neg-types/caseclass.log.check | 14 ++++++ .../project-load/neg-types/changes/bad1.sbt | 3 ++ .../project-load/neg-types/changes/bad2.sbt | 3 ++ .../project-load/neg-types/changes/bad3.sbt | 4 ++ .../project-load/neg-types/changes/good.sbt | 44 +++++++++++++++++++ .../project-load/neg-types/class.log.check | 14 ++++++ .../project-load/neg-types/enum.log.check | 16 +++++++ .../src/sbt-test/project-load/neg-types/test | 24 ++++++++++ 13 files changed, 196 insertions(+), 5 deletions(-) create mode 100644 buildfile/src/test/scala/sbt/internal/LoggingEvalReporterTest.scala create mode 100644 sbt-app/src/sbt-test/project-load/neg-types/caseclass.log.check create mode 100644 sbt-app/src/sbt-test/project-load/neg-types/changes/bad1.sbt create mode 100644 sbt-app/src/sbt-test/project-load/neg-types/changes/bad2.sbt create mode 100644 sbt-app/src/sbt-test/project-load/neg-types/changes/bad3.sbt create mode 100644 sbt-app/src/sbt-test/project-load/neg-types/changes/good.sbt create mode 100644 sbt-app/src/sbt-test/project-load/neg-types/class.log.check create mode 100644 sbt-app/src/sbt-test/project-load/neg-types/enum.log.check create mode 100644 sbt-app/src/sbt-test/project-load/neg-types/test diff --git a/buildfile/src/main/scala/sbt/internal/EvalReporter.scala b/buildfile/src/main/scala/sbt/internal/EvalReporter.scala index c0da90f1c..6e6a728c4 100644 --- a/buildfile/src/main/scala/sbt/internal/EvalReporter.scala +++ b/buildfile/src/main/scala/sbt/internal/EvalReporter.scala @@ -2,10 +2,15 @@ package sbt package internal import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.interfaces import dotty.tools.dotc.reporting.ConsoleReporter import dotty.tools.dotc.reporting.Diagnostic +import dotty.tools.dotc.reporting.HideNonSensicalMessages +import dotty.tools.dotc.reporting.MessageRendering import dotty.tools.dotc.reporting.Reporter import dotty.tools.dotc.reporting.StoreReporter +import dotty.tools.dotc.reporting.UniqueMessagePositions +import sbt.util.Logger abstract class EvalReporter extends Reporter: /** @@ -19,6 +24,7 @@ abstract class EvalReporter extends Reporter: object EvalReporter: def console: EvalReporter = ForwardingReporter(ConsoleReporter()) def store: EvalReporter = ForwardingReporter(StoreReporter()) + def logging(log: Logger): EvalReporter = LoggingEvalReporter(log) end EvalReporter class ForwardingReporter(delegate: Reporter) extends EvalReporter: @@ -26,3 +32,19 @@ class ForwardingReporter(delegate: Reporter) extends EvalReporter: def finalReport(sourceName: String): Unit = () end ForwardingReporter + +/** Forwards diagnostics to an sbt Logger, rendered as the console reporter would. */ +class LoggingEvalReporter(logger: Logger) + extends EvalReporter + with UniqueMessagePositions + with HideNonSensicalMessages + with MessageRendering: + def doReport(dia: Diagnostic)(using Context): Unit = + val text = messageAndPos(dia) + dia.level match + case interfaces.Diagnostic.ERROR => logger.error(text) + case interfaces.Diagnostic.WARNING => logger.warn(text) + case _ => logger.info(text) + + def finalReport(sourceName: String): Unit = () +end LoggingEvalReporter diff --git a/buildfile/src/test/scala/sbt/internal/LoggingEvalReporterTest.scala b/buildfile/src/test/scala/sbt/internal/LoggingEvalReporterTest.scala new file mode 100644 index 000000000..f9dedf512 --- /dev/null +++ b/buildfile/src/test/scala/sbt/internal/LoggingEvalReporterTest.scala @@ -0,0 +1,34 @@ +/* + * sbt + * Copyright 2023, Scala center + * Copyright 2011 - 2022, Lightbend, Inc. + * Copyright 2008 - 2010, Mark Harrah + * Licensed under Apache License 2.0 (see LICENSE) + */ + +package sbt +package internal + +import scala.collection.mutable.ListBuffer + +import sbt.util.Level +import sbt.util.Logger + +object LoggingEvalReporterTest extends verify.BasicTestSuite: + private def stripAnsi(s: String): String = s.replaceAll("\\[[0-9;]*m", "") + + private class RecordingLogger extends Logger: + val messages = ListBuffer.empty[(Level.Value, String)] + def log(level: Level.Value, message: => String): Unit = messages += ((level, message)) + def success(message: => String): Unit = () + def trace(t: => Throwable): Unit = () + + test("forwards compile errors to the logger") { + val log = new RecordingLogger + val eval = Eval(() => EvalReporter.logging(log)) + intercept[EvalException] { + eval.evalInfer("\"\".undefined") + } + val errors = log.messages.collect { case (Level.Error, m) => stripAnsi(m) } + assert(errors.exists(_.contains("undefined is not a member of String"))) + } diff --git a/main/src/main/scala/sbt/Main.scala b/main/src/main/scala/sbt/Main.scala index 8ded3c871..fb2017339 100644 --- a/main/src/main/scala/sbt/Main.scala +++ b/main/src/main/scala/sbt/Main.scala @@ -286,12 +286,20 @@ object StandardMain { else sys.props.get(execLogProp).map(Paths.get(_)).map(ActionCache.setExecLog) execLog.foreach: log => ShutdownHooks.add(() => log.close()) - GlobalLogging.initial( + val logging = GlobalLogging.initial( MainAppender.globalDefault(ConsoleOut.globalProxy), createTemp("sbt-global-log")(), ConsoleOut.globalProxy, initialLevel ) + val currentLog = logging.backing.file.getCanonicalFile + val previousLog = file.toList + .flatMap(dir => Option(dir.listFiles).getOrElse(Array.empty[File]).toList) + .filter(f => f.getName.startsWith("sbt-global-log") && f.getName.endsWith(".log")) + .filterNot(_.getCanonicalFile == currentLog) + .sortBy(f => (f.lastModified, f.getName)) + .lastOption + logging.copy(backing = logging.backing.copy(last = previousLog)) private def initialGlobalLogging(file: Option[File]): GlobalLogging = initialGlobalLogging(file, Level.Info) diff --git a/main/src/main/scala/sbt/MainLoop.scala b/main/src/main/scala/sbt/MainLoop.scala index 7814a74ae..52e5e4717 100644 --- a/main/src/main/scala/sbt/MainLoop.scala +++ b/main/src/main/scala/sbt/MainLoop.scala @@ -53,9 +53,12 @@ private[sbt] object MainLoop: /** Run loop that evaluates remaining commands and manages changes to global logging configuration. */ @tailrec def runLoggedLoop(state: State, logBacking: GlobalLogBacking): xsbti.MainResult = runAndClearLast(state, logBacking) match - // delete current and last log files when exiting normally + // delete the log files when exiting successfully; keep the current one on failure case RunNext.Return(result) => - logBacking.file.delete() + val failed = result match + case e: xsbti.Exit => e.code != 0 + case _ => false + if !failed then logBacking.file.delete() deleteLastLog(logBacking) result // delete previous log file, move current to previous, and start writing to a new file diff --git a/main/src/main/scala/sbt/internal/Load.scala b/main/src/main/scala/sbt/internal/Load.scala index 6de842084..619ef2529 100755 --- a/main/src/main/scala/sbt/internal/Load.scala +++ b/main/src/main/scala/sbt/internal/Load.scala @@ -196,6 +196,7 @@ private[sbt] object Load { classpath = data(config.globalPluginClasspath).map(converter.toPath), base = base, options = defaultEvalOptions, + mkReporter = () => EvalReporter.logging(config.log), ) val imports = @@ -826,8 +827,9 @@ private[sbt] object Load { // NOTE - because we create an eval here, we need a clean-eval later for this URI. lazy val eval = timed("Load.loadUnit: mkEval", log) { def mkReporter(): EvalReporter = plugs.pluginData.buildTarget match { - case None => EvalReporter.console - case Some(buildTarget) => new BuildServerEvalReporter(buildTarget, EvalReporter.console) + case None => EvalReporter.logging(log) + case Some(buildTarget) => + new BuildServerEvalReporter(buildTarget, EvalReporter.logging(log)) } mkEval( classpath = plugs.classpath.map(converter.toPath), diff --git a/sbt-app/src/sbt-test/project-load/neg-types/caseclass.log.check b/sbt-app/src/sbt-test/project-load/neg-types/caseclass.log.check new file mode 100644 index 000000000..47a739f1e --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/neg-types/caseclass.log.check @@ -0,0 +1,14 @@ +[error] -- [E007] Type Mismatch Error: BASE/build.sbt:4:3 +[error] 4 | } +[error] | ^ +[error] |Found: Unit +[error] |Required: sbt.internal.DslEntry +[error] | +[error] |Maybe the enclosing block is missing a final expression after the definition of `A`? +[error] | +[error] | longer explanation available when compiling with `-explain` +[error] -- Error: BASE/build.sbt:3:11 +[error] 3 |case class A() +[error] |^^^^^^^^^^^^^^ +[error] |Defining types in *.sbt file is not supported +[error] an error in expression: class dotty.tools.dotc.reporting.Diagnostic$Error at BASE/build.sbt:[980..991..994] L3: Defining types in *.sbt file is not supported \ No newline at end of file diff --git a/sbt-app/src/sbt-test/project-load/neg-types/changes/bad1.sbt b/sbt-app/src/sbt-test/project-load/neg-types/changes/bad1.sbt new file mode 100644 index 000000000..c7383055b --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/neg-types/changes/bad1.sbt @@ -0,0 +1,3 @@ +scalaVersion := "3.8.4" + +class A diff --git a/sbt-app/src/sbt-test/project-load/neg-types/changes/bad2.sbt b/sbt-app/src/sbt-test/project-load/neg-types/changes/bad2.sbt new file mode 100644 index 000000000..72e85d476 --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/neg-types/changes/bad2.sbt @@ -0,0 +1,3 @@ +scalaVersion := "3.8.4" + +case class A() diff --git a/sbt-app/src/sbt-test/project-load/neg-types/changes/bad3.sbt b/sbt-app/src/sbt-test/project-load/neg-types/changes/bad3.sbt new file mode 100644 index 000000000..31826005e --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/neg-types/changes/bad3.sbt @@ -0,0 +1,4 @@ +scalaVersion := "3.8.4" + +enum A: + case B diff --git a/sbt-app/src/sbt-test/project-load/neg-types/changes/good.sbt b/sbt-app/src/sbt-test/project-load/neg-types/changes/good.sbt new file mode 100644 index 000000000..24a43a3e4 --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/neg-types/changes/good.sbt @@ -0,0 +1,44 @@ +import complete.DefaultParsers.{ *, given } + +LocalRootProject / name := "hello" +scalaVersion := "3.8.4" +autoScalaLibrary := false +crossPaths := false + +def logLines(files: List[File]): List[String] = + files + .filter(_.exists) + .flatMap(IO.readLines(_)) + .map(sbt.internal.util.EscHelpers.stripColorsAndMoves) + .filterNot(_.contains("[debug]")) + +def globalLogLines(st: State): List[String] = { + val backing = st.globalLogging.backing + logLines(backing.last.toList :+ backing.file) +} + +lazy val checkGlobalLogContains = inputKey[Unit]("checks that the global log contains the given string") + +checkGlobalLogContains := { + val expected: String = (Space ~> StringBasic).parsed + val contents = globalLogLines(state.value).mkString("\n") + assert(contents.contains(expected), s"missing '$expected' in global logs:\n$contents") +} + +lazy val exportFailedSessionLog = taskKey[Unit]("exports the last session of the previous global log, delimited by the welcome banner") + +exportFailedSessionLog := Def.uncached { + val st = state.value + val t = target.value + val b = baseDirectory.value.toString + val lastLog = st.globalLogging.backing.last.getOrElse(sys.error("no previous global log")) + val chunks: List[List[String]] = + logLines(lastLog :: Nil) + .foldLeft(List(List.empty[String])) { (acc, line) => + if line.contains("welcome to sbt") then Nil :: acc + else (line.replace(b, "BASE").replaceAll(" -{4,}$", "") :: acc.head) :: acc.tail + } + .map(_.reverse) + .reverse + IO.writeLines(t / "failed-session.log", chunks.last.filter(_.startsWith("[error]"))) +} diff --git a/sbt-app/src/sbt-test/project-load/neg-types/class.log.check b/sbt-app/src/sbt-test/project-load/neg-types/class.log.check new file mode 100644 index 000000000..073d70a5e --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/neg-types/class.log.check @@ -0,0 +1,14 @@ +[error] -- [E007] Type Mismatch Error: BASE/build.sbt:4:3 +[error] 4 | } +[error] | ^ +[error] |Found: Unit +[error] |Required: sbt.internal.DslEntry +[error] | +[error] |Maybe the enclosing block is missing a final expression after the definition of `A`? +[error] | +[error] | longer explanation available when compiling with `-explain` +[error] -- Error: BASE/build.sbt:3:6 +[error] 3 |class A +[error] |^^^^^^^ +[error] |Defining types in *.sbt file is not supported +[error] an error in expression: class dotty.tools.dotc.reporting.Diagnostic$Error at BASE/build.sbt:[980..986..987] L3: Defining types in *.sbt file is not supported \ No newline at end of file diff --git a/sbt-app/src/sbt-test/project-load/neg-types/enum.log.check b/sbt-app/src/sbt-test/project-load/neg-types/enum.log.check new file mode 100644 index 000000000..0ae73804d --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/neg-types/enum.log.check @@ -0,0 +1,16 @@ +[error] -- [E007] Type Mismatch Error: BASE/build.sbt:5:3 +[error] 5 | } +[error] | ^ +[error] |Found: Unit +[error] |Required: sbt.internal.DslEntry +[error] | +[error] |Maybe the enclosing block is missing a final expression after the definition of `A`? +[error] | +[error] | longer explanation available when compiling with `-explain` +[error] -- Error: BASE/build.sbt:3:5 +[error] 3 |enum A: +[error] |^ +[error] |Defining types in *.sbt file is not supported +[error] | +[error] 4 | case B +[error] an error in expression: class dotty.tools.dotc.reporting.Diagnostic$Error at BASE/build.sbt:[980..985..996] L3: Defining types in *.sbt file is not supported \ No newline at end of file diff --git a/sbt-app/src/sbt-test/project-load/neg-types/test b/sbt-app/src/sbt-test/project-load/neg-types/test new file mode 100644 index 000000000..102e25810 --- /dev/null +++ b/sbt-app/src/sbt-test/project-load/neg-types/test @@ -0,0 +1,24 @@ +$ copy-file changes/good.sbt build.sbt +> show version + +# class +$ copy-file changes/bad1.sbt build.sbt +-> reload +$ copy-file changes/good.sbt build.sbt +> checkGlobalLogContains "Defining types in *.sbt file is not supported" +> exportFailedSessionLog +$ must-mirror class.log.check target/out/jvm/u/hello/failed-session.log + +# case class +$ copy-file changes/bad2.sbt build.sbt +-> reload +$ copy-file changes/good.sbt build.sbt +> exportFailedSessionLog +$ must-mirror caseclass.log.check target/out/jvm/u/hello/failed-session.log + +# enum +$ copy-file changes/bad3.sbt build.sbt +-> reload +$ copy-file changes/good.sbt build.sbt +> exportFailedSessionLog +$ must-mirror enum.log.check target/out/jvm/u/hello/failed-session.log