From 2cded9f8db6c65c5e6658ab549405598e39e044c Mon Sep 17 00:00:00 2001 From: Brian Hotopp Date: Mon, 13 Jul 2026 07:06:26 -0400 Subject: [PATCH] [2.x] fix: Propagate -java-home to a server the thin client starts The thin client (sbtn) parsed the launcher value flags (-java-home, -mem, -jvm-debug, -sbt-dir, ...) but then dropped them. The space form was consumed and discarded; the flag=value form fell through to the residual arguments and was forwarded to the server verbatim, where --java-home=/path was rejected as a command (`Not a valid command: --`). When the client had to start a server (none running, no --server), the consumed -java-home never reached the forked sbt launcher, so the server came up under the default JVM and, in CI where the intended JDK is only reachable via -java-home, failed to connect. parseArgs now captures the consumed launcher value flags (both `flag value` and `flag=value`) into Arguments.launcherValueArgs, and the cold-start fork re-passes them to the sbt launcher so the server runs under the requested JVM. The client tokenizes arguments by splitting on whitespace, which would otherwise fragment a value that contains spaces (a Windows path like C:\Program Files\Java); parseArgs tracks those split boundaries and rejoins a value flag's value. An empty flag= value and a dangling flag with no value are consumed but not propagated, since the launcher's require_arg would otherwise fail the fork. The fork command construction is extracted into a pure, package-visible serverCommand so a test can assert the propagated flag reaches the started server. The sbt-launch-jar path is unchanged: it invokes java directly, with no launcher to interpret the flag. Fixes #9418 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../sbt/internal/client/NetworkClient.scala | 97 +++++++++++++------ .../client/NetworkClientParseArgsTest.scala | 65 +++++++++++++ notes/2.0.0/thin-client-java-home.md | 11 +++ 3 files changed, 143 insertions(+), 30 deletions(-) create mode 100644 notes/2.0.0/thin-client-java-home.md 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 1e3cfb5a1..eade24901 100644 --- a/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala +++ b/main-command/src/main/scala/sbt/internal/client/NetworkClient.scala @@ -376,33 +376,19 @@ class NetworkClient( term.isSupershellEnabled ).mkString(",") - val cmd = arguments.sbtLaunchJar match { - case Some(lj) => - if (log) { - val sbtScript = if (Properties.isWin) "sbt.bat" else "sbt" - console.appendLog(Level.Warn, s"server is started using sbt-launch jar directly") - console.appendLog( - Level.Warn, - "this is not the recommended way: .sbtopts and .jvmopts files are not loaded and SBT_OPTS is ignored" - ) - console.appendLog( - Level.Warn, - s"either upgrade $sbtScript to its latest version or make sure it is accessible from $$PATH, and run 'sbt bspConfig'" - ) - } - val java = Option(Properties.javaHome) - .map { javaHome => - s"$javaHome/bin/java" - } - .getOrElse("java") - List(java) ++ arguments.sbtArguments.filterNot( - NetworkClient.emptyBuildFlags.contains - ) ++ - List("-jar", lj, DashDashDetachStdio, DashDashServer) - case _ => - List(arguments.sbtScript) ++ arguments.sbtArguments ++ - List(DashDashDetachStdio, DashDashServer) + if (log && arguments.sbtLaunchJar.isDefined) { + val sbtScript = if (Properties.isWin) "sbt.bat" else "sbt" + console.appendLog(Level.Warn, s"server is started using sbt-launch jar directly") + console.appendLog( + Level.Warn, + "this is not the recommended way: .sbtopts and .jvmopts files are not loaded and SBT_OPTS is ignored" + ) + console.appendLog( + Level.Warn, + s"either upgrade $sbtScript to its latest version or make sure it is accessible from $$PATH, and run 'sbt bspConfig'" + ) } + val cmd = NetworkClient.serverCommand(arguments) // https://github.com/sbt/sbt/issues/6271 val nohup = @@ -1204,6 +1190,7 @@ object NetworkClient { val sbtScript: String, val bsp: Boolean, val sbtLaunchJar: Option[String], + val launcherValueArgs: Seq[String] = Nil, ) { def withBaseDirectory(file: File): Arguments = new Arguments( @@ -1214,8 +1201,20 @@ object NetworkClient { sbtScript, bsp, sbtLaunchJar, + launcherValueArgs, ) } + private[client] def serverCommand(arguments: Arguments): List[String] = + arguments.sbtLaunchJar match { + case Some(lj) => + val java = + Option(Properties.javaHome).map(javaHome => s"$javaHome/bin/java").getOrElse("java") + List(java) ++ arguments.sbtArguments.filterNot(emptyBuildFlags.contains) ++ + List("-jar", lj, DashDashDetachStdio, DashDashServer) + case _ => + List(arguments.sbtScript) ++ arguments.launcherValueArgs ++ arguments.sbtArguments ++ + List(DashDashDetachStdio, DashDashServer) + } private[client] val completions = "--completions" private[client] val noTab = "--no-tab" private[client] val noStdErr = "--no-stderr" @@ -1293,6 +1292,8 @@ object NetworkClient { "--autostart=", "-autostart=", ) + private[client] val launcherValueEqPrefixes: Seq[String] = + launcherValueFlags.toSeq.map(_ + "=") private[client] def parseArgs(args: Array[String]): Arguments = { val defaultSbtScript = if (Properties.isWin) "sbt.bat" else "sbt" var sbtScript = Properties.propOrNone("sbt.script") @@ -1301,10 +1302,32 @@ object NetworkClient { val commandArgs = new mutable.ArrayBuffer[String] val sbtArguments = new mutable.ArrayBuffer[String] val completionArguments = new mutable.ArrayBuffer[String] + val launcherValueArgs = new mutable.ArrayBuffer[String] val SysProp = "-D([^=]+)=(.*)".r - val sanitized = args.flatMap { - case a if a.startsWith("\"") => Array(a) - case a => a.split(" ") + val sanitized = new mutable.ArrayBuffer[String] + val splitFromPrev = new mutable.ArrayBuffer[Boolean] + args.foreach { + case a if a.startsWith("\"") => + sanitized += a + splitFromPrev += false + case a => + var first = true + a.split(" ").foreach { part => + if (part.nonEmpty) { + sanitized += part + splitFromPrev += !first + first = false + } + } + } + def valueFrom(start: Int): (String, Int) = { + var last = start + val sb = new StringBuilder(sanitized(start)) + while (last + 1 < sanitized.length && splitFromPrev(last + 1)) { + last += 1 + sb.append(" ").append(sanitized(last)) + } + (sb.toString, last) } var i = 0 while (i < sanitized.length) { @@ -1339,7 +1362,20 @@ object NetworkClient { 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 + if (i + 1 < sanitized.length) { + launcherValueArgs += a + val (value, last) = valueFrom(i + 1) + launcherValueArgs += value + i = last + } + case a if launcherValueEqPrefixes.exists(p => a.startsWith(p)) => + val (full, last) = valueFrom(i) + i = last + val eq = full.indexOf('=') + if (eq < full.length - 1) { + launcherValueArgs += full.substring(0, eq) + launcherValueArgs += full.substring(eq + 1) + } case a if launcherNoValueFlags.contains(a) => () case a if launcherEqPrefixes.exists(p => a.startsWith(p)) => () case a if a.startsWith("-J") => () @@ -1366,6 +1402,7 @@ object NetworkClient { sbtScript.getOrElse(defaultSbtScript).replace("%20", " "), bsp, launchJar, + launcherValueArgs.toSeq, ) } diff --git a/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala b/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala index 4cb18c8b8..c6f1448b0 100644 --- a/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala +++ b/main-command/src/test/scala/sbt/internal/client/NetworkClientParseArgsTest.scala @@ -184,6 +184,7 @@ object NetworkClientParseArgsTest extends BasicTestSuite: assert(!result.sbtArguments.contains("-mem")) assert(!result.sbtArguments.contains("10000")) assert(result.sbtArguments.exists(_.contains("-Dfoo=bar"))) + assert(result.launcherValueArgs == Seq("-mem", "10000")) assert(result.commandArguments.contains("compile")) assert(result.commandArguments.contains("test")) @@ -196,7 +197,71 @@ object NetworkClientParseArgsTest extends BasicTestSuite: assert(!result.sbtArguments.contains("/jdk")) assert(!result.sbtArguments.exists(_.contains("color=never"))) assert(result.sbtArguments.exists(_.contains("-Dfoo=bar"))) + assert(result.launcherValueArgs == Seq("-java-home", "/jdk")) assert(result.commandArguments.contains("compile")) assert(result.commandArguments.size == 1) + // -- Launcher value flags are captured for a forked server (#9418) -- + + test("-java-home /path is captured in launcherValueArgs for propagation"): + val result = parse("-java-home", "/path/to/jdk", "compile") + assert(result.launcherValueArgs == Seq("-java-home", "/path/to/jdk")) + + test("--java-home=/path is consumed, not forwarded, and captured as flag + value"): + val result = parse("--java-home=/path/to/jdk", "compile") + assert(!result.sbtArguments.exists(_.contains("java-home"))) + assert(!result.commandArguments.exists(_.contains("java-home"))) + assert(result.launcherValueArgs == Seq("--java-home", "/path/to/jdk")) + assert(result.commandArguments.contains("compile")) + + test("--java-home=/path does not leak a bare -- into forwarded args"): + val result = parse("--java-home=/usr/lib/jvm/java-17", "scalafmtCheckAll") + assert(!result.sbtArguments.exists(_.startsWith("--java-home"))) + assert(result.commandArguments == Seq("scalafmtCheckAll")) + + test("-mem=10000 eq-form is consumed and captured as flag + value"): + val result = parse("-mem=10000", "compile") + assert(!result.sbtArguments.exists(_.contains("mem"))) + assert(result.launcherValueArgs == Seq("-mem", "10000")) + assert(result.commandArguments.contains("compile")) + + test("--java-home= with an empty value is consumed but not propagated"): + val result = parse("--java-home=", "compile") + assert(!result.sbtArguments.exists(_.contains("java-home"))) + assert(result.launcherValueArgs.isEmpty) + assert(result.commandArguments == Seq("compile")) + + test("-java-home with a spaced path keeps the path intact"): + val result = parse("-java-home", "C:\\Program Files\\Java\\jdk-17", "compile") + assert(result.launcherValueArgs == Seq("-java-home", "C:\\Program Files\\Java\\jdk-17")) + assert(result.commandArguments == Seq("compile")) + + test("--java-home= with a spaced path keeps the path intact"): + val result = parse("--java-home=C:\\Program Files\\Java\\jdk-17", "compile") + assert(result.launcherValueArgs == Seq("--java-home", "C:\\Program Files\\Java\\jdk-17")) + assert(result.commandArguments == Seq("compile")) + + test("a single arg joining a flag and its value is still split and captured"): + val result = parse("-mem 10000", "compile") + assert(result.launcherValueArgs == Seq("-mem", "10000")) + assert(result.commandArguments == Seq("compile")) + + test("serverCommand propagates -java-home to the forked server before --server"): + val args = parse("-java-home", "/opt/jdk17", "compile") + val cmd = NetworkClient.serverCommand(args) + val jh = cmd.indexOf("-java-home") + assert(jh >= 0, cmd.toString) + assert(cmd(jh + 1) == "/opt/jdk17", cmd.toString) + assert(jh < cmd.indexOf("--server"), cmd.toString) + + test("a value flag with no value is dropped, not propagated as a dangling flag"): + val result = parse("-java-home") + assert(result.launcherValueArgs.isEmpty) + assert(result.commandArguments.isEmpty) + + test("extra whitespace in a value does not leave a stray leading space"): + val result = parse("-mem", " 10000", "compile") + assert(result.launcherValueArgs == Seq("-mem", "10000")) + assert(result.commandArguments == Seq("compile")) + end NetworkClientParseArgsTest diff --git a/notes/2.0.0/thin-client-java-home.md b/notes/2.0.0/thin-client-java-home.md new file mode 100644 index 000000000..c0d8f6edb --- /dev/null +++ b/notes/2.0.0/thin-client-java-home.md @@ -0,0 +1,11 @@ +### The thin client honors `-java-home` when it starts a server + +Launcher value flags such as `-java-home` were parsed by the thin client but then +dropped: the `=` form (`--java-home=/path`) was forwarded to the server verbatim +and rejected as a command (`Not a valid command: --`), and the space form +(`-java-home /path`) was silently discarded when the client had to start a server, +so that server came up under the default JVM. Both forms are now consumed by the +client and re-passed to a server it starts, so the server runs under the requested +JVM, including values that contain spaces such as a Windows path +(`C:\Program Files\Java\...`). This also applies to the other launcher value flags +(`-mem`, `-jvm-debug`, `-sbt-dir`, ...), which were dropped the same way.