Skip to content

Commit

Permalink
Merge pull request #2363 from jchyb/fix-metals-issues
Browse files Browse the repository at this point in the history
Fix recent regressions not allowing Metals to pass its tests
  • Loading branch information
tgodzik authored Jul 4, 2024
2 parents c505385 + 5c804c4 commit e757787
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 68 deletions.
4 changes: 2 additions & 2 deletions frontend/src/main/scala/bloop/bsp/BloopBspServices.scala
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ final class BloopBspServices(
}

val isPipeline = compileArgs.exists(_ == "--pipeline")
val isBestEffort = compileArgs.exists(_ == "--best-effort")
val bestEffortAllowed = compileArgs.exists(_ == "--best-effort")
def compile(projects: List[Project]): Task[State] = {
val config = ReporterConfig.defaultFormat.copy(reverseOrder = false)

Expand Down Expand Up @@ -486,7 +486,7 @@ final class BloopBspServices(
dag,
createReporter,
isPipeline,
isBestEffort,
bestEffortAllowed,
cancelCompilation,
store,
logger
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/main/scala/bloop/engine/Interpreter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ object Interpreter {
dag,
createReporter,
cmd.pipeline,
bestEffort = false,
bestEffortAllowed = false,
Promise[Unit](),
CompileClientStore.NoStore,
state.logger
Expand Down
119 changes: 60 additions & 59 deletions frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ object CompileTask {
dag: Dag[Project],
createReporter: ReporterInputs[UseSiteLogger] => Reporter,
pipeline: Boolean,
bestEffort: Boolean,
bestEffortAllowed: Boolean,
cancelCompilation: Promise[Unit],
store: CompileClientStore,
rawLogger: UseSiteLogger
Expand Down Expand Up @@ -276,73 +276,74 @@ object CompileTask {
}

val client = state.client
CompileGraph.traverse(dag, client, store, bestEffort, setup(_), compile).flatMap { pdag =>
val partialResults = Dag.dfs(pdag, mode = Dag.PreOrder)
val finalResults = partialResults.map(r => PartialCompileResult.toFinalResult(r))
Task.gatherUnordered(finalResults).map(_.flatten).flatMap { results =>
val cleanUpTasksToRunInBackground =
markUnusedClassesDirAndCollectCleanUpTasks(results, state, rawLogger)

val failures = results.flatMap {
case FinalNormalCompileResult(p, results) =>
results.fromCompiler match {
case Compiler.Result.NotOk(_) => List(p)
// Consider success with reported fatal warnings as error to simulate -Xfatal-warnings
case s: Compiler.Result.Success if s.reportedFatalWarnings => List(p)
case _ => Nil
}
case _ => Nil
}
CompileGraph.traverse(dag, client, store, bestEffortAllowed, setup(_), compile).flatMap {
pdag =>
val partialResults = Dag.dfs(pdag, mode = Dag.PreOrder)
val finalResults = partialResults.map(r => PartialCompileResult.toFinalResult(r))
Task.gatherUnordered(finalResults).map(_.flatten).flatMap { results =>
val cleanUpTasksToRunInBackground =
markUnusedClassesDirAndCollectCleanUpTasks(results, state, rawLogger)

val failures = results.flatMap {
case FinalNormalCompileResult(p, results) =>
results.fromCompiler match {
case Compiler.Result.NotOk(_) => List(p)
// Consider success with reported fatal warnings as error to simulate -Xfatal-warnings
case s: Compiler.Result.Success if s.reportedFatalWarnings => List(p)
case _ => Nil
}
case _ => Nil
}

val newState: State = {
val stateWithResults = state.copy(results = state.results.addFinalResults(results))
if (failures.isEmpty) {
stateWithResults.copy(status = ExitStatus.Ok)
} else {
results.foreach {
case FinalNormalCompileResult.HasException(project, err) =>
val errMsg = err.fold(identity, Logger.prettyPrintException)
rawLogger.error(s"Unexpected error when compiling ${project.name}: $errMsg")
case _ => () // Do nothing when the final compilation result is not an actual error
}
val newState: State = {
val stateWithResults = state.copy(results = state.results.addFinalResults(results))
if (failures.isEmpty) {
stateWithResults.copy(status = ExitStatus.Ok)
} else {
results.foreach {
case FinalNormalCompileResult.HasException(project, err) =>
val errMsg = err.fold(identity, Logger.prettyPrintException)
rawLogger.error(s"Unexpected error when compiling ${project.name}: $errMsg")
case _ => () // Do nothing when the final compilation result is not an actual error
}

client match {
case _: ClientInfo.CliClientInfo =>
// Reverse list of failed projects to get ~correct order of failure
val projectsFailedToCompile = failures.map(p => s"'${p.name}'").reverse
val failureMessage =
if (failures.size <= 2) projectsFailedToCompile.mkString(",")
else {
s"${projectsFailedToCompile.take(2).mkString(", ")} and ${projectsFailedToCompile.size - 2} more projects"
}
client match {
case _: ClientInfo.CliClientInfo =>
// Reverse list of failed projects to get ~correct order of failure
val projectsFailedToCompile = failures.map(p => s"'${p.name}'").reverse
val failureMessage =
if (failures.size <= 2) projectsFailedToCompile.mkString(",")
else {
s"${projectsFailedToCompile.take(2).mkString(", ")} and ${projectsFailedToCompile.size - 2} more projects"
}

rawLogger.error("Failed to compile " + failureMessage)
case _: ClientInfo.BspClientInfo => () // Don't report if bsp client
}
rawLogger.error("Failed to compile " + failureMessage)
case _: ClientInfo.BspClientInfo => () // Don't report if bsp client
}

stateWithResults.copy(status = ExitStatus.CompilationError)
stateWithResults.copy(status = ExitStatus.CompilationError)
}
}
}

// Schedule to run clean-up tasks in the background
runIOTasksInParallel(cleanUpTasksToRunInBackground)
// Schedule to run clean-up tasks in the background
runIOTasksInParallel(cleanUpTasksToRunInBackground)

val runningTasksRequiredForCorrectness = Task.sequence {
results.flatMap {
case FinalNormalCompileResult(_, result) =>
val tasksAtEndOfBuildCompilation =
Task.fromFuture(result.runningBackgroundTasks)
List(tasksAtEndOfBuildCompilation)
case _ => Nil
val runningTasksRequiredForCorrectness = Task.sequence {
results.flatMap {
case FinalNormalCompileResult(_, result) =>
val tasksAtEndOfBuildCompilation =
Task.fromFuture(result.runningBackgroundTasks)
List(tasksAtEndOfBuildCompilation)
case _ => Nil
}
}
}

// Block on all background task that are running and are required for correctness
runningTasksRequiredForCorrectness
.executeOn(ExecutionContext.ioScheduler)
.map(_ => newState)
.doOnFinish(_ => Task(rootTracer.terminate()))
}
// Block on all background task that are running and are required for correctness
runningTasksRequiredForCorrectness
.executeOn(ExecutionContext.ioScheduler)
.map(_ => newState)
.doOnFinish(_ => Task(rootTracer.terminate()))
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/main/scala/bloop/engine/tasks/TestTask.scala
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ object TestTask {
taskDefs.map {
case TaskDefWithFramework(taskDef, _) =>
selectedTests.get(taskDef.fullyQualifiedName()) match {
case None =>
case None | Some(Nil) =>
taskDef
case Some(value) =>
new TaskDef(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ object CompileGraph {
dag: Dag[Project],
client: ClientInfo,
store: CompileClientStore,
bestEffort: Boolean,
bestEffortAllowed: Boolean,
computeBundle: BundleInputs => Task[CompileBundle],
compile: (Inputs, Boolean, Boolean) => Task[ResultBundle]
): CompileTraversal = {
Expand Down Expand Up @@ -393,7 +393,7 @@ object CompileGraph {
case Leaf(project) =>
val bundleInputs = BundleInputs(project, dag, Map.empty)
setupAndDeduplicate(client, bundleInputs, computeBundle) { bundle =>
compile(Inputs(bundle, Map.empty), bestEffort, isBestEffortDep).map { results =>
compile(Inputs(bundle, Map.empty), bestEffortAllowed && project.isBestEffort, isBestEffortDep).map { results =>
results.fromCompiler match {
case Compiler.Result.Ok(_) => Leaf(partialSuccess(bundle, results))
case _ => Leaf(toPartialFailure(bundle, results))
Expand Down Expand Up @@ -428,8 +428,8 @@ object CompileGraph {
case (_, ResultBundle(f: Compiler.Result.Failed, _, _, _)) => f.bestEffortProducts.isEmpty
case _ => false
}
val continue = bestEffort && depsSupportBestEffort && successfulBestEffort || failed.isEmpty
val dependsOnBestEffort = failed.nonEmpty && bestEffort && depsSupportBestEffort || isBestEffortDep
val continue = bestEffortAllowed && depsSupportBestEffort && successfulBestEffort || failed.isEmpty
val dependsOnBestEffort = failed.nonEmpty && bestEffortAllowed && depsSupportBestEffort || isBestEffortDep

if (!continue) {
// Register the name of the projects we're blocked on (intransitively)
Expand Down Expand Up @@ -459,7 +459,7 @@ object CompileGraph {
val bundleInputs = BundleInputs(project, dag, dependentProducts.toMap)
setupAndDeduplicate(client, bundleInputs, computeBundle) { bundle =>
val inputs = Inputs(bundle, resultsMap)
compile(inputs, bestEffort, dependsOnBestEffort).map { results =>
compile(inputs, bestEffortAllowed && project.isBestEffort, dependsOnBestEffort).map { results =>
results.fromCompiler match {
case Compiler.Result.Ok(_) if failed.isEmpty =>
Parent(partialSuccess(bundle, results), dagResults)
Expand Down

0 comments on commit e757787

Please sign in to comment.