Fix VM argument passing by .sbtopts file and JAVA_TOOL_OPTIONS environmental variable (#7393)

Fixes #7387

* Fix VM argument passing by .sbtopts file and JAVA_TOOL_OPTIONS environmental variable, improve launcher script integration test setup
* Fix sbt process not exiting in launcher test for --sbt-version
* Fix JAVA_TOOL_OPTIONS in launcher for linux/mac

.github/workflows/ci.yml

@@ -163,7 +163,7 @@ jobs:
         echo build using JDK 8 test using JDK 8 and JDK 11
         cd launcher-package
         sbt -Dsbt.build.version=$TEST_SBT_VER rpm:packageBin debian:packageBin
-        sbt -Dsbt.build.version=$TEST_SBT_VER universal:packageBin universal:stage integrationTest/test
+        sbt -Dsbt.build.version=$TEST_SBT_VER integrationTest/test
         cd citest && ./test.sh
         $HOME/bin/jabba install $JDK11 && exec $HOME/bin/jabba which --home $JDK11
         java -Xmx32m -version
@@ -178,7 +178,7 @@ jobs:
         echo build using JDK 8, test using JDK 8, on macOS
         cd launcher-package
         bin/coursier resolve
-        sbt -Dsbt.build.version=$TEST_SBT_VER universal:packageBin universal:stage integrationTest/test
+        sbt -Dsbt.build.version=$TEST_SBT_VER integrationTest/test
         cd citest && ./test.sh
     - name: Build and test (9)
       if: ${{ matrix.jobtype == 9 }}
@@ -190,7 +190,7 @@ jobs:
         echo build using JDK 8, test using JDK 8, on Windows
         cd launcher-package
         bin/coursier.bat resolve
-        sbt -Dsbt.build.version=$TEST_SBT_VER universal:packageBin universal:stage integrationTest/test
+        sbt -Dsbt.build.version=$TEST_SBT_VER integrationTest/test
         cd citest
         ./test.bat
         test3/test3.bat

launcher-package/build.sbt

@@ -379,7 +379,13 @@ lazy val integrationTest = (project in file("integration-test"))
       "com.eed3si9n.expecty" %% "expecty" % "0.11.0" % Test,
       "org.scala-sbt" %% "io" % "1.3.1" % Test
     ),
