[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.

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

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")))
+  }

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)

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

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),

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

sbt-app/src/sbt-test/project-load/neg-types/changes/bad1.sbt

@@ -0,0 +1,3 @@
+scalaVersion := "3.8.4"
+
+class A

sbt-app/src/sbt-test/project-load/neg-types/changes/bad2.sbt

@@ -0,0 +1,3 @@
+scalaVersion := "3.8.4"
+
+case class A()

sbt-app/src/sbt-test/project-load/neg-types/changes/bad3.sbt

@@ -0,0 +1,4 @@
+scalaVersion := "3.8.4"
+
+enum A:
+  case 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]")))
+}

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

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

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