From 32dbfb14dbaec488eccb515db67a2b0df8339f54 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Thu, 28 Nov 2024 15:56:58 +0100 Subject: [PATCH 1/2] 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 --- backend/src/main/scala/bloop/Compiler.scala | 6 ++- .../scala/bloop/util/BestEffortUtils.scala | 8 ++- .../bloop/engine/tasks/CompileTask.scala | 3 ++ .../scala/bloop/bsp/BspMetalsClientSpec.scala | 54 +++++++++++++++++++ 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/backend/src/main/scala/bloop/Compiler.scala b/backend/src/main/scala/bloop/Compiler.scala index 3e4aa3304..415f0a68f 100644 --- 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 index f9dd5cbc3..893ea07f6 100644 --- 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("".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 index 86a4a91c2..eeb7050c3 100644 --- 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 index 955f687cb..6552807ff 100644 --- 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) From cdbe45b3eae92e40afbb0e7859134b61664df881 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Thu, 5 Dec 2024 18:06:27 +0100 Subject: [PATCH 2/2] Add better comments about hashing --- backend/src/main/scala/bloop/Compiler.scala | 10 +++++++++- .../main/scala/bloop/util/BestEffortUtils.scala | 16 +++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/backend/src/main/scala/bloop/Compiler.scala b/backend/src/main/scala/bloop/Compiler.scala index 415f0a68f..9822129ea 100644 --- a/backend/src/main/scala/bloop/Compiler.scala +++ b/backend/src/main/scala/bloop/Compiler.scala @@ -394,11 +394,19 @@ object Compiler { BestEffortProducts(previousCompilationResults, previousHash, _) ) ) if isBestEffortMode => + // We filter out certain directories from classpath, as: + // * we do not want to hash readOnlyClassesDir, as that contains classfiles unrelated to + // best effort compilation + // * newClassesDir on the classpath of the previous successful best effort compilation + // is also different from newClassesDir of the current compilation. val newHash = BestEffortUtils.hashResult( previousCompilationResults.newClassesDir, compileInputs.sources, compileInputs.classpath, - List(compileOut.internalReadOnlyClassesDir, compileOut.internalNewClassesDir) + ignoredClasspathDirectories = List( + compileOut.internalReadOnlyClassesDir, + compileOut.internalNewClassesDir + ) ) if (newHash == previousHash) { diff --git a/backend/src/main/scala/bloop/util/BestEffortUtils.scala b/backend/src/main/scala/bloop/util/BestEffortUtils.scala index 893ea07f6..7d3eff8bc 100644 --- a/backend/src/main/scala/bloop/util/BestEffortUtils.scala +++ b/backend/src/main/scala/bloop/util/BestEffortUtils.scala @@ -29,7 +29,7 @@ object BestEffortUtils { outputDir: Path, sources: Array[AbsolutePath], classpath: Array[AbsolutePath], - ignoredClasspathDirectory: List[AbsolutePath] + ignoredClasspathDirectories: List[AbsolutePath] ): String = { val md = MessageDigest.getInstance("SHA-1") @@ -50,21 +50,19 @@ object BestEffortUtils { md.update(Files.readAllBytes(underlying)) } } - md.update("".getBytes()) classpath.map(_.underlying).foreach { classpathFile => if ( - !Files.exists(classpathFile) || ignoredClasspathDirectory - .exists(_.underlying == classpathFile) + !Files.exists(classpathFile) + || ignoredClasspathDirectories.exists(_.underlying == classpathFile) + || outputDir == classpathFile ) () else if (Files.isRegularFile(classpathFile)) { md.update(Files.readAllBytes(classpathFile)) } else if (Files.isDirectory(classpathFile)) { - if (outputDir != classpathFile) { - Files.walk(classpathFile).iterator().asScala.foreach { file => - if (Files.isRegularFile(file)) { - md.update(Files.readAllBytes(file)) - } + Files.walk(classpathFile).iterator().asScala.foreach { file => + if (Files.isRegularFile(file)) { + md.update(Files.readAllBytes(file)) } } }