-    testFrameworks += new TestFramework("minitest.runner.Framework")
+    testFrameworks += new TestFramework("minitest.runner.Framework"),
+    test in Test := {
+      (test in Test).dependsOn(((packageBin in Universal) in LocalRootProject).dependsOn(((stage in (Universal) in LocalRootProject)))).value
+    },
+    testOnly in Test := {
+      (testOnly in Test).dependsOn(((packageBin in Universal) in LocalRootProject).dependsOn(((stage in (Universal) in LocalRootProject)))).evaluated
+    }
   )
 
 def downloadUrlForVersion(v: String) = (v split "[^\\d]" flatMap (i => catching(classOf[Exception]) opt (i.toInt))) match {

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

@@ -1,7 +1,11 @@
 package example.test
 
 import minitest._
+import sbt.io.IO
+
 import java.io.File
+import java.io.PrintWriter
+import java.nio.file.Files
 
 object SbtScriptTest extends SimpleTestSuite with PowerAssertions {
   lazy val isWindows: Boolean =
@@ -16,28 +20,40 @@ object SbtScriptTest extends SimpleTestSuite with PowerAssertions {
       name: String,
       javaOpts: String = "",
       sbtOpts: String = "",
+      sbtOptsFileContents: String = "",
+      javaToolOptions: String = ""
   )(args: String*)(f: List[String] => Any) = {
     test(name) {
-      val out =
-        sbtProcessWithOpts(args: _*)(javaOpts = javaOpts, sbtOpts = sbtOpts).!!.linesIterator.toList
-      f(out)
-      ()
-    }
-  }
+      val workingDirectory = Files.createTempDirectory("sbt-launcher-package-test").toFile
+      IO.copyDirectory(new File("citest"), workingDirectory)
 
-  def sbtProcess(args: String*) = sbtProcessWithOpts(args: _*)("", "")
-  def sbtProcessWithOpts(args: String*)(javaOpts: String, sbtOpts: String) = {
-    val path = sys.env("PATH")
-    sbt.internal.Process(
-      Seq(sbtScript.getAbsolutePath) ++ args,
-      new File("citest"),
-      "JAVA_OPTS" -> javaOpts,
-      "SBT_OPTS" -> sbtOpts,
-      if (isWindows)
-        "JAVACMD" -> new File(javaBinDir, "java.cmd").getAbsolutePath()
-      else
-        "PATH" -> (javaBinDir + File.pathSeparator + path)
-    )
+      try {
+        val sbtOptsFile = new File(workingDirectory, ".sbtopts")
+        sbtOptsFile.createNewFile()
+        val writer = new PrintWriter(sbtOptsFile)
+        try {
+          writer.write(sbtOptsFileContents)
+        } finally {
+          writer.close()
+        }
+        val path = sys.env.getOrElse("PATH", sys.env("Path"))
+        val out = sbt.internal.Process(
+          Seq(sbtScript.getAbsolutePath) ++ args,
+          workingDirectory,
+          "JAVA_OPTS" -> javaOpts,
+          "SBT_OPTS" -> sbtOpts,
+          "JAVA_TOOL_OPTIONS" -> javaToolOptions,
+          if (isWindows)
+            "JAVACMD" -> new File(javaBinDir, "java").getAbsolutePath()
+          else
+            "PATH" -> (javaBinDir + File.pathSeparator + path)
+        ).!!.linesIterator.toList
+        f(out)
+        ()
+      } finally {
+        IO.delete(workingDirectory)
+      }
+    }
   }
 
   makeTest("sbt -no-colors")("compile", "-no-colors", "-v") { out: List[String] =>
@@ -161,11 +177,9 @@ object SbtScriptTest extends SimpleTestSuite with PowerAssertions {
     assert(!out.contains[String]("-XX:+UseG1GC=-XX:+PrintGC"))
   }
 
-  test("sbt with -debug in SBT_OPTS appears in sbt commands") {
+  makeTest("sbt with -debug in SBT_OPTS appears in sbt commands", javaOpts = "", sbtOpts = "-debug")("compile", "-v") {out: List[String] =>
     if (isWindows) cancel("Test not supported on windows")
 
-    val out: List[String] =
-      sbtProcessWithOpts("compile", "-v")(javaOpts = "", sbtOpts = "-debug").!!.linesIterator.toList
     // Debug argument must appear in the 'commands' section (after the sbt-launch.jar argument) to work
     val sbtLaunchMatcher = """^.+sbt-launch.jar["]{0,1}$""".r
     val locationOfSbtLaunchJarArg = out.zipWithIndex.collectFirst {
@@ -196,14 +210,45 @@ object SbtScriptTest extends SimpleTestSuite with PowerAssertions {
     assert(out.contains[String]("-Dsbt.ivy.home=/ivy/dir"))
   }
 
-  test("sbt --script-version should print sbtVersion") {
-    val out = sbtProcess("--script-version").!!.trim
+  makeTest("sbt --script-version should print sbtVersion")("--script-version") { out: List[String] =>
     val expectedVersion = "^" + SbtRunnerTest.versionRegEx + "$"
-    assert(out.matches(expectedVersion))
+    assert(out.mkString(System.lineSeparator()).trim.matches(expectedVersion))
     ()
   }
 
   makeTest("--sbt-cache")("--sbt-cache", "./cachePath") { out: List[String] =>
     assert(out.contains[String](s"-Dsbt.global.localcache=./cachePath"))
   }
+
+  makeTest(
+    "sbt use .sbtopts file for memory options", sbtOptsFileContents =
+      """-J-XX:MaxInlineLevel=20
+        |-J-Xmx222m
+        |-J-Xms111m
+        |-J-Xss12m""".stripMargin
+
+  )("compile", "-v") { out: List[String] =>
+    assert(out.contains[String]("-XX:MaxInlineLevel=20"))
+    assert(out.contains[String]("-Xmx222m"))
+    assert(out.contains[String]("-Xms111m"))
+    assert(out.contains[String]("-Xss12m"))
+  }
+
+  makeTest(
+    "sbt use JAVA_OPTS for memory options", javaOpts = "-XX:MaxInlineLevel=20 -Xmx222m -Xms111m -Xss12m"
+  )("compile", "-v") { out: List[String] =>
+    assert(out.contains[String]("-XX:MaxInlineLevel=20"))
+    assert(out.contains[String]("-Xmx222m"))
+    assert(out.contains[String]("-Xms111m"))
+    assert(out.contains[String]("-Xss12m"))
+  }
+
+  makeTest(
+    "sbt use JAVA_TOOL_OPTIONS for memory options", javaToolOptions = "-XX:MaxInlineLevel=20 -Xmx222m -Xms111m -Xss12m"
+  )("compile", "-v") { out: List[String] =>
+    assert(out.contains[String]("-XX:MaxInlineLevel=20"))
+    assert(out.contains[String]("-Xmx222m"))
+    assert(out.contains[String]("-Xms111m"))
+    assert(out.contains[String]("-Xss12m"))
+  }
 }

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

@@ -682,6 +682,7 @@ if defined sbt_args_verbose (
   echo "!_JAVACMD!"
   if defined _JAVA_OPTS ( call :echolist !_JAVA_OPTS! )
   if defined _SBT_OPTS ( call :echolist !_SBT_OPTS! )
+  if defined JAVA_TOOL_OPTIONS ( call :echolist %JAVA_TOOL_OPTIONS% )
   echo -cp
   echo "!sbt_jar!"
   echo xsbt.boot.Boot
@@ -689,7 +690,7 @@ if defined sbt_args_verbose (
   echo.
 )
 
-"!_JAVACMD!" !_JAVA_OPTS! !_SBT_OPTS! -cp "!sbt_jar!" xsbt.boot.Boot %*
+"!_JAVACMD!" !_JAVA_OPTS! !_SBT_OPTS! %JAVA_TOOL_OPTIONS% -cp "!sbt_jar!" xsbt.boot.Boot %*
 
 goto :eof
 
@@ -828,21 +829,21 @@ exit /B 0
 
   set _has_memory_args=
 
-  if defined _JAVA_OPTS for /F %%g in ("!_JAVA_OPTS!") do (
+  if defined _JAVA_OPTS for %%g in (%_JAVA_OPTS%) do (
     set "p=%%g"
     if "!p:~0,4!" == "-Xmx" set _has_memory_args=1
     if "!p:~0,4!" == "-Xms" set _has_memory_args=1
     if "!p:~0,4!" == "-Xss" set _has_memory_args=1
   )
 
-  if defined JAVA_TOOL_OPTIONS for /F %%g in ("%JAVA_TOOL_OPTIONS%") do (
+  if defined JAVA_TOOL_OPTIONS for %%g in (%JAVA_TOOL_OPTIONS%) do (
     set "p=%%g"
     if "!p:~0,4!" == "-Xmx" set _has_memory_args=1
     if "!p:~0,4!" == "-Xms" set _has_memory_args=1
     if "!p:~0,4!" == "-Xss" set _has_memory_args=1
   )
 
-  if defined _SBT_OPTS for /F %%g in ("!_SBT_OPTS!") do (
+  if defined _SBT_OPTS for %%g in (%_SBT_OPTS%) do (
     set "p=%%g"
     if "!p:~0,4!" == "-Xmx" set _has_memory_args=1
     if "!p:~0,4!" == "-Xms" set _has_memory_args=1

sbt

@@ -515,6 +515,7 @@ run() {
     execRunner "$java_cmd" \
       "${java_args[@]}" \
       "${sbt_options[@]}" \
+      "${java_tool_options[@]}" \
       -jar "$sbt_jar" \
       "${sbt_commands[@]}" \
       "${residual_args[@]}"
@@ -785,6 +786,7 @@ original_args=("$@")
 
 java_args=($JAVA_OPTS)
 sbt_options0=(${SBT_OPTS:-$default_sbt_opts})
+java_tool_options=($JAVA_TOOL_OPTIONS)
 if [[ "$SBT_NATIVE_CLIENT" == "true" ]]; then
   use_sbtn=1
 fi