Fix issue with BE compilations incorrectly reusing successful artifacts

Also fixes an issue with best effort compilation not being properly cached, if recompiled from successful compilations
scalacenter · Nov 28, 2024 · 32dbfb1 · 32dbfb1
1 parent e79a78e
commit 32dbfb1
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 4 deletions.
diff --git a/backend/src/main/scala/bloop/Compiler.scala b/backend/src/main/scala/bloop/Compiler.scala
@@ -397,7 +397,8 @@ object Compiler {
         val newHash = BestEffortUtils.hashResult(
           previousCompilationResults.newClassesDir,
           compileInputs.sources,
-          compileInputs.classpath
+          compileInputs.classpath,
+          List(compileOut.internalReadOnlyClassesDir, compileOut.internalNewClassesDir)
         )
 
         if (newHash == previousHash) {
@@ -1036,7 +1037,8 @@ object Compiler {
         BestEffortUtils.hashResult(
           products.newClassesDir,
           compileInputs.sources,
-          compileInputs.classpath
+          compileInputs.classpath,
+          List(compileOut.internalReadOnlyClassesDir, compileOut.internalNewClassesDir)
         )
       else ""
     val failedProblems = findFailedProblems(reporter, errorCause)

diff --git a/backend/src/main/scala/bloop/util/BestEffortUtils.scala b/backend/src/main/scala/bloop/util/BestEffortUtils.scala
@@ -28,7 +28,8 @@ object BestEffortUtils {
   def hashResult(
       outputDir: Path,
       sources: Array[AbsolutePath],
-      classpath: Array[AbsolutePath]
+      classpath: Array[AbsolutePath],
+      ignoredClasspathDirectory: List[AbsolutePath]
   ): String = {
     val md = MessageDigest.getInstance("SHA-1")
 
@@ -52,7 +53,10 @@ object BestEffortUtils {
 
     md.update("<classpath>".getBytes())
     classpath.map(_.underlying).foreach { classpathFile =>
-      if (!Files.exists(classpathFile)) ()
+      if (
+        !Files.exists(classpathFile) || ignoredClasspathDirectory
+          .exists(_.underlying == classpathFile)
+      ) ()
       else if (Files.isRegularFile(classpathFile)) {
         md.update(Files.readAllBytes(classpathFile))
       } else if (Files.isDirectory(classpathFile)) {

diff --git a/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala b/frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala
@@ -192,8 +192,11 @@ object CompileTask {
                   inputs.reporter.reset()
                   val emptyResult =
                     PreviousResult.of(Optional.empty[CompileAnalysis], Optional.empty[MiniSetup])
+                  val nonIncrementalClasspath =
+                    inputs.classpath.filter(_ != inputs.out.internalReadOnlyClassesDir)
                   val newInputs = inputs.copy(
                     sources = inputs.sources,
+                    classpath = nonIncrementalClasspath,
                     previousCompilerResult = result,
                     previousResult = emptyResult
                   )

diff --git a/frontend/src/test/scala/bloop/bsp/BspMetalsClientSpec.scala b/frontend/src/test/scala/bloop/bsp/BspMetalsClientSpec.scala
@@ -691,6 +691,60 @@ class BspMetalsClientSpec(
     }
   }
 
+  test(
+    "best-effort: do not use previous successful compilation artifacts on best effort compilations"
+  ) {
+    val initFile =
+      """/Source.scala
+        |object A {
+        |  def usesB: B = ???
+        |}
+        |trait B
+        |""".stripMargin
+    val updatedFile =
+      """/Source.scala
+        |object A {
+        |  def usesB: B = ???
+        |}
+        |""".stripMargin
+    TestUtil.withinWorkspace { workspace =>
+      val `A` = TestProject(
+        workspace,
+        "A",
+        List(initFile),
+        scalaVersion = Some(bestEffortScalaVersion)
+      )
+      def updateProject(content: String) =
+        Files.write(`A`.config.sources.head.resolve("Source.scala"), content.getBytes())
+      val projects = List(`A`)
+      TestProject.populateWorkspace(workspace, projects)
+      val logger = new RecordingLogger(ansiCodesSupported = false)
+      val extraParams = BloopExtraBuildParams(
+        ownsBuildFiles = None,
+        clientClassesRootDir = None,
+        semanticdbVersion = Some(semanticdbVersion),
+        supportedScalaVersions = Some(List(bestEffortScalaVersion)),
+        javaSemanticdbVersion = None,
+        enableBestEffortMode = Some(true)
+      )
+      loadBspState(workspace, projects, logger, "Metals", bloopExtraParams = extraParams) { state =>
+        val compiledState = state.compile(`A`, arguments = Some(List("--best-effort"))).toTestState
+        assert(compiledState.status == ExitStatus.Ok)
+        updateProject(updatedFile)
+        val compiledState2 = state.compile(`A`, arguments = Some(List("--best-effort"))).toTestState
+        val compilationResult =
+          compiledState2.results.latestResult(compiledState2.build.getProjectFor("A").get)
+        assert(compiledState2.status == ExitStatus.CompilationError)
+        compilationResult match {
+          case Compiler.Result.Failed(problems, _, _, _, _) =>
+            assert(problems.exists(_.problem.message.contains("Not found: type B")))
+          case _ =>
+            assert(false)
+        }
+      }
+    }
+  }
+
   test("compile is successful with semanticDB and javac processorpath") {
     TestUtil.withinWorkspace { workspace =>
       val logger = new RecordingLogger(ansiCodesSupported = false)