mirror of https://github.com/sbt/sbt.git
[2.x] fix: Propagate -java-home to a server the thin client starts (#9448)
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) <noreply@anthropic.com>
This commit is contained in:
BrianHotopp
GitHub
parent
5ff0c638d4
commit
3de18d446f
|
|
@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
Loading…
Reference in New Issue