From b1c8c46a5b4ca42d82d66a53d9baa1d0da745c6b Mon Sep 17 00:00:00 2001 From: bitloi Date: Sun, 15 Feb 2026 23:38:50 +0100 Subject: [PATCH] Address PR review: optional braces, JProcess for interactive run, workflow comment - ArgParser: use optional brace / fewer braces style per sbt coding style - Runner.runJvm: use Java ProcessBuilder with inheritIO() for interactive sbt - client-test.yml: clarify why sbtw native image is not smoke-tested --- .github/workflows/client-test.yml | 2 +- sbtw/src/main/scala/sbtw/ArgParser.scala | 13 +++++-------- sbtw/src/main/scala/sbtw/Runner.scala | 17 +++++++++++------ 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/.github/workflows/client-test.yml b/.github/workflows/client-test.yml index 2b553695d..23453c215 100644 --- a/.github/workflows/client-test.yml +++ b/.github/workflows/client-test.yml @@ -96,7 +96,7 @@ jobs: # smoke test native Image (sbtn) ./client/target/bin/sbtn --sbt-script=$(pwd)/launcher-package/src/universal/bin/sbt.bat about ./client/target/bin/sbtn --sbt-script=$(pwd)/launcher-package/src/universal/bin/sbt.bat shutdown - # verify sbtw native image was built (do not run: scopt lazy vals break under Graal native image) + # verify sbtw native image was built (we do not smoke-test it: scopt uses lazy vals that misbehave under Graal native-image; sbtw is tested as JVM runner below) test -f sbtw/target/bin/sbtw.exe || test -f sbtw/target/bin/sbtw # test launcher script echo build using JDK 17 test using JDK 17 and JDK 25 diff --git a/sbtw/src/main/scala/sbtw/ArgParser.scala b/sbtw/src/main/scala/sbtw/ArgParser.scala index abf5ffbc9..65d980946 100644 --- a/sbtw/src/main/scala/sbtw/ArgParser.scala +++ b/sbtw/src/main/scala/sbtw/ArgParser.scala @@ -2,11 +2,11 @@ package sbtw import scopt.OParser -object ArgParser { +object ArgParser: - def parse(args: Array[String]): Option[LauncherOptions] = { + def parse(args: Array[String]): Option[LauncherOptions] = val b = OParser.builder[LauncherOptions] - val parser = { + val parser = import b.* OParser.sequence( programName("sbtw"), @@ -48,10 +48,7 @@ object ArgParser { .optional() .action((x, c) => c.copy(residual = c.residual :+ x)), ) - } - OParser.parse(parser, args, LauncherOptions()).map { opts => + OParser.parse(parser, args, LauncherOptions()).map: opts => val sbtNew = opts.residual.contains("new") || opts.residual.contains("init") opts.copy(sbtNew = sbtNew) - } - } -} +end ArgParser diff --git a/sbtw/src/main/scala/sbtw/Runner.scala b/sbtw/src/main/scala/sbtw/Runner.scala index 3ccffc4e9..4cde4f710 100644 --- a/sbtw/src/main/scala/sbtw/Runner.scala +++ b/sbtw/src/main/scala/sbtw/Runner.scala @@ -1,6 +1,7 @@ package sbtw import java.io.File +import java.lang.{ Process as JProcess, ProcessBuilder as JProcessBuilder } import scala.sys.process.* object Runner { @@ -91,18 +92,22 @@ object Runner { sbtJar: String, bootArgs: Seq[String], verbose: Boolean - ): Int = { + ): Int = val toolOpts = sys.env.get("JAVA_TOOL_OPTIONS").toSeq.flatMap(_.split("\\s+").filter(_.nonEmpty)) val jdkOpts = sys.env.get("JDK_JAVA_OPTIONS").toSeq.flatMap(_.split("\\s+").filter(_.nonEmpty)) val fullJavaOpts = javaOpts ++ sbtOpts ++ toolOpts ++ jdkOpts val cmd = Seq(javaCmd) ++ fullJavaOpts ++ Seq("-cp", sbtJar, "xsbt.boot.Boot") ++ bootArgs - if (verbose) { + if verbose then System.err.println("# Executing command line:") - cmd.foreach(a => System.err.println(if (a.contains(" ")) s""""$a"""" else a)) - } - Process(cmd).! - } + cmd.foreach(a => System.err.println(if a.contains(" ") then s""""$a"""" else a)) + val jpb = new JProcessBuilder(cmd*) + jpb.inheritIO() + val p = jpb.start() + try + p.waitFor() + p.exitValue() + finally if p.isAlive then p.destroy() def shutdownAll(javaCmd: String): Int = { try {