Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: report compile progress when no-op after unsuccessful compilation #2395

Merged
merged 2 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions backend/src/main/scala/bloop/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import scala.collection.mutable
import scala.concurrent.Promise
import scala.util.Try

import bloop.Compiler.Result.Failed
import bloop.Compiler.Result.Ok
import bloop.io.AbsolutePath
import bloop.io.ParallelOps
import bloop.io.ParallelOps.CopyMode
Expand All @@ -20,9 +22,12 @@ import bloop.logging.ObservedLogger
import bloop.reporter.ProblemPerPhase
import bloop.reporter.Reporter
import bloop.reporter.ZincReporter
import bloop.rtexport.RtJarCache
import bloop.task.Task
import bloop.tracing.BraveTracer
import bloop.util.AnalysisUtils
import bloop.util.BestEffortUtils
import bloop.util.BestEffortUtils.BestEffortProducts
import bloop.util.CacheHashCode
import bloop.util.JavaRuntime
import bloop.util.UUIDUtil
Expand All @@ -41,11 +46,6 @@ import xsbti.T2
import xsbti.VirtualFileRef
import xsbti.compile._

import bloop.Compiler.Result.Failed
import bloop.util.BestEffortUtils
import bloop.util.BestEffortUtils.BestEffortProducts
import bloop.rtexport.RtJarCache

case class CompileInputs(
scalaInstance: ScalaInstance,
compilerCache: CompilerCache,
Expand Down Expand Up @@ -366,7 +366,13 @@ object Compiler {
}

val uniqueInputs = compileInputs.uniqueInputs
reporter.reportStartCompilation(previousProblems)
val wasPreviousSuccessful =
compileInputs.previousCompilerResult match {
case Ok(_) => true
case _ => false
}

reporter.reportStartCompilation(previousProblems, wasPreviousSuccessful)
val fileManager = newFileManager

// Manually skip redundant best-effort compilations. This is necessary because compiler
Expand Down
7 changes: 5 additions & 2 deletions backend/src/main/scala/bloop/reporter/ObservedReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,11 @@ final class ObservedReporter(
registerAction(ReporterAction.ReportCancelledCompilation)
}

override def reportStartCompilation(previousProblems: List[ProblemPerPhase]): Unit = {
underlying.reportStartCompilation(previousProblems)
override def reportStartCompilation(
previousProblems: List[ProblemPerPhase],
wasPreviousSuccessful: Boolean
): Unit = {
underlying.reportStartCompilation(previousProblems, wasPreviousSuccessful)
registerAction(ReporterAction.ReportStartCompilation)
}

Expand Down
5 changes: 4 additions & 1 deletion backend/src/main/scala/bloop/reporter/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,10 @@ trait ZincReporter extends xsbti.Reporter with ConfigurableReporter {
def reportCancelledCompilation(): Unit

/** A function called *always* at the very beginning of compilation. */
def reportStartCompilation(previousProblems: List[ProblemPerPhase]): Unit
def reportStartCompilation(
previousProblems: List[ProblemPerPhase],
wasPreviousSuccessful: Boolean
): Unit

/**
* A function called at the very end of compilation that processes the end of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ object CompileGraph {
val analysis = runningCompilation.usedLastSuccessful.previous.analysis().toOption
val previousSuccessfulProblems =
Compiler.previousProblemsFromSuccessfulCompilation(analysis)
val wasPreviousSuccessful = bundle.latestResult match {
case Compiler.Result.Ok(_) => true
case _ => false
}
val previousProblems =
Compiler.previousProblemsFromResult(bundle.latestResult, previousSuccessfulProblems)

Expand All @@ -166,7 +170,7 @@ object CompileGraph {
case ReporterAction.EnableFatalWarnings =>
reporter.enableFatalWarnings()
case ReporterAction.ReportStartCompilation =>
reporter.reportStartCompilation(previousProblems)
reporter.reportStartCompilation(previousProblems, wasPreviousSuccessful)
case a: ReporterAction.ReportStartIncrementalCycle =>
reporter.reportStartIncrementalCycle(a.sources, a.outputDirs)
case a: ReporterAction.ReportProblem => reporter.log(a.problem)
Expand Down
53 changes: 42 additions & 11 deletions frontend/src/main/scala/bloop/reporter/BspProjectReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,13 @@ final class BspProjectReporter(
}

private var recentlyReportProblemsPerFile: Map[File, List[ProblemPerPhase]] = Map.empty
private var wasPreviousSuccessful: Boolean = false

override def reportStartCompilation(recentProblems: List[ProblemPerPhase]): Unit = {
override def reportStartCompilation(
recentProblems: List[ProblemPerPhase],
wasPreviousSuccessful: Boolean
): Unit = {
this.wasPreviousSuccessful = wasPreviousSuccessful
recentlyReportProblemsPerFile = Reporter.groupProblemsByFile(recentProblems)
}

Expand Down Expand Up @@ -296,7 +301,7 @@ final class BspProjectReporter(
): Unit = {
val problemsInPreviousAnalysisPerFile = Reporter.groupProblemsByFile(previousSuccessfulProblems)

def recheckProblems = {
def mockNoOpCompileEventsAndEnd: Option[CompilationEvent.EndCompilation] = {
recentlyReportProblemsPerFile.foreach {
case (sourceFile, problemsPerFile) if reportAllPreviousProblems =>
reportAllProblems(sourceFile, problemsPerFile)
Expand All @@ -321,17 +326,43 @@ final class BspProjectReporter(
logger.noDiagnostic(CompilationEvent.NoDiagnostic(project.bspUri, sourceFile))
}
}
if (wasPreviousSuccessful) None
else {
// When no-op after unsuccessful report the start and the end of compilation
val startMsg = s"Start no-op compilation for ${project.name}"
logger.publishCompilationStart(
CompilationEvent.StartCompilation(
project.name,
project.bspUri,
startMsg,
taskId
)
)
val liftedProblems = allProblems.toIterator.map(super.liftFatalWarning(_)).toList
Some(
CompilationEvent.EndCompilation(
project.name,
project.bspUri,
taskId,
liftedProblems,
code,
isNoOp = true,
isLastCycle = true,
clientClassesDir,
clientAnalysisOut
)
)
}
}
wasEndProcessed = true
endEvent = if (cycleCount.get == 0) {
recheckProblems
None
} else {
// Great, let's report the pending end incremental cycle as the last one
val inputs =
CycleInputs(true, problemsInPreviousAnalysisPerFile, clientClassesDir, clientAnalysisOut)
Some(processEndPreviousCycle(inputs, Some(code)))
}
endEvent =
if (cycleCount.get == 0) mockNoOpCompileEventsAndEnd
else {
// Great, let's report the pending end incremental cycle as the last one
val inputs =
CycleInputs(true, problemsInPreviousAnalysisPerFile, clientClassesDir, clientAnalysisOut)
Some(processEndPreviousCycle(inputs, Some(code)))
}

// Clear the state of files with problems at the end of compilation
clearedFilesForClient.clear()
Expand Down
5 changes: 4 additions & 1 deletion frontend/src/main/scala/bloop/reporter/LogReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ final class LogReporter(
logger.info(s"Compiled ${project.name} (${durationMs}ms)")
}

override def reportStartCompilation(previousProblems: List[ProblemPerPhase]): Unit = ()
override def reportStartCompilation(
previousProblems: List[ProblemPerPhase],
wasPreviousSuccessful: Boolean
): Unit = ()

override def reportEndCompilation(): Unit = ()

Expand Down
40 changes: 27 additions & 13 deletions frontend/src/test/scala/bloop/bsp/BspCompileSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,13 @@ class BspCompileSpec(
"""#3: a/src/A.scala
| -> List()
| -> reset = true
|#3: task start 3
| -> Msg: Start no-op compilation for a
| -> Data kind: compile-task
|#3: task finish 3
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
""".stripMargin // no-op so it only gives the compile-report
)

Expand All @@ -590,7 +597,7 @@ class BspCompileSpec(
assertSameExternalClassesDirs(fourthCompiledState, compiledState, projects)
assertNoDiff(
fourthCompiledState.lastDiagnostics(`A`),
"""#4: task start 3
"""#4: task start 4
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#4: a/src/A.scala
Expand All @@ -599,7 +606,7 @@ class BspCompileSpec(
|#4: a/src/A.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = false
|#4: task finish 3
|#4: task finish 4
| -> errors 1, warnings 1
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand All @@ -613,13 +620,13 @@ class BspCompileSpec(
assertDifferentExternalClassesDirs(fifthCompiledState, compiledState, projects)
assertNoDiff(
fifthCompiledState.lastDiagnostics(`A`),
"""#5: task start 4
"""#5: task start 5
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#5: a/src/A.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#5: task finish 4
|#5: task finish 5
| -> errors 0, warnings 1
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand All @@ -639,17 +646,17 @@ class BspCompileSpec(
assertSameExternalClassesDirs(sixthCompiledState, fifthCompiledState, projects)
assertNoDiff(
sixthCompiledState.lastDiagnostics(`A`),
"""#6: task start 5
"""#6: task start 6
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#6: a/src/A.scala
| -> List()
| -> reset = true
|#6: task finish 5
|#6: task finish 6
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
|#6: task start 5
|#6: task start 6
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#6: a/src/A.scala
Expand All @@ -658,7 +665,7 @@ class BspCompileSpec(
|#6: a/src/A.scala
| -> List(Diagnostic(Range(Position(1,0),Position(3,1)),Some(Error),Some(_),Some(_),object creation impossible, since value y in trait Base of type Int is not defined,None,None,Some({"actions":[]})))
| -> reset = false
|#6: task finish 5
|#6: task finish 6
| -> errors 1, warnings 1
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand All @@ -673,13 +680,13 @@ class BspCompileSpec(

assertNoDiff(
seventhCompiledState.lastDiagnostics(`A`),
"""#7: task start 6
"""#7: task start 7
| -> Msg: Compiling a (1 Scala source)
| -> Data kind: compile-task
|#7: a/src/A.scala
| -> List(Diagnostic(Range(Position(0,0),Position(0,26)),Some(Warning),Some(_),Some(_),Unused import,None,None,Some({"actions":[]})))
| -> reset = true
|#7: task finish 6
|#7: task finish 7
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report
Expand Down Expand Up @@ -953,9 +960,16 @@ class BspCompileSpec(

assertNoDiff(
thirdCompiledState.lastDiagnostics(`A`),
"""#3: a/src/main/scala/Bar.scala
| -> List()
| -> reset = true""".stripMargin
"""|#3: a/src/main/scala/Bar.scala
| -> List()
| -> reset = true
|#3: task start 3
| -> Msg: Start no-op compilation for a
| -> Data kind: compile-task
|#3: task finish 3
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report""".stripMargin
)
}
}
Expand Down
9 changes: 8 additions & 1 deletion frontend/src/test/scala/bloop/bsp/BspSbtClientSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,14 @@ class BspSbtClientSpec(
thirdCompiledState.lastDiagnostics(`A`),
"""#3: a/src/main/scala/Foo.scala
| -> List()
| -> reset = true""".stripMargin
| -> reset = true
|#3: task start 3
| -> Msg: Start no-op compilation for a
| -> Data kind: compile-task
|#3: task finish 3
| -> errors 0, warnings 0
| -> Msg: Compiled 'a'
| -> Data kind: compile-report""".stripMargin
)
}
}
Expand Down
Loading