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 issue with BE compilations incorrectly reusing successful artifacts #2520

Merged
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
14 changes: 12 additions & 2 deletions backend/src/main/scala/bloop/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -394,10 +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
compileInputs.classpath,
ignoredClasspathDirectories = List(
compileOut.internalReadOnlyClassesDir,
compileOut.internalNewClassesDir
)
)

if (newHash == previousHash) {
Expand Down Expand Up @@ -1036,7 +1045,8 @@ object Compiler {
BestEffortUtils.hashResult(
products.newClassesDir,
compileInputs.sources,
compileInputs.classpath
compileInputs.classpath,
List(compileOut.internalReadOnlyClassesDir, compileOut.internalNewClassesDir)
)
else ""
val failedProblems = findFailedProblems(reporter, errorCause)
Expand Down
18 changes: 10 additions & 8 deletions backend/src/main/scala/bloop/util/BestEffortUtils.scala
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ object BestEffortUtils {
def hashResult(
outputDir: Path,
sources: Array[AbsolutePath],
classpath: Array[AbsolutePath]
classpath: Array[AbsolutePath],
ignoredClasspathDirectories: List[AbsolutePath]
): String = {
val md = MessageDigest.getInstance("SHA-1")

Expand All @@ -49,18 +50,19 @@ object BestEffortUtils {
md.update(Files.readAllBytes(underlying))
}
}

md.update("<classpath>".getBytes())
classpath.map(_.underlying).foreach { classpathFile =>
if (!Files.exists(classpathFile)) ()
if (
!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))
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/main/scala/bloop/engine/tasks/CompileTask.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
54 changes: 54 additions & 0 deletions frontend/src/test/scala/bloop/bsp/BspMetalsClientSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading