Merge pull request #3029 from jozanek/fix/2888-caller-jvm-options-win

Let caller-supplied JVM options override config javaOptions

Author: tgodzik

frontend/src/main/scala/bloop/exec/JvmProcessForker.scala

@@ -138,7 +138,9 @@ final class JvmForker(config: JdkConfig, classpath: Array[AbsolutePath]) extends
       opts: CommonOptions,
       extraClasspath: Array[AbsolutePath]
   ): Task[Int] = {
-    val jvmOptions = jargs ++ config.javaOptions
+    // Caller options come last so they take precedence: for single-valued options
+    // (e.g. -Duser.dir or -Xmx) the JVM honours the last occurrence
+    val jvmOptions = config.javaOptions ++ jargs
     val fullClasspath = classpath ++ extraClasspath
     val fullClasspathStr = fullClasspath.map(_.syntax).mkString(File.pathSeparator)
 

frontend/src/test/scala/bloop/ForkerSpec.scala

@@ -58,13 +58,15 @@ class ForkerSpec {
       cwd: AbsolutePath,
       args: Array[String],
       extraClasspath: Array[AbsolutePath] = Array.empty,
-      envs: List[String] = Nil
+      envs: List[String] = Nil,
+      configJavaOptions: Array[String] = Array.empty,
+      jargs: Array[String] = Array.empty
   )(
       op: (Int, List[(String, String)]) => Unit
   ): Unit =
     TestUtil.checkAfterCleanCompilation(runnableProject, dependencies) { state =>
       val project = TestUtil.getProject(TestUtil.RootProject, state)
-      val env = JdkConfig.default
+      val env = JdkConfig.default.copy(javaOptions = configJavaOptions)
       val classpath = project.fullRuntimeClasspath(state.build.getDagFor(project), state.client)
       val config = JvmProcessForker(env, classpath)
       val logger = new RecordingLogger
@@ -76,7 +78,7 @@ class ForkerSpec {
           cwd,
           mainClass,
           args,
-          Array.empty,
+          jargs,
           envVars = envs,
           logger.asVerbose,
           opts,
@@ -223,6 +225,25 @@ class ForkerSpec {
     }
   }
 
+  @Test
+  def callerJvmOptionsOverrideConfigJavaOptions(): Unit = TestUtil.withinWorkspace { tmp =>
+    val injectedDir = tmp.resolve("injected-dir")
+    val wantedDir = tmp.resolve("wanted-dir")
+    Files.createDirectory(injectedDir.underlying)
+    Files.createDirectory(wantedDir.underlying)
+    run(
+      tmp,
+      Array.empty,
+      configJavaOptions = Array(s"-Duser.dir=${injectedDir.syntax}"),
+      jargs = Array(s"-Duser.dir=${wantedDir.syntax}")
+    ) {
+      case (exitCode, messages) =>
+        val expected = "info" -> s"CWD: ${wantedDir.underlying.toRealPath()}"
+        assertEquals(0, exitCode.toLong)
+        assert(messages.contains(expected), s"$messages did not contain $expected")
+    }
+  }
+
   @Test
   def jdwpAgentArgKeepsDapDefaults(): Unit = {
     // The default debug mode (no fixed address, suspend) is what the DAP/Metals path relies on:

frontend/src/test/scala/bloop/dap/DebugProtocolSpec.scala

@@ -5,6 +5,7 @@ import java.nio.file.Files
 
 import ch.epfl.scala.bsp
 
+import bloop.config.Config
 import bloop.logging.RecordingLogger
 import bloop.util.TestProject
 import bloop.util.TestUtil
@@ -90,6 +91,45 @@ object DebugProtocolSpec extends DebugBspBaseSuite {
     }
   }
 
+  test("caller jvm options override config java options") {
+    TestUtil.withinWorkspace { workspace =>
+      val main =
+        """|/main/scala/Main.scala
+           |object Main {
+           |  def main(args: Array[String]): Unit = {
+           |    print(sys.props("world"))
+           |  }
+           |}
+           |""".stripMargin
+
+      val logger = new RecordingLogger(ansiCodesSupported = false)
+      val jvmConfig = Config.JvmConfig(None, List("-Dworld=config"))
+      val project = TestProject(workspace, "p", List(main), jvmConfig = Some(jvmConfig))
+
+      loadBspState(workspace, List(project), logger) { state =>
+        val params = mainClassParams("Main", jvmOptions = List("-J-Dworld=user"))
+        val output = state.withDebugSession(project, params) { client =>
+          for {
+            _ <- client.initialize()
+            _ <- client.launch(noDebug = true)
+            _ <- client.configurationDone()
+            _ <- client.exited
+            _ <- client.terminated
+            output <- client.blockForAllOutput
+          } yield output
+        }
+
+        assertNoDiff(
+          output.linesIterator
+            .filterNot(_.contains("ERROR: JDWP Unable to get JNI 1.2 environment"))
+            .filterNot(_.contains("JDWP exit error AGENT_ERROR_NO_JNI_ENV"))
+            .mkString("\n"),
+          "user"
+        )
+      }
+    }
+  }
+
   // when the session detaches from the JVM, the JDI once again writes to the standard output
   flakyTest("restarted session does not contain JDI output", 3) {
     TestUtil.withinWorkspace { workspace =>