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

bugfix: Don't hang on ProjectDirs #6763

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
20 changes: 14 additions & 6 deletions metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import java.nio.charset.Charset
import java.nio.charset.StandardCharsets
import java.security.MessageDigest

import scala.concurrent.ExecutionContext
import scala.concurrent.ExecutionContextExecutorService
import scala.concurrent.Future
import scala.concurrent.Promise
Expand All @@ -19,6 +20,7 @@ import scala.meta.internal.metals.Directories
import scala.meta.internal.metals.JdkSources
import scala.meta.internal.metals.MetalsBuildClient
import scala.meta.internal.metals.MetalsEnrichments._
import scala.meta.internal.metals.MetalsProjectDirectories
import scala.meta.internal.metals.MetalsServerConfig
import scala.meta.internal.metals.QuietInputStream
import scala.meta.internal.metals.SocketConnection
Expand All @@ -33,7 +35,6 @@ import scala.meta.io.AbsolutePath

import ch.epfl.scala.bsp4j.BspConnectionDetails
import com.google.gson.Gson
import dev.dirs.ProjectDirectories

/**
* Implements BSP server discovery, named "BSP Connection Protocol" in the spec.
Expand Down Expand Up @@ -202,11 +203,18 @@ final class BspServers(
}

object BspServers {
def globalInstallDirectories: List[AbsolutePath] = {
val dirs = ProjectDirectories.fromPath("bsp")
List(dirs.dataLocalDir, dirs.dataDir).distinct
.map(path => Try(AbsolutePath(path)).toOption)
.flatten
def globalInstallDirectories(implicit
ec: ExecutionContext
): List[AbsolutePath] = {
val dirs = MetalsProjectDirectories.fromPath("bsp")
dirs match {
case Some(dirs) =>
List(dirs.dataLocalDir, dirs.dataDir).distinct
.map(path => Try(AbsolutePath(path)).toOption)
.flatten
case None =>
Nil
}
}

def readInBspConfig(
Expand Down
55 changes: 31 additions & 24 deletions metals/src/main/scala/scala/meta/internal/metals/BloopServers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import java.util.concurrent.atomic.AtomicInteger

import scala.annotation.tailrec
import scala.collection.concurrent.TrieMap
import scala.concurrent.ExecutionContext
import scala.concurrent.ExecutionContextExecutorService
import scala.concurrent.Future
import scala.concurrent.Promise
Expand All @@ -36,7 +37,6 @@ import bloop.rifle.BloopRifleConfig
import bloop.rifle.BloopRifleLogger
import bloop.rifle.BspConnection
import bloop.rifle.BspConnectionAddress
import dev.dirs.ProjectDirectories

/**
* Establishes a connection with a bloop server using Bloop Launcher.
Expand All @@ -61,8 +61,11 @@ final class BloopServers(

import BloopServers._

private def metalsJavaHome = sys.props.get("java.home")

private def metalsJavaHome = sys.props
.get("java.home")
.orElse(sys.env.get("JAVA_HOME"))
private val bloopWorkingDir = createBloopWorkingDir
private val bloopDaemonDir = bloopWorkingDir.resolve("daemon")
private val folderIdMap = TrieMap.empty[AbsolutePath, Int]

def shutdownServer(): Boolean = {
Expand Down Expand Up @@ -106,7 +109,7 @@ final class BloopServers(
.recover { case NonFatal(e) =>
Try(
// Bloop output
BloopServers.bloopDaemonDir.resolve("output").readText
bloopDaemonDir.resolve("output").readText
).foreach {
scribe.error(_)
}
Expand Down Expand Up @@ -181,11 +184,6 @@ final class BloopServers(

}

private def metalsJavaHome(userConfiguration: UserConfiguration) =
sys.props
.get("java.home")
.orElse(sys.env.get("JAVA_HOME"))

private lazy val bloopLogger: BloopRifleLogger = new BloopRifleLogger {
def info(msg: => String): Unit = scribe.info(msg)
def debug(msg: => String, ex: Throwable): Unit = scribe.debug(msg, ex)
Expand Down Expand Up @@ -355,8 +353,7 @@ final class BloopServers(
} else {
scribe.info("No running Bloop server found, starting one.")
val ext = if (Properties.isWin) ".exe" else ""
val metalsJavaHomeOpt = metalsJavaHome(userConfiguration)
val javaCommand = metalsJavaHomeOpt match {
val javaCommand = metalsJavaHome match {
case Some(metalsJavaHome) =>
Paths.get(metalsJavaHome).resolve(s"bin/java$ext").toString
case None => "java"
Expand Down Expand Up @@ -436,19 +433,29 @@ object BloopServers {
// Needed for creating unique socket files for each bloop connection
private[BloopServers] val connectionCounter = new AtomicInteger(0)

private val bloopDirectories = {
// Scala CLI is still used since we wanted to avoid having two separate servers
ProjectDirectories.from(null, null, "ScalaCli")
}

lazy val bloopDaemonDir: AbsolutePath =
bloopWorkingDir.resolve("daemon")

lazy val bloopWorkingDir: AbsolutePath = {
val baseDir =
if (Properties.isMac) bloopDirectories.cacheDir
else bloopDirectories.dataLocalDir
AbsolutePath(Paths.get(baseDir).resolve("bloop"))
def createBloopWorkingDir(implicit ec: ExecutionContext): AbsolutePath = {

val baseDir = MetalsProjectDirectories.from(null, null, "ScalaCli") match {
case None =>
val userHome = Paths.get(System.getProperty("user.home"))

val potential =
if (Properties.isWin) userHome.resolve("AppData/Local/ScalaCli/data")
else if (Properties.isMac) userHome.resolve("Library/Caches/ScalaCli")
else userHome.resolve(".local/share/scalacli")
Files.createDirectories(potential)
if (potential.toFile.exists()) potential
else
throw new IllegalStateException(
s"Could not create directory $potential for Bloop, please try using a different BSP server and reporting your issue."
)
case Some(bloopDirectories) =>
val baseDir =
if (Properties.isMac) bloopDirectories.cacheDir
else bloopDirectories.dataLocalDir
Paths.get(baseDir)
}
AbsolutePath(baseDir.resolve("bloop"))
}

def fetchBloop(version: String): Either[Throwable, Seq[File]] = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package scala.meta.internal.metals

import scala.concurrent.Await
import scala.concurrent.ExecutionContext
import scala.concurrent.Future
import scala.concurrent.duration._
import scala.util.Failure
import scala.util.Success
import scala.util.Try

import dev.dirs.ProjectDirectories

object MetalsProjectDirectories {

def from(qualifier: String, organization: String, application: String)(
implicit ec: ExecutionContext
): Option[ProjectDirectories] =
wrap { () =>
ProjectDirectories.from(qualifier, organization, application)
}

def fromPath(path: String)(implicit
ec: ExecutionContext
): Option[ProjectDirectories] =
wrap { () =>
ProjectDirectories.fromPath(path)
}

private def wrap(
f: () => ProjectDirectories
)(implicit ec: ExecutionContext): Option[ProjectDirectories] = {
Try {
val dirs = Future { f() }
Await.result(dirs, 10.seconds)

} match {
case Failure(exception) =>
scribe.error("Failed to get project directories", exception)
None
case Success(value) => Some(value)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package scala.meta.internal.metals
import java.nio.charset.Charset
import java.nio.charset.StandardCharsets

import scala.concurrent.ExecutionContext

import scala.meta.internal.bsp.BspServers
import scala.meta.internal.metals.Buffers
import scala.meta.internal.metals.ClasspathSearch
Expand Down Expand Up @@ -69,7 +71,8 @@ object MetalsServerInputs {
time = Time.system,
initialServerConfig = MetalsServerConfig.default,
initialUserConfig = UserConfiguration.default,
bspGlobalDirectories = BspServers.globalInstallDirectories,
bspGlobalDirectories =
BspServers.globalInstallDirectories(ExecutionContext.global),
mtagsResolver = MtagsResolver.default(),
onStartCompilation = () => (),
redirectSystemOut = true,
Expand Down
25 changes: 13 additions & 12 deletions metals/src/main/scala/scala/meta/internal/metals/Trace.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,34 @@ import java.nio.file.Paths
import java.nio.file.StandardOpenOption

import scala.annotation.tailrec
import scala.concurrent.ExecutionContext
import scala.util.Try

import scala.meta.internal.io.PathIO
import scala.meta.io.AbsolutePath

import dev.dirs.ProjectDirectories

/**
* Manages JSON-RPC tracing of incoming/outgoing messages via BSP and LSP.
*/
object Trace {
// jvm-directories can fail for less common OS versions: https://github.com/soc/directories-jvm/issues/17
val globalDirectory: Option[AbsolutePath] =
def globalDirectory(implicit ec: ExecutionContext): Option[AbsolutePath] =
Try {
val projectDirectories =
ProjectDirectories.from("org", "scalameta", "metals")
MetalsProjectDirectories.from("org", "scalameta", "metals")
// NOTE(olafur): strictly speaking we should use `dataDir` instead of `cacheDir` but on
// macOS this maps to `$HOME/Library/Application Support` which has an annoying space in
// the path making it difficult to tail/cat from the terminal and cmd+click from VS Code.
// Instead, we use the `cacheDir` which has no spaces. The logs are safe to remove so
// putting them in the "cache directory" makes more sense compared to the "config directory".
val cacheDir = Paths.get(projectDirectories.cacheDir)
// https://github.com/scalameta/metals/issues/5590
// deal with issue on windows and PowerShell, which would cause us to create a null directory in the workspace
if (cacheDir.isAbsolute())
Some(AbsolutePath(cacheDir))
else None
projectDirectories.flatMap { dirs =>
val cacheDir = Paths.get(dirs.cacheDir)
// https://github.com/scalameta/metals/issues/5590
// deal with issue on windows and PowerShell, which would cause us to create a null directory in the workspace
if (cacheDir.isAbsolute())
Some(AbsolutePath(cacheDir))
else None
}
}.toOption.flatten

private val localDirectory: AbsolutePath =
Expand All @@ -44,7 +45,7 @@ object Trace {
// normally be fine, but in others like nvim where "workspace" doesn't really
// exist, we can only rely on the rootUri that is passed in, but we don't have
// access to that yet when we use this.
val metalsLog: AbsolutePath =
def metalsLog(implicit ec: ExecutionContext): AbsolutePath =
globalDirectory.getOrElse(localDirectory).resolve("metals.log")

def protocolTracePath(
Expand All @@ -63,7 +64,7 @@ object Trace {
def setupTracePrinter(
protocolName: String,
workspace: AbsolutePath = PathIO.workingDirectory,
): Option[PrintWriter] = {
)(implicit ec: ExecutionContext): Option[PrintWriter] = {
val metalsDir = workspace.resolve(".metals")
val tracePaths = (metalsDir :: globalDirectory.toList).map(dir =>
protocolTracePath(protocolName, dir)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ private[debug] object DebugProxy {
workspace: AbsolutePath,
endpoint: RemoteEndpoint,
name: String,
): RemoteEndpoint =
)(implicit ec: ExecutionContext): RemoteEndpoint =
Trace.setupTracePrinter(name, workspace) match {
case Some(trace) => new EndpointLogger(endpoint, trace)
case None => endpoint
Expand Down
4 changes: 2 additions & 2 deletions metals/src/main/scala/scala/meta/metals/Main.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ object Main extends SupportedScalaVersions {
}
val systemIn = System.in
val systemOut = System.out
MetalsLogger.redirectSystemOut(Trace.metalsLog)
val tracePrinter = Trace.setupTracePrinter("LSP")
val exec = Executors.newCachedThreadPool()
val ec = ExecutionContext.fromExecutorService(exec)
MetalsLogger.redirectSystemOut(Trace.metalsLog(ec))
val tracePrinter = Trace.setupTracePrinter("LSP")(ec)
val sh = Executors.newSingleThreadScheduledExecutor()
val server = new MetalsLanguageServer(ec, sh)
try {
Expand Down
Loading