Merge pull request #410 from olafurpg/sbt

Run official sbt scripts instead of sbt launcher with custom .sbtopts and .jvmopts logic

build.sbt

@@ -263,7 +263,7 @@ lazy val bench = project
     moduleName := "metals-bench",
     libraryDependencies ++= List(
       // for measuring memory usage
-      "org.spire-math" %% "clouseau" % "0.2.2",
+      "org.spire-math" %% "clouseau" % "0.2.2"
     )
   )
   .dependsOn(unit)

docs/editors/vscode.md

@@ -28,7 +28,9 @@ Next, open a directory containing a `build.sbt` file. The extension activates
 when a `*.scala` or `*.sbt` file is opened.
 
 ```scala mdoc:editor:vscode
+Update the "Sbt Launcher" setting to use a custom `sbt` script instead of the embedded launcher.
 
+![Sbt Launcher](assets/vscode-sbt-launcher.png)
 ```
 
 ```scala mdoc:command-palette:vscode

metals-docs/src/main/scala/docs/TextEditorModifier.scala

@@ -11,6 +11,13 @@ class TextEditorModifier extends StringModifier {
       code: Input,
       reporter: Reporter
   ): String = {
+    val sections = code.text.split("---+").lift.andThen(_.filterNot(_.isEmpty))
+    val sbtLauncher = sections(0).getOrElse(
+      """
+        |Update the server property `-Dmetals.sbt-launcher=/path/to/sbt` to
+        |use a custom sbt script instead of the embedded launcher.
+      """.stripMargin
+    )
     s"""
        |## Importing a build
        |
@@ -22,7 +29,7 @@ class TextEditorModifier extends StringModifier {
        |- "Not now" disables this prompt for 2 minutes.
        |- "Don't show again" disables this prompt forever, use `rm -rf .metals/` to re-enable
        |  the prompt.
-       |- Behind the scenese, Metals uses [Bloop](https://scalacenter.github.io/bloop/) to
+       |- Behind the scenes, Metals uses [Bloop](https://scalacenter.github.io/bloop/) to
        |  import sbt builds, but you don't need Bloop installed on your machine to run this step.
        |
        |Once the import step completes, compilation starts for your open `*.scala`
@@ -31,6 +38,14 @@ class TextEditorModifier extends StringModifier {
        |Once the sources have compiled successfully, you can navigate the codebase with
        |"goto definition" with `Cmd+Click`.
        |
+       |### Custom sbt launcher
+       |
+       |By default, Metals runs an embedded `sbt-launch.jar` launcher that respects `.sbtopts` and `.jvmopts`.
+       |The environment variables `SBT_OPTS` and `JAVA_OPTS` are also respected, but may not available to the Metals
+       |process in case it's started from a GUI application.
+       |
+       |$sbtLauncher
+       |
        |### Speeding up import
        |
        |The "Import build" step can take a long time, especially the first time you

metals/src/main/scala/scala/meta/internal/metals/BloopInstall.scala

@@ -66,13 +66,12 @@ final class BloopInstall(
         )
         val jarArgs = List(
           "-jar",
-          embedded.sbtLauncher.toString()
+          embedded.embeddedSbtLauncher.toString()
         )
         List(
           javaArgs,
-          SbtOpts.loadFrom(workspace),
-          JvmOpts.loadFrom(workspace),
-          userConfig().sbtOpts,
+          SbtOpts.fromWorkspace(workspace),
+          JvmOpts.fromWorkspace(workspace),
           jarArgs,
           sbtArgs
         ).flatten

metals/src/main/scala/scala/meta/internal/metals/Embedded.scala

@@ -28,20 +28,14 @@ final class Embedded(
     }
   }
 
-  def sbtLauncher: AbsolutePath = {
-    userConfig().sbtLauncher
-      .map(AbsolutePath(_))
-      .getOrElse(embeddedSbtLauncher)
-  }
-
   /**
    * Returns path to a local copy of sbt-launch.jar.
    *
    * We use embedded sbt-launch.jar instead of user `sbt` command because
    * we can't rely on `sbt` resolving correctly when using system processes, at least
    * it failed on Windows when I tried it.
    */
-  private lazy val embeddedSbtLauncher: AbsolutePath = {
+  lazy val embeddedSbtLauncher: AbsolutePath = {
     val embeddedLauncher = this.getClass.getResourceAsStream("/sbt-launch.jar")
     val out = Files.createTempDirectory("metals").resolve("sbt-launch.jar")
     out.toFile.deleteOnExit()

metals/src/main/scala/scala/meta/internal/metals/JvmOpts.scala

@@ -13,7 +13,7 @@ import scala.meta.io.AbsolutePath
  */
 object JvmOpts {
 
-  def loadFrom(workspace: AbsolutePath): List[String] = {
+  def fromWorkspace(workspace: AbsolutePath): List[String] = {
     val jvmOpts = workspace.resolve(".jvmopts")
     if (jvmOpts.isFile && Files.isReadable(jvmOpts.toNIO)) {
       val text = FileIO.slurp(jvmOpts, StandardCharsets.UTF_8)
@@ -23,4 +23,11 @@ object JvmOpts {
     }
   }
 
+  def fromEnvironment: List[String] = {
+    Option(System.getenv("JVM_OPTS")) match {
+      case Some(value) => value.split(" ").toList
+      case None => Nil
+    }
+  }
+
 }

metals/src/main/scala/scala/meta/internal/metals/SbtOpts.scala

@@ -13,7 +13,7 @@ import scala.meta.io.AbsolutePath
  */
 object SbtOpts {
 
-  def loadFrom(workspace: AbsolutePath): List[String] = {
+  def fromWorkspace(workspace: AbsolutePath): List[String] = {
     val sbtOpts = workspace.resolve(".sbtopts")
     if (sbtOpts.isFile && Files.isReadable(sbtOpts.toNIO)) {
       val text = FileIO.slurp(sbtOpts, StandardCharsets.UTF_8)
@@ -23,6 +23,13 @@ object SbtOpts {
     }
   }
 
+  def fromEnvironment: List[String] = {
+    Option(System.getenv("SBT_OPTS")) match {
+      case Some(value) => process(value.split(" ").toList)
+      case None => Nil
+    }
+  }
+
   private val noShareOpts =
     "-Dsbt.global.base=project/.sbtboot -Dsbt.boot.directory=project/.boot -Dsbt.ivy.home=project/.ivy"
   private val noGlobalOpts = "-Dsbt.global.base=project/.sbtboot"

metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala

@@ -13,8 +13,6 @@ import scala.util.Try
  */
 case class UserConfiguration(
     javaHome: Option[String] = None,
-    sbtLauncher: Option[String] = None,
-    sbtOpts: List[String] = Nil,
     sbtScript: Option[String] = None
 )
 
@@ -27,35 +25,15 @@ object UserConfiguration {
       "Java Home directory",
       "The Java Home directory used for indexing JDK sources and locating the `java` binary."
     ),
-    UserConfigurationOption(
-      "sbt-launcher",
-      "Metals embedded sbt-launch.jar.",
-      "/usr/local/Cellar/sbt/1.2.6/libexec/bin/sbt-launch.jar",
-      "sbt launcher jar",
-      "Optional sbt-launch.jar launcher to use when running `sbt bloopInstall`."
-    ),
-    UserConfigurationOption(
-      "sbt-options",
-      """empty string `""`.""",
-      "-Dsbt.override.build.repos=true -Divy.home=/home/ivy-cache",
-      "sbt JVM options",
-      """Additional space separated JVM options used for the `sbt bloopInstall` step.
-        |By default, Metals respects custom options in `.jvmopts` and `.sbtopts` of the workspace root directory,
-        |it's recommended to use those instead of customizing this setting. The benefit of `.jvmopts` and `.sbtopts`
-        |is that it's respected by other tools such as IntelliJ.
-        |""".stripMargin
-    ),
     UserConfigurationOption(
       "sbt-script",
       """empty string `""`.""",
       "/usr/local/bin/sbt",
       "sbt script",
-      """Custom `sbt` executable to use for running `sbt bloopInstall`, overrides
-        |`sbt-options` and `sbt-launcher` options.
-        |
-        |By default, Metals uses `java -jar sbt-launch.jar` to launch sbt while respecting
-        |`.jvmopts` and `.sbtopts`. In case this option is defined, then Metals
-        |executes the configured sbt script instead.
+      """Optional absolute path to an `sbt` executable to use for running `sbt bloopInstall`.
+        |By default, Metals uses `java -jar sbt-launch.jar` with an embedded launcher while respecting
+        |`.jvmopts` and `.sbtopts`. Update this setting if your `sbt` script requires more customizations
+        |like using environment variables.
         |""".stripMargin
     )
   )
@@ -90,22 +68,13 @@ object UserConfiguration {
 
     val javaHome =
       getKey("java-home")
-    val sbtLauncher =
-      getKey("sbt-launcher")
-    val sbtOpts =
-      getKey("sbt-options") match {
-        case Some(value) => value.split(" ").toList
-        case None => Nil
-      }
     val sbtScript =
       getKey("sbt-script")
 
     if (errors.isEmpty) {
       Right(
         UserConfiguration(
           javaHome,
-          sbtLauncher,
-          sbtOpts,
           sbtScript
         )
       )

tests/unit/src/test/scala/tests/SbtOptsSuite.scala

@@ -8,8 +8,8 @@ object SbtOptsSuite extends BaseSuite {
     test(name) {
       val root = FileLayout.fromString(original)
       val obtained =
-        SbtOpts.loadFrom(root).mkString("\n") ++
-          JvmOpts.loadFrom(root).mkString("\n")
+        SbtOpts.fromWorkspace(root).mkString("\n") ++
+          JvmOpts.fromWorkspace(root).mkString("\n")
       assertNoDiff(obtained, expected)
     }
   }

tests/unit/src/test/scala/tests/UserConfigurationSuite.scala

@@ -51,16 +51,12 @@ object UserConfigurationSuite extends BaseSuite {
     "basic",
     """
       |{
-      | "sbt-launcher": "launch",
       | "java-home": "home",
-      | "sbt-options": "a b",
       | "sbt-script": "script"
       |}
     """.stripMargin
   ) { obtained =>
     assert(obtained.javaHome == Some("home"))
-    assert(obtained.sbtLauncher == Some("launch"))
-    assert(obtained.sbtOpts == List("a", "b"))
     assert(obtained.sbtScript == Some("script"))
   }
 
@@ -69,8 +65,6 @@ object UserConfigurationSuite extends BaseSuite {
     "{}"
   ) { obtained =>
     assert(obtained.javaHome.isEmpty)
-    assert(obtained.sbtLauncher.isEmpty)
-    assert(obtained.sbtOpts.isEmpty)
     assert(obtained.sbtScript.isEmpty)
   }
 
@@ -88,15 +82,11 @@ object UserConfigurationSuite extends BaseSuite {
       |}
     """.stripMargin,
     Map(
-      "metals.sbt-launcher" -> "launch",
       "metals.java-home" -> "home",
-      "metals.sbt-options" -> "a b",
       "metals.sbt-script" -> "script"
     )
   ) { obtained =>
     assert(obtained.javaHome == Some("home"))
-    assert(obtained.sbtLauncher == Some("launch"))
-    assert(obtained.sbtOpts == List("a", "b"))
     assert(obtained.sbtScript == Some("script"))
   }
 
@@ -130,11 +120,11 @@ object UserConfigurationSuite extends BaseSuite {
     "type-mismatch",
     """
       |{
-      | "sbt-launcher": []
+      | "sbt-script": []
       |}
     """.stripMargin,
     """
-      |json error: key 'sbt-launcher' should have value of type string but obtained []
+      |json error: key 'sbt-script' should have value of type string but obtained []
     """.stripMargin
   )
 }