Merge pull request #8964 from eed3si9n/bport/network-client

[2.0.x] bport: sbtn-related changes

launcher-package/integration-test/src/test/scala/BspConfigTest.scala

@@ -1,84 +1,90 @@
-package example.test
+package example.test
-
+
-import scala.sys.process.*
+import scala.sys.process.*
-import java.io.File
+import java.io.File
-import java.util.Locale
+import java.util.Locale
-import sbt.io.IO
+import sbt.io.IO
-import verify.BasicTestSuite
+import verify.BasicTestSuite
-
+
-// Test for issues #7792/#7794: BSP config generation and argv execution
+// Test for issues #7792/#7794: BSP config generation and argv execution
-object BspConfigTest extends BasicTestSuite:
+object BspConfigTest extends BasicTestSuite:
-  lazy val isWindows: Boolean =
+  lazy val isWindows: Boolean =
-    sys.props("os.name").toLowerCase(Locale.ENGLISH).contains("windows")
+    sys.props("os.name").toLowerCase(Locale.ENGLISH).contains("windows")
-  lazy val sbtScript = IntegrationTestPaths.sbtScript(isWindows)
+  lazy val sbtScript = IntegrationTestPaths.sbtScript(isWindows)
-
+
-  private def launcherCmd = LauncherTestHelper.launcherCommand(sbtScript.getAbsolutePath)
+  private def launcherCmd = LauncherTestHelper.launcherCommand(sbtScript.getAbsolutePath)
-
+
-  def sbtProcessInDir(dir: File)(args: String*) =
+  def sbtProcessInDir(dir: File)(args: String*) =
-    Process(
+    Process(
-      launcherCmd ++ args,
+      launcherCmd ++ args,
-      dir,
+      dir,
-      "JAVA_OPTS" -> "",
+      "JAVA_OPTS" -> "",
-      "SBT_OPTS" -> ""
+      "SBT_OPTS" -> ""
-    )
+    )
-
+
-  test("sbt bspConfig") {
+  test("sbt bspConfig") {
-    import ujson.*
+    import ujson.*
-
+
-    IO.withTemporaryDirectory { tmp =>
+    IO.withTemporaryDirectory { tmp =>
-      // Create minimal build.sbt for the test project
+      // Create minimal build.sbt for the test project
-      IO.write(new File(tmp, "build.sbt"), """name := "test-bsp-config"""")
+      IO.write(new File(tmp, "build.sbt"), """name := "test-bsp-config"""")
-
+
-      // Run bspConfig to generate .bsp/sbt.json
+      // Run bspConfig to generate .bsp/sbt.json
-      val configResult = sbtProcessInDir(tmp)("bspConfig", "--batch").!
+      val configResult = sbtProcessInDir(tmp)("bspConfig", "--batch").!
-      assert(configResult == 0, s"bspConfig command failed with exit code $configResult")
+      assert(configResult == 0, s"bspConfig command failed with exit code $configResult")
-
+
-      // Verify .bsp/sbt.json exists
+      // Verify .bsp/sbt.json exists
-      val bspFile = new File(tmp, ".bsp/sbt.json")
+      val bspFile = new File(tmp, ".bsp/sbt.json")
-      assert(bspFile.exists, ".bsp/sbt.json should exist after running bspConfig")
+      assert(bspFile.exists, ".bsp/sbt.json should exist after running bspConfig")
-
+
-      // Parse and verify JSON content
+      // Parse and verify JSON content
-      val content = IO.read(bspFile)
+      val content = IO.read(bspFile)
-      val json = ujson.read(content)
+      val json = ujson.read(content)
-
+
-      // Extract argv array from JSON
+      // Extract argv array from JSON
-      val argvValue = json.obj.get("argv")
+      val argvValue = json.obj.get("argv")
-      assert(argvValue.isDefined, "argv field not found in sbt.json")
+      assert(argvValue.isDefined, "argv field not found in sbt.json")
-
+
-      val argv = argvValue.get.arr.map(_.str).toVector
+      val argv = argvValue.get.arr.map(_.str).toVector
-
+
-      // Verify argv structure
+      // Verify argv structure
-      assert(argv.nonEmpty, "argv should not be empty")
+      assert(argv.nonEmpty, "argv should not be empty")
-      assert(argv.head.contains("java"), s"argv should start with java command, got: ${argv.head}")
+      // When sbt script is available, argv uses the sbt script with "bsp" command.
-      assert(argv.contains("-bsp"), s"argv should contain -bsp flag, got: $argv")
+      // When not, argv falls back to direct java invocation with "-bsp" flag.
-
+      val usesSbtScript = argv.last == "bsp" && !argv.head.contains("java")
-      // Test execution of the generated argv
+      val usesJavaDirect = argv.head.contains("java") && argv.contains("-bsp")
-      // Run the BSP command with a very short timeout to verify it starts correctly
+      assert(
-      // We just need to verify the command doesn't fail immediately on startup
+        usesSbtScript || usesJavaDirect,
-      if (!isWindows) {
+        s"argv should either use sbt script with 'bsp' command or java with '-bsp' flag, got: $argv"
-        // On Unix, we can test the argv execution
+      )
-        // Create a process and check if it starts (will timeout waiting for BSP input)
+
-        val process = Process(argv.toSeq, tmp)
+      // Test execution of the generated argv
-        val processBuilder = process.run(ProcessLogger(_ => (), _ => ()))
+      // Run the BSP command with a very short timeout to verify it starts correctly
-
+      // We just need to verify the command doesn't fail immediately on startup
-        // Give it a moment to fail if it's going to fail immediately
+      if (!isWindows) {
-        Thread.sleep(500)
+        // On Unix, we can test the argv execution
-
+        // Create a process and check if it starts (will timeout waiting for BSP input)
-        // If still running, it means the BSP server started successfully
+        val process = Process(argv.toSeq, tmp)
-        val isAlive = processBuilder.isAlive()
+        val processBuilder = process.run(ProcessLogger(_ => (), _ => ()))
-        processBuilder.destroy()
+
-
+        // Give it a moment to fail if it's going to fail immediately
-        // The process should either still be alive (waiting for BSP messages)
+        Thread.sleep(500)
-        // or have exited with code 0 (graceful)
+
-        if (!isAlive) {
+        // If still running, it means the BSP server started successfully
-          val exitCode = processBuilder.exitValue()
+        val isAlive = processBuilder.isAlive()
-          assert(
+        processBuilder.destroy()
-            exitCode == 0 || exitCode == 143, // 143 = SIGTERM from destroy()
+
-            s"BSP process failed with exit code $exitCode"
+        // The process should either still be alive (waiting for BSP messages)
-          )
+        // or have exited with code 0 (graceful)
-        }
+        if (!isAlive) {
-      }
+          val exitCode = processBuilder.exitValue()
-    }
+          assert(
-    ()
+            exitCode == 0 || exitCode == 143, // 143 = SIGTERM from destroy()
-  }
+            s"BSP process failed with exit code $exitCode"
-end BspConfigTest
+          )
+        }
+      }
+    }
+    ()
+  }
+end BspConfigTest

launcher-package/src/universal/bin/sbt.bat

@@ -442,6 +442,11 @@ if "%~0" == "shutdownall" (
   goto args_loop
 )
 
+if "%~0" == "bsp" (
+  set sbt_args_client=0
+  goto args_loop
+)
+
 if "%~0" == "--script-version" (
   set sbt_args_print_sbt_script_version=1
   goto args_loop

main-command/src/main/scala/sbt/internal/client/NetworkClient.scala

@@ -1193,6 +1193,77 @@ object NetworkClient {
   private[client] val noTab = "--no-tab"
   private[client] val noStdErr = "--no-stderr"
   private[client] val sbtBase = "--sbt-base-directory"
+  // Launcher flags that take a value argument (flag + next arg should be skipped)
+  private[client] val launcherValueFlags: Set[String] = Set(
+    "-mem",
+    "--mem",
+    "-jvm-debug",
+    "--jvm-debug",
+    "-sbt-jar",
+    "--sbt-jar",
+    "-sbt-cache",
+    "--sbt-cache",
+    "-sbt-version",
+    "--sbt-version",
+    "-java-home",
+    "--java-home",
+    "-ivy",
+    "--ivy",
+    "-sbt-boot",
+    "--sbt-boot",
+    "-sbt-dir",
+    "--sbt-dir",
+  )
+  // Launcher flags that take no value (flag itself should be skipped)
+  private[client] val launcherNoValueFlags: Set[String] = Set(
+    "-client",
+    "--client",
+    "--server",
+    "--jvm-client",
+    "-h",
+    "-help",
+    "--help",
+    "-v",
+    "-verbose",
+    "--verbose",
+    "-V",
+    "-version",
+    "--version",
+    "--numeric-version",
+    "--script-version",
+    "-d",
+    "-debug",
+    "--debug",
+    "-debug-inc",
+    "--debug-inc",
+    "-batch",
+    "--batch",
+    "--no-hide-jdk-warnings",
+    "-no-colors",
+    "--no-colors",
+    "-timings",
+    "--timings",
+    "-traces",
+    "--traces",
+    "-no-server",
+    "--no-server",
+    "-no-share",
+    "--no-share",
+    "-no-global",
+    "--no-global",
+    "-allow-empty",
+    "--allow-empty",
+    "-sbt-create",
+    "--sbt-create",
+    "shutdownall",
+  )
+  // Prefixes for launcher flags using = syntax
+  private[client] val launcherEqPrefixes: Seq[String] = Seq(
+    "--supershell=",
+    "-supershell=",
+    "--color=",
+    "-color=",
+  )
   private[client] def parseArgs(args: Array[String]): Arguments = {
     val defaultSbtScript = if (Properties.isWin) "sbt.bat" else "sbt"
     var sbtScript = Properties.propOrNone("sbt.script")
@@ -1229,8 +1300,19 @@ object NetworkClient {
         case "--sbt-launch-jar" if i + 1 < sanitized.length =>
           i += 1
           launchJar = Option(sanitized(i).replace("%20", " "))
-        case "-bsp" | "--bsp"        => bsp = true
+        case "-bsp" | "--bsp" | "bsp" => bsp = true
-        case a if !a.startsWith("-") => commandArgs += a
+        case "-no-server" | "--no-server" =>
+          System.setProperty("sbt.server.autostart", "false")
+        case a if a.startsWith("--autostart=") =>
+          System.setProperty("sbt.server.autostart", a.stripPrefix("--autostart="))
+        case a if a.startsWith("-autostart=") =>
+          System.setProperty("sbt.server.autostart", a.stripPrefix("-autostart="))
+        case a if launcherValueFlags.contains(a) =>
+          if (i + 1 < sanitized.length) i += 1
+        case a if launcherNoValueFlags.contains(a)                => ()
+        case a if launcherEqPrefixes.exists(p => a.startsWith(p)) => ()
+        case a if a.startsWith("-J")                              => ()
+        case a if !a.startsWith("-")                              => commandArgs += a
         case a @ SysProp(key, value) =>
           System.setProperty(key, value)
           sbtArguments += a

main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala

@@ -0,0 +1,146 @@
+/*
+ * sbt
+ * Copyright 2023, Scala center
+ * Copyright 2011 - 2022, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package sbt.internal.client
+
+import verify.BasicTestSuite
+
+object NetworkClientParseArgsTest extends BasicTestSuite:
+
+  private def parse(args: String*): NetworkClient.Arguments =
+    NetworkClient.parseArgs(args.toArray)
+
+  // -- Value-taking launcher flags --
+
+  test("-mem 10000 compile drops -mem and its value"):
+    val result = parse("-mem", "10000", "compile")
+    assert(!result.sbtArguments.contains("-mem"))
+    assert(!result.sbtArguments.contains("10000"))
+    assert(!result.commandArguments.contains("-mem"))
+    assert(!result.commandArguments.contains("10000"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("--mem 10000 compile drops --mem and its value"):
+    val result = parse("--mem", "10000", "compile")
+    assert(!result.sbtArguments.contains("--mem"))
+    assert(!result.sbtArguments.contains("10000"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-jvm-debug 5005 compile drops both flag and port"):
+    val result = parse("-jvm-debug", "5005", "compile")
+    assert(!result.sbtArguments.contains("-jvm-debug"))
+    assert(!result.sbtArguments.contains("5005"))
+    assert(!result.commandArguments.contains("-jvm-debug"))
+    assert(!result.commandArguments.contains("5005"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-java-home /path/to/jdk compile drops both"):
+    val result = parse("-java-home", "/path/to/jdk", "compile")
+    assert(!result.sbtArguments.contains("-java-home"))
+    assert(!result.sbtArguments.contains("/path/to/jdk"))
+    assert(!result.commandArguments.contains("-java-home"))
+    assert(!result.commandArguments.contains("/path/to/jdk"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-mem at end of args with no value does not crash"):
+    val result = parse("-mem")
+    assert(!result.sbtArguments.contains("-mem"))
+    assert(result.commandArguments.isEmpty)
+
+  // -- No-value launcher flags --
+
+  test("--client compile drops --client"):
+    val result = parse("--client", "compile")
+    assert(!result.sbtArguments.contains("--client"))
+    assert(!result.commandArguments.contains("--client"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-client compile drops -client"):
+    val result = parse("-client", "compile")
+    assert(!result.sbtArguments.contains("-client"))
+    assert(!result.commandArguments.contains("-client"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-debug is dropped"):
+    val result = parse("-debug", "compile")
+    assert(!result.sbtArguments.contains("-debug"))
+    assert(!result.commandArguments.contains("-debug"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-batch is dropped"):
+    val result = parse("-batch", "compile")
+    assert(!result.sbtArguments.contains("-batch"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-allow-empty is dropped"):
+    val result = parse("-allow-empty", "compile")
+    assert(!result.sbtArguments.contains("-allow-empty"))
+    assert(!result.commandArguments.contains("-allow-empty"))
+    assert(result.commandArguments.contains("compile"))
+
+  // -- Eq-syntax flags and -J* --
+
+  test("--supershell=false is dropped"):
+    val result = parse("--supershell=false", "compile")
+    assert(!result.sbtArguments.exists(_.contains("supershell")))
+    assert(result.commandArguments.contains("compile"))
+
+  test("--color=never is dropped"):
+    val result = parse("--color=never", "compile")
+    assert(!result.sbtArguments.exists(_.contains("color")))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-J-Xss4m is dropped"):
+    val result = parse("-J-Xss4m", "compile")
+    assert(!result.sbtArguments.contains("-J-Xss4m"))
+    assert(result.commandArguments.contains("compile"))
+
+  // -- Flags that should be preserved --
+
+  test("-Dfoo=bar compile forwards -D property to sbtArguments"):
+    val result = parse("-Dfoo=bar", "compile")
+    assert(result.sbtArguments.exists(_.contains("-Dfoo=bar")))
+    assert(result.commandArguments.contains("compile"))
+
+  test("-bsp is still recognized"):
+    val result = parse("-bsp")
+    assert(result.bsp)
+
+  test("--sbt-launch-jar is preserved"):
+    val result = parse("--sbt-launch-jar", "/path/to/sbt-launch.jar", "compile")
+    assert(result.sbtLaunchJar.contains("/path/to/sbt-launch.jar"))
+    assert(result.commandArguments.contains("compile"))
+
+  test("--sbt-script is preserved"):
+    val result = parse("--sbt-script", "/usr/bin/sbt", "compile")
+    assert(result.sbtScript == "/usr/bin/sbt")
+    assert(result.commandArguments.contains("compile"))
+
+  // -- Combined / integration --
+
+  test("combined: -mem 10000 -Dfoo=bar compile test"):
+    val result = parse("-mem", "10000", "-Dfoo=bar", "compile", "test")
+    assert(!result.sbtArguments.contains("-mem"))
+    assert(!result.sbtArguments.contains("10000"))
+    assert(result.sbtArguments.exists(_.contains("-Dfoo=bar")))
+    assert(result.commandArguments.contains("compile"))
+    assert(result.commandArguments.contains("test"))
+
+  test("combined: --client -batch -java-home /jdk --color=never -Dfoo=bar compile"):
+    val result =
+      parse("--client", "-batch", "-java-home", "/jdk", "--color=never", "-Dfoo=bar", "compile")
+    assert(!result.sbtArguments.contains("--client"))
+    assert(!result.sbtArguments.contains("-batch"))
+    assert(!result.sbtArguments.contains("-java-home"))
+    assert(!result.sbtArguments.contains("/jdk"))
+    assert(!result.sbtArguments.exists(_.contains("color=never")))
+    assert(result.sbtArguments.exists(_.contains("-Dfoo=bar")))
+    assert(result.commandArguments.contains("compile"))
+    assert(result.commandArguments.size == 1)
+
+end NetworkClientParseArgsTest

main/src/main/scala/sbt/Main.scala

@@ -75,7 +75,7 @@ private[sbt] object xMain:
         .map(_.trim)
         .filterNot(_ == DashDashServer)
       val isClient: String => Boolean = cmd => (cmd == JavaClient) || (cmd == DashDashClient)
-      val isBsp: String => Boolean = cmd => (cmd == "-bsp") || (cmd == "--bsp")
+      val isBsp: String => Boolean = cmd => (cmd == "-bsp") || (cmd == "--bsp") || (cmd == "bsp")
       val isNew: String => Boolean = cmd => (cmd == "new") || (cmd == "init")
       lazy val isServer = !userCommands.exists(c => isBsp(c) || isClient(c))
       // keep this lazy to prevent project directory created prematurely

protocol/src/main/scala/sbt/internal/bsp/BuildServerConnection.scala

@@ -26,42 +26,48 @@ object BuildServerConnection {
 
   private[sbt] def writeConnectionFile(sbtVersion: String, baseDir: File): Unit = {
     val bspConnectionFile = new File(baseDir, ".bsp/sbt.json")
-    val javaHome = Util.javaHome
-    val classPath = System.getProperty("java.class.path")
 
     val sbtScript = Option(System.getProperty("sbt.script"))
+      .map(_.replace("%20", " "))
+      .filter(_.nonEmpty)
       .orElse(sbtScriptInPath)
-      .map(script => s"-Dsbt.script=$script")
 
-    val sbtOptsArgs = parseSbtOpts(sys.env.get("SBT_OPTS"))
+    val argv = sbtScript match
-
+      case Some(script) =>
-    val sbtLaunchJar = classPath
+        Vector(script, "bsp")
-      .split(File.pathSeparator)
+      case None =>
-      .find(jar => SbtLaunchJar.findFirstIn(jar).nonEmpty)
+        buildFallbackArgv
-      .map(_.replace(" ", "%20"))
-      .map(jar => s"--sbt-launch-jar=$jar")
-
-    val argv =
-      Vector(
-        s"$javaHome/bin/java",
-        "-Xms100m",
-        "-Xmx100m",
-      ) ++
-        sbtOptsArgs ++
-        Vector(
-          "-classpath",
-          classPath,
-        ) ++
-        sbtScript ++
-        Vector("xsbt.boot.Boot", "-bsp") ++
-        (if (sbtScript.isEmpty) sbtLaunchJar else None)
 
     val details = BspConnectionDetails(name, sbtVersion, bspVersion, languages, argv)
     val json = Converter.toJson(details).get
     IO.write(bspConnectionFile, CompactPrinter(json), append = false)
   }
 
-  private def sbtScriptInPath: Option[String] = {
+  private[sbt] def buildFallbackArgv: Vector[String] = {
+    val javaHome = Util.javaHome
+    val classPath = System.getProperty("java.class.path")
+    val sbtOptsArgs = parseSbtOpts(sys.env.get("SBT_OPTS"))
+    val sbtLaunchJar = classPath
+      .split(File.pathSeparator)
+      .find(jar => SbtLaunchJar.findFirstIn(jar).nonEmpty)
+      .map(_.replace(" ", "%20"))
+      .map(jar => s"--sbt-launch-jar=$jar")
+
+    Vector(
+      s"$javaHome/bin/java",
+      "-Xms100m",
+      "-Xmx100m",
+    ) ++
+      sbtOptsArgs ++
+      Vector(
+        "-classpath",
+        classPath,
+      ) ++
+      Vector("xsbt.boot.Boot", "-bsp") ++
+      sbtLaunchJar
+  }
+
+  private[sbt] def sbtScriptInPath: Option[String] = {
     val fileName = if (Properties.isWin) "sbt.bat" else "sbt"
     val envPath = sys.env.collectFirst {
       case (k, v) if k.toUpperCase() == "PATH" => v
@@ -72,7 +78,7 @@ object BuildServerConnection {
     allPaths
       .map(_.resolve(fileName))
       .find(file => Files.exists(file) && Files.isExecutable(file))
-      .map(_.toString.replace(" ", "%20"))
+      .map(_.toString)
   }
 
   private[sbt] def parseSbtOpts(sbtOpts: Option[String]): Vector[String] =

protocol/src/test/scala/sbt/internal/bsp/BuildServerConnectionSpec.scala

@@ -45,3 +45,18 @@ object BuildServerConnectionSpec extends BasicTestSuite:
   test("parseSbtOpts should handle whitespace-separated options"):
     val result = BuildServerConnection.parseSbtOpts(Some("  -Dfoo=bar   -Xmx1g  "))
     assert(result == Vector("-Dfoo=bar", "-Xmx1g"))
+
+  test("sbtScriptInPath should return None when sbt is not in PATH"):
+    val result = BuildServerConnection.sbtScriptInPath
+    result match
+      case Some(path) => assert(path.nonEmpty)
+      case None       => assert(true)
+
+  test("buildFallbackArgv should include java path and -bsp flag"):
+    val argv = BuildServerConnection.buildFallbackArgv
+    assert(argv.head.contains("java"), s"argv should start with java, got: ${argv.head}")
+    assert(argv.contains("-bsp"), s"argv should contain -bsp, got: $argv")
+    assert(argv.contains("-Xms100m"), s"argv should contain -Xms100m, got: $argv")
+    assert(argv.contains("-Xmx100m"), s"argv should contain -Xmx100m, got: $argv")
+    assert(argv.contains("-classpath"), s"argv should contain -classpath, got: $argv")
+    assert(argv.contains("xsbt.boot.Boot"), s"argv should contain xsbt.boot.Boot, got: $argv")

sbt

@@ -770,6 +770,7 @@ process_args () {
  -Dsbt.color=never|-Dsbt.log.noformat=true) addJava "$1" && use_colors=0 && shift ;;
                   "-D*"|-D*) addJava "$1" && shift ;;
                         -J*) addJava "${1:2}" && shift ;;
+                        bsp) use_sbtn=0 && addResidual "-bsp" && shift ;;
                           *) addResidual "$1" && shift ;;
     esac
   done

sbtw/src/main/scala/sbtw/Main.scala

@@ -51,8 +51,11 @@ object Main:
       else System.err.println("[error] sbt requires at least JDK 8+, you have " + javaVer)
       return 1
 
+    val bspMode = opts.residual.exists(a => a == "bsp" || a == "-bsp" || a == "--bsp")
     val clientOpt = opts.client || sys.env.get("SBT_NATIVE_CLIENT").contains("true")
-    val useNativeClient = shouldRunNativeClient(opts.copy(client = clientOpt), buildPropsVersion)
+    val useNativeClient =
+      if bspMode then false
+      else shouldRunNativeClient(opts.copy(client = clientOpt), buildPropsVersion)
 
     if useNativeClient then
       val scriptPath = sbtBinDir.getAbsolutePath.replace("\\", "/") + "/sbt.bat"