Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class TurbineClasspathFileManager(
workspaceClasspath: () => TurbineCompileResult,
listSourcepath: String => java.lang.Iterable[JavaFileObject],
isDeleted: String => Boolean,
projectClasspath: ClassPath,
) extends ForwardingJavaFileManager[JavaFileManager](delegate) {

override def contains(
Expand Down Expand Up @@ -83,6 +82,13 @@ class TurbineClasspathFileManager(
val turbinePackageName = packageNames.mkString("/")
val objects = new ju.ArrayList[JavaFileObject]()
val cp = workspaceClasspath()
val isAddedBinaryName = new ju.HashSet[String]()
super.list(location, packageName, kinds, recurse).forEach { obj =>
val binaryName = inferBinaryName(location, obj).replace('.', '/')
if (isAddedBinaryName.add(binaryName)) {
objects.add(obj)
}
}
cp.symbolsByPackage.get(turbinePackageName) match {
case None =>
case Some(values) =>
Expand All @@ -92,7 +98,7 @@ class TurbineClasspathFileManager(
val binaryName = sym.binaryName()
// Skip classes that have been deleted but not yet recompiled,
// or have a pending source on SOURCE_PATH (so javac uses the updated source)
if (!isDeleted(binaryName)) {
if (!isDeleted(binaryName) && isAddedBinaryName.add(binaryName)) {
val bytes = cp.lowered.bytes().get(binaryName)
if (bytes != null) {
Comment thread
tgodzik marked this conversation as resolved.
Outdated
val obj = new TurbineClassfileObject(
Expand All @@ -104,14 +110,11 @@ class TurbineClasspathFileManager(
}
}
}
val isAddedBinaryName = new ju.HashSet[String]()
for {
cp <- List(
// Prioritize the project classpath over the fallback classpath
projectClasspath,
cp.classpath,
)
} listPackageClasspath(cp, packageNames, isAddedBinaryName) { obj =>
listPackageClasspath(
cp.classpath,
packageNames,
isAddedBinaryName,
) { obj =>
objects.add(obj)
}
objects
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,20 +328,17 @@ class TurbineCompiler[T](
underlying: StandardJavaFileManager,
projectClasspathJars: ju.List[Path],
): JavaFileManager = {
val isGlobalClasspathEntry = this.classpath().toSet
val filteredProjectClasspath =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a quick LLM assisted look:

Dropping the filter also re-indexes every project jar (including ones already on the global CP) on each presentation-compiler create. In a large MBT classpath that can be a noticeable startup cost; the filter existed to avoid that.

the worry here seems legit, we will be radding jars, which were already added.

The project jars might also be outdated compared to what turbine has actually 🤔

Could we instead make it work with protobuf jars only?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, kept isGlobalClasspathEntry and adjusted the project classpath with protobuf jars. Let me know if this is closer to what you had in mind

projectClasspathJars.asScala.filter(file =>
!isGlobalClasspathEntry(file) && TurbineCompiler.isJarFile(file)
)
val projectClasspathEntries = projectClasspathJars.asScala.filter(
TurbineCompiler.isJarFile
)
val projectClasspath =
ClassPathBinder.bindClasspath(filteredProjectClasspath.asJava)
ClassPathBinder.bindClasspath(projectClasspathEntries.asJava)
onNewProjectClasspath(projectClasspath)
new TurbineClasspathFileManager(
underlying,
() => result,
listSourcepath = listCombinedSourcepath,
isDeleted,
projectClasspath,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,30 @@ package tests.mbt

import java.nio.file.Files
import java.nio.file.Paths
import java.util.EnumSet
import java.util.zip.ZipEntry
import java.util.zip.ZipOutputStream
import javax.tools.JavaFileObject
import javax.tools.StandardLocation
import javax.tools.ToolProvider

import scala.collection.JavaConverters._
import scala.collection.parallel.mutable.ParArray
import scala.concurrent.Await
import scala.concurrent.ExecutionContext
import scala.concurrent.duration._

import scala.meta.internal.metals.Configs
import scala.meta.internal.metals.EmptyWorkDoneProgress
import scala.meta.internal.metals.LoggerReportContext
import scala.meta.internal.metals.mbt.IndexingStats
import scala.meta.internal.metals.mbt.MbtWorkspaceSymbolProvider
import scala.meta.internal.metals.mbt.TurbineClasspathFileManager
import scala.meta.internal.metals.mbt.TurbineCompileResult
import scala.meta.internal.metals.mbt.TurbineCompiler
import scala.meta.io.AbsolutePath

import com.google.turbine.diag.SourceFile
import munit.AnyFixture
import munit.TestOptions
import org.eclipse.{lsp4j => l}
Expand Down Expand Up @@ -227,3 +241,61 @@ module com.example {
)

}

class TurbineClasspathFileManagerSuite extends munit.FunSuite {
implicit val reportContext: LoggerReportContext.type = LoggerReportContext

private def compile(source: String): TurbineCompileResult =
TurbineCompiler.compileClassfiles(
ParArray(source),
(text: String) => Seq(new SourceFile("Dependency.java", text)),
Nil,
EmptyWorkDoneProgress,
)

test("target-classpath-before-workspace-headers") {
val workspaceResult = compile(
"package example; public class Dependency { public static void workspace() {} }"
)
val projectResult = compile(
"package example; public class Dependency { public static void project() {} }"
)
val jar = Files.createTempFile("metals-project-classpath", ".jar")
val output = new ZipOutputStream(Files.newOutputStream(jar))
try {
for ((name, bytes) <- projectResult.lowered.bytes().asScala) {
output.putNextEntry(new ZipEntry(s"$name.class"))
output.write(bytes)
output.closeEntry()
}
} finally output.close()

val standardFileManager = ToolProvider
.getSystemJavaCompiler()
.getStandardFileManager(null, null, null)
standardFileManager.setLocationFromPaths(
StandardLocation.CLASS_PATH,
List(jar).asJava,
)
val fileManager = new TurbineClasspathFileManager(
Comment thread
tgodzik marked this conversation as resolved.
Outdated
standardFileManager,
() => workspaceResult,
_ => java.util.Collections.emptyList(),
_ => false,
)
val classfiles = fileManager
.list(
StandardLocation.CLASS_PATH,
"example",
EnumSet.of(JavaFileObject.Kind.CLASS),
false,
)
.asScala
.toList
assertEquals(classfiles.size, 1)
assertEquals(
classfiles.head.openInputStream().readAllBytes().toSeq,
projectResult.lowered.bytes().get("example/Dependency").toSeq,
)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Loading