From 50ec1837f15b6bb68d4e4b3819c30db31c1de2b9 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Thu, 4 Jul 2024 11:14:23 +0200 Subject: [PATCH 1/2] Fix regression introduced in the best effort PR Previously, if `--best-effort` option was set, every project would be compiled in the best effort mode, including those using scala 2. --- .../scala/bloop/bsp/BloopBspServices.scala | 4 +- .../main/scala/bloop/engine/Interpreter.scala | 2 +- .../bloop/engine/tasks/CompileTask.scala | 119 +++++++++--------- .../tasks/compilation/CompileGraph.scala | 10 +- 4 files changed, 68 insertions(+), 67 deletions(-) diff --git a/frontend/src/main/scala/bloop/bsp/BloopBspServices.scala b/frontend/src/main/scala/bloop/bsp/BloopBspServices.scala index d1e7f7dcd..f34104a04 100644 --- a/frontend/src/main/scala/bloop/bsp/BloopBspServices.scala +++ b/frontend/src/main/scala/bloop/bsp/BloopBspServices.scala @@ -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) @@ -486,7 +486,7 @@ final class BloopBspServices( dag, createReporter, isPipeline, - isBestEffort, + bestEffortAllowed, cancelCompilation, store, logger diff --git a/frontend/src/main/scala/bloop/engine/Interpreter.scala b/frontend/src/main/scala/bloop/engine/Interpreter.scala index 545fd12f0..a647c7da7 100644 --- a/frontend/src/main/scala/bloop/engine/Interpreter.scala +++ b/frontend/src/main/scala/bloop/engine/Interpreter.scala @@ -173,7 +173,7 @@ object Interpreter { dag, createReporter, cmd.pipeline, - bestEffort = false, + bestEffortAllowed = false, Promise[Unit](), CompileClientStore.NoStore, state.logger diff --git a/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala b/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala index 631f3f402..0366d8829 100644 --- a/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala +++ b/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala @@ -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 @@ -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())) + } } } diff --git a/frontend/src/main/scala/bloop/engine/tasks/compilation/CompileGraph.scala b/frontend/src/main/scala/bloop/engine/tasks/compilation/CompileGraph.scala index 151dabc4b..0fc070736 100644 --- a/frontend/src/main/scala/bloop/engine/tasks/compilation/CompileGraph.scala +++ b/frontend/src/main/scala/bloop/engine/tasks/compilation/CompileGraph.scala @@ -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 = { @@ -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)) @@ -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) @@ -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) From 5c804c4ce44c61cbe5dd365822e507b3289a28f4 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Thu, 4 Jul 2024 13:22:24 +0200 Subject: [PATCH 2/2] Fix second regression --- frontend/src/main/scala/bloop/engine/tasks/TestTask.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/main/scala/bloop/engine/tasks/TestTask.scala b/frontend/src/main/scala/bloop/engine/tasks/TestTask.scala index b94ef4650..e4a70357f 100644 --- a/frontend/src/main/scala/bloop/engine/tasks/TestTask.scala +++ b/frontend/src/main/scala/bloop/engine/tasks/TestTask.scala @@ -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(