Skip to content

Commit

Permalink
Small fixes to make Metals work on large Pants workspaces
Browse files Browse the repository at this point in the history
While testing the new Pants integration in a large Pants workspace I
encountered several issues related file watching and reading `*.jar`
files that have invalid ZIP byte contents.
  • Loading branch information
Olafur Pall Geirsson committed Oct 3, 2019
1 parent 314b5d0 commit 4f0af50
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 28 deletions.
26 changes: 10 additions & 16 deletions metals/src/main/scala/scala/meta/internal/metals/FileWatcher.scala
Expand Up @@ -10,6 +10,7 @@ import java.util.concurrent.CompletableFuture
import java.util.concurrent.Executors
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.io.AbsolutePath
import java.{util => ju}

/**
* Handles file watching of interesting files in this build.
Expand Down Expand Up @@ -62,7 +63,7 @@ final class FileWatcher(
}

def restart(): Unit = {
val sourceDirectoriesToWatch = new util.ArrayList[Path]()
val sourceDirectoriesToWatch = new util.LinkedHashSet[Path]()
val sourceFilesToWatch = new util.ArrayList[Path]()
val createdSourceDirectories = new util.ArrayList[AbsolutePath]()
def watch(path: AbsolutePath, isSource: Boolean): Unit = {
Expand All @@ -78,8 +79,11 @@ final class FileWatcher(
// work on some other systems like Linux
if (isSource) createdSourceDirectories.add(pathToCreate)
}
if (path.isScalaOrJava) sourceFilesToWatch.add(path.toNIO)
else sourceDirectoriesToWatch.add(path.toNIO)
if (path.isScalaOrJava) {
sourceDirectoriesToWatch.add(path.toNIO.getParent())
} else {
sourceDirectoriesToWatch.add(path.toNIO)
}
}
// Watch the source directories for "goto definition" index.
buildTargets.sourceItems.foreach(watch(_, isSource = true))
Expand All @@ -93,30 +97,20 @@ final class FileWatcher(
)
}
}
startWatching(sourceFilesToWatch, sourceDirectoriesToWatch)
startWatching(new ju.ArrayList(sourceDirectoriesToWatch))
createdSourceDirectories.asScala.foreach(_.delete())
}

private def startWatching(
files: util.List[Path],
directories: util.List[Path]
): Unit = {
private def startWatching(directories: util.List[Path]): Unit = {
stopWatching()
val directoryWatcher = DirectoryWatcher
.builder()
.paths(directories)
.listener(new DirectoryListener())
.fileHashing(false)
.build()
activeDirectoryWatcher = Some(directoryWatcher)
directoryWatching = directoryWatcher.watchAsync(directoryExecutor)

val fileWatcher = DirectoryWatcher
.builder()
.paths(files.map(_.getParent()))
.listener(new FileListener(watched = files.asScala.toSet))
.build()
activeFileWatcher = Some(fileWatcher)
fileWatching = fileWatcher.watchAsync(fileExecutor)
}

private def stopWatching(): Unit = {
Expand Down
Expand Up @@ -3,7 +3,6 @@ package scala.meta.internal.metals
import java.nio.file.Files
import java.nio.file.attribute.BasicFileAttributeView
import java.sql.{Connection, PreparedStatement, Statement}

import scala.collection.concurrent.TrieMap
import scala.meta.internal.io.PlatformFileIO
import scala.meta.internal.metals.JdbcEnrichments._
Expand Down
Expand Up @@ -34,6 +34,8 @@ import scala.meta.tokens.Token
import scala.util.Properties
import scala.{meta => m}
import java.nio.file.StandardOpenOption
import java.io.RandomAccessFile
import scala.util.control.NonFatal

/**
* One stop shop for all extension methods that are used in the metals build.
Expand Down Expand Up @@ -270,6 +272,39 @@ object MetalsEnrichments
FileIO.slurp(path, StandardCharsets.UTF_8)
}

def isValidClasspathEntry(): Boolean =
path.isDirectory || hasZipMagicNumbers()

/**
* Returns true if this file can be read as a ZIP/Jar file.
*
* It's not sufficient to check the filename extension because `*.jar`
* files can contain non-ZIP contents causing `java.util.zip.ZipError`
* which is difficult to recover from.
*/
def hasZipMagicNumbers(): Boolean = {
path.isFile && {
try {
val raf = new RandomAccessFile(path.toFile, "r")
try {
val n = raf.readInt()
n match {
// Magic numbers from https://en.wikipedia.org/wiki/List_of_file_signatures
case 0x504B0304 | 0x504B0506 | 0x504B0708 =>
true
case _ =>
false
}
} finally {
raf.close()
}
} catch {
case NonFatal(_) =>
false
}
}
}

def isJar: Boolean = {
val filename = path.toNIO.getFileName.toString
filename.endsWith(".jar")
Expand Down
Expand Up @@ -48,6 +48,7 @@ import scala.util.control.NonFatal
import scala.util.Success
import com.google.gson.JsonPrimitive
import com.google.gson.JsonObject
import java.util.zip.ZipError

class MetalsLanguageServer(
ec: ExecutionContextExecutorService,
Expand Down Expand Up @@ -1308,10 +1309,10 @@ class MetalsLanguageServer(
}
for {
i <- statusBar.trackFuture("Importing build", importedBuild)
_ <- profiledIndexWorkspace { () =>
indexWorkspace(i)
}
_ = indexingPromise.trySuccess(())
_ <- profiledIndexWorkspace(
() => indexWorkspace(i),
() => indexingPromise.trySuccess(())
)
_ <- Future.sequence[Unit, List](
compilations
.cascadeCompileFiles(buffers.open.toSeq)
Expand Down Expand Up @@ -1412,18 +1413,21 @@ class MetalsLanguageServer(
}

def profiledIndexWorkspace(
thunk: () => Unit
thunk: () => Unit,
onFinally: () => Unit
): Future[Unit] = {
val tracked = statusBar.trackFuture(
s"Indexing",
Future {
timedThunk("indexed workspace", onlyIf = true) {
try thunk()
catch {
case NonFatal(e) =>
scribe.error("unexpected error indexing workspace", e)
finally {
onFinally()
}
}
}.recover {
case e: ZipError =>
scribe.error("unexpected error indexing workspace", e)
}
)
tracked.foreach { _ =>
Expand Down Expand Up @@ -1521,9 +1525,10 @@ class MetalsLanguageServer(
for {
item <- dependencySources.getItems.asScala
sourceUri <- Option(item.getSources).toList.flatMap(_.asScala)
path = sourceUri.toAbsolutePath
if path.isValidClasspathEntry()
} {
try {
val path = sourceUri.toAbsolutePath
buildTargets.addDependencySource(path, item.getTarget)
if (path.isJar) {
usedJars += path
Expand Down
Expand Up @@ -197,8 +197,8 @@ final class StatusBar(
timer.elapsedSeconds > 10 ||
firstShow.exists(_.elapsedSeconds > 5)
def isStale: Boolean = this match {
case Message(_) => (firstShow.isDefined && !isRecent) || isOutdated
case Progress(_, job, _, _) => job.isCompleted
case _: Message => (firstShow.isDefined && !isRecent) || isOutdated
case p: Progress => p.job.isCompleted
}
}
private case class Message(params: MetalsStatusParams) extends Item
Expand Down
Expand Up @@ -9,6 +9,7 @@ import org.eclipse.{lsp4j => l}
import scala.collection.concurrent.TrieMap
import scala.concurrent.ExecutionContext
import scala.meta.internal.mtags.OnDemandSymbolIndex
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.semanticdb.SymbolInformation.Kind
import scala.meta.io.AbsolutePath
import scala.meta.pc.SymbolSearch
Expand Down Expand Up @@ -90,6 +91,7 @@ final class WorkspaceSymbolProvider(
packages.visitBootClasspath()
for {
classpathEntry <- buildTargets.allWorkspaceJars
if classpathEntry.isValidClasspathEntry()
} {
packages.visit(classpathEntry)
}
Expand Down

0 comments on commit 4f0af50

Please sign in to comment.