diff --git a/launcher-package/integration-test/src/test/scala/BspConfigTest.scala b/launcher-package/integration-test/src/test/scala/BspConfigTest.scala index 70c179fdf..da6d8369d 100644 --- a/launcher-package/integration-test/src/test/scala/BspConfigTest.scala +++ b/launcher-package/integration-test/src/test/scala/BspConfigTest.scala @@ -1,84 +1,90 @@ -package example.test - -import scala.sys.process.* -import java.io.File -import java.util.Locale -import sbt.io.IO -import verify.BasicTestSuite - -// Test for issues #7792/#7794: BSP config generation and argv execution -object BspConfigTest extends BasicTestSuite: - lazy val isWindows: Boolean = - sys.props("os.name").toLowerCase(Locale.ENGLISH).contains("windows") - lazy val sbtScript = IntegrationTestPaths.sbtScript(isWindows) - - private def launcherCmd = LauncherTestHelper.launcherCommand(sbtScript.getAbsolutePath) - - def sbtProcessInDir(dir: File)(args: String*) = - Process( - launcherCmd ++ args, - dir, - "JAVA_OPTS" -> "", - "SBT_OPTS" -> "" - ) - - test("sbt bspConfig") { - import ujson.* - - IO.withTemporaryDirectory { tmp => - // Create minimal build.sbt for the test project - IO.write(new File(tmp, "build.sbt"), """name := "test-bsp-config"""") - - // Run bspConfig to generate .bsp/sbt.json - val configResult = sbtProcessInDir(tmp)("bspConfig", "--batch").! - assert(configResult == 0, s"bspConfig command failed with exit code $configResult") - - // Verify .bsp/sbt.json exists - val bspFile = new File(tmp, ".bsp/sbt.json") - assert(bspFile.exists, ".bsp/sbt.json should exist after running bspConfig") - - // Parse and verify JSON content - val content = IO.read(bspFile) - val json = ujson.read(content) - - // Extract argv array from JSON - val argvValue = json.obj.get("argv") - assert(argvValue.isDefined, "argv field not found in sbt.json") - - val argv = argvValue.get.arr.map(_.str).toVector - - // Verify argv structure - assert(argv.nonEmpty, "argv should not be empty") - assert(argv.head.contains("java"), s"argv should start with java command, got: ${argv.head}") - assert(argv.contains("-bsp"), s"argv should contain -bsp flag, got: $argv") - - // Test execution of the generated argv - // 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 - if (!isWindows) { - // 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) - val processBuilder = process.run(ProcessLogger(_ => (), _ => ())) - - // Give it a moment to fail if it's going to fail immediately - Thread.sleep(500) - - // If still running, it means the BSP server started successfully - val isAlive = processBuilder.isAlive() - processBuilder.destroy() - - // 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 +package example.test + +import scala.sys.process.* +import java.io.File +import java.util.Locale +import sbt.io.IO +import verify.BasicTestSuite + +// Test for issues #7792/#7794: BSP config generation and argv execution +object BspConfigTest extends BasicTestSuite: + lazy val isWindows: Boolean = + sys.props("os.name").toLowerCase(Locale.ENGLISH).contains("windows") + lazy val sbtScript = IntegrationTestPaths.sbtScript(isWindows) + + private def launcherCmd = LauncherTestHelper.launcherCommand(sbtScript.getAbsolutePath) + + def sbtProcessInDir(dir: File)(args: String*) = + Process( + launcherCmd ++ args, + dir, + "JAVA_OPTS" -> "", + "SBT_OPTS" -> "" + ) + + test("sbt bspConfig") { + import ujson.* + + IO.withTemporaryDirectory { tmp => + // Create minimal build.sbt for the test project + IO.write(new File(tmp, "build.sbt"), """name := "test-bsp-config"""") + + // Run bspConfig to generate .bsp/sbt.json + val configResult = sbtProcessInDir(tmp)("bspConfig", "--batch").! + assert(configResult == 0, s"bspConfig command failed with exit code $configResult") + + // Verify .bsp/sbt.json exists + val bspFile = new File(tmp, ".bsp/sbt.json") + assert(bspFile.exists, ".bsp/sbt.json should exist after running bspConfig") + + // Parse and verify JSON content + val content = IO.read(bspFile) + val json = ujson.read(content) + + // Extract argv array from JSON + val argvValue = json.obj.get("argv") + assert(argvValue.isDefined, "argv field not found in sbt.json") + + val argv = argvValue.get.arr.map(_.str).toVector + + // Verify argv structure + assert(argv.nonEmpty, "argv should not be empty") + // When sbt script is available, argv uses the sbt script with "bsp" command. + // When not, argv falls back to direct java invocation with "-bsp" flag. + val usesSbtScript = argv.last == "bsp" && !argv.head.contains("java") + val usesJavaDirect = argv.head.contains("java") && argv.contains("-bsp") + assert( + usesSbtScript || usesJavaDirect, + s"argv should either use sbt script with 'bsp' command or java with '-bsp' flag, got: $argv" + ) + + // Test execution of the generated argv + // 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 + if (!isWindows) { + // 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) + val processBuilder = process.run(ProcessLogger(_ => (), _ => ())) + + // Give it a moment to fail if it's going to fail immediately + Thread.sleep(500) + + // If still running, it means the BSP server started successfully + val isAlive = processBuilder.isAlive() + processBuilder.destroy() + + // 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 diff --git a/launcher-package/src/universal/bin/sbt.bat b/launcher-package/src/universal/bin/sbt.bat index 4dc2416c4..d07156c90 100755 --- a/launcher-package/src/universal/bin/sbt.bat +++ b/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 diff --git a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala index a3bdad2b4..5d5f089e0 100644 --- a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala +++ b/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 a if !a.startsWith("-") => commandArgs += a + case "-bsp" | "--bsp" | "bsp" => bsp = true + 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 diff --git a/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala b/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala new file mode 100644 index 000000000..40f174e21 --- /dev/null +++ b/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 diff --git a/main/src/main/scala/sbt/Main.scala b/main/src/main/scala/sbt/Main.scala index 9cb26bdc0..f41a26cec 100644 --- a/main/src/main/scala/sbt/Main.scala +++ b/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 diff --git a/protocol/src/main/scala/sbt/internal/bsp/BuildServerConnection.scala b/protocol/src/main/scala/sbt/internal/bsp/BuildServerConnection.scala index 340daad06..f45b4e65e 100644 --- a/protocol/src/main/scala/sbt/internal/bsp/BuildServerConnection.scala +++ b/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 sbtLaunchJar = classPath - .split(File.pathSeparator) - .find(jar => SbtLaunchJar.findFirstIn(jar).nonEmpty) - .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 argv = sbtScript match + case Some(script) => + Vector(script, "bsp") + case None => + buildFallbackArgv 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] = diff --git a/protocol/src/test/scala/sbt/internal/bsp/BuildServerConnectionSpec.scala b/protocol/src/test/scala/sbt/internal/bsp/BuildServerConnectionSpec.scala index 2c153c9e1..df86b7216 100644 --- a/protocol/src/test/scala/sbt/internal/bsp/BuildServerConnectionSpec.scala +++ b/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") diff --git a/sbt b/sbt index 9c564bb88..e27a45eff 100755 --- a/sbt +++ b/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 diff --git a/sbtw/src/main/scala/sbtw/Main.scala b/sbtw/src/main/scala/sbtw/Main.scala index fb67b2770..bf68af4fe 100644 --- a/sbtw/src/main/scala/sbtw/Main.scala +++ b/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"