Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle jvmOptions without -J prefix in debugSession/start request #1455

Merged
merged 1 commit into from
Feb 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions frontend/src/main/scala/bloop/dap/DebuggeeRunner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,18 @@ private final class MainClassDebugAdapter(

def run(debugLogger: DebugSessionLogger): Task[ExitStatus] = {
val workingDir = state.commonOptions.workingPath
// TODO: https://github.com/scalacenter/bloop/issues/1456
// Metals used to add the `-J` prefix but it is not needed anymore
// So we cautiously strip it off
adpi2 marked this conversation as resolved.
Show resolved Hide resolved
val jvmOptions = mainClass.jvmOptions.map(_.stripPrefix("-J"))
val runState = Tasks.runJVM(
state.copy(logger = debugLogger),
project,
env,
workingDir,
mainClass.`class`,
(mainClass.arguments ++ mainClass.jvmOptions).toArray,
skipJargs = false,
mainClass.arguments.toArray,
jvmOptions.toArray,
mainClass.environmentVariables,
RunMode.Debug
)
Expand Down
34 changes: 32 additions & 2 deletions frontend/src/main/scala/bloop/engine/tasks/Tasks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ object Tasks {
* @param project The project to run.
* @param cwd The directory in which to start the forked JVM.
* @param fqn The fully qualified name of the main class.
* @param args The arguments to pass to the main class.
* @param args The arguments to pass to the main class. If they contain args
* starting with `-J`, they will be interpreted as jvm options.
* @param skipJargs Skip the interpretation of `-J` options in `args`.
* @param mode The run mode.
*/
Expand All @@ -162,12 +163,41 @@ object Tasks {
skipJargs: Boolean,
envVars: List[String],
mode: RunMode
): Task[State] = {
val (userJvmOptions, userArgs) =
if (skipJargs) (Array.empty[String], args)
else args.partition(_.startsWith("-J"))
val jvmOptions = userJvmOptions.map(_.stripPrefix("-J"))
runJVM(state, project, config, cwd, fqn, userArgs, jvmOptions, envVars, mode)
}

/**
* Runs the fully qualified class `className` in `project`.
*
* @param state The current state of Bloop.
* @param project The project to run.
* @param cwd The directory in which to start the forked JVM.
* @param fqn The fully qualified name of the main class.
* @param args The arguments to pass to the main class.
* @param jvmOptions The java options to pass to the jvm.
* @param mode The run mode.
*/
def runJVM(
state: State,
project: Project,
config: JdkConfig,
cwd: AbsolutePath,
fqn: String,
args: Array[String],
jvmOptions: Array[String],
envVars: List[String],
mode: RunMode
): Task[State] = {
val dag = state.build.getDagFor(project)
val classpath = project.fullRuntimeClasspath(dag, state.client)
val forker = JvmProcessForker(config, classpath, mode)
val runTask =
forker.runMain(cwd, fqn, args, skipJargs, envVars, state.logger, state.commonOptions)
forker.runMain(cwd, fqn, args, jvmOptions, envVars, state.logger, state.commonOptions)
runTask.map { exitCode =>
val exitStatus = Forker.exitStatus(exitCode)
state.mergeStatus(exitStatus)
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/main/scala/bloop/engine/tasks/TestTask.scala
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ object TestTask {
discoverTestSuites(state, project, configuredFrameworks, compileAnalysis, testFilter)
val discoveredFrameworks = suites.iterator.filterNot(_._2.isEmpty).map(_._1).toList
val (userJvmOptions, userTestOptions) = rawTestOptions.partition(_.startsWith("-J"))
val jvmOptions = userJvmOptions.map(_.stripPrefix("-J"))
val frameworkArgs = considerFrameworkArgs(discoveredFrameworks, userTestOptions, logger)
val args = project.testOptions.arguments ++ frameworkArgs
logger.debug(s"Running test suites with arguments: $args")
Expand All @@ -93,7 +94,7 @@ object TestTask {
case DiscoveredTestFrameworks.Jvm(frameworks, forker, loader) =>
val opts = state.commonOptions
// FORMAT: OFF
TestInternals.execute(cwd, forker, loader, suites, args, userJvmOptions, handler, logger, opts)
TestInternals.execute(cwd, forker, loader, suites, args, jvmOptions, handler, logger, opts)
// FORMAT: ON
case DiscoveredTestFrameworks.Js(frameworks, closeResources) =>
val cancelled: AtomicBoolean = AtomicBoolean(false)
Expand Down
41 changes: 10 additions & 31 deletions frontend/src/main/scala/bloop/exec/JvmProcessForker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,44 +33,24 @@ trait JvmProcessForker {
/**
* Run the main function in class `className`, passing it `args`
*
* @param cwd The directory in which to start the forked JVM
* @param mainClass The fully qualified name of the class to run
* @param args0 The arguments to pass to the main method. If they contain args
* starting with `-J`, they will be interpreted as jvm options.
* @param skipJargs Skip the interpretation of `-J` options in `args`.
* @param logger Where to log the messages from execution
* @param opts The options to run the program with
* @param extraClasspath Paths to append to the classpath before running
* @param cwd The directory in which to start the forked JVM
* @param mainClass The fully qualified name of the class to run
* @param args The arguments to pass to the main method.
* @param jvmOptions The java options to pass to the jvm.
* @param logger Where to log the messages from execution
* @param opts The options to run the program with
* @param extraClasspath Paths to append to the classpath before running
* @return 0 if the execution exited successfully, a non-zero number otherwise
*
*
*/
final def runMain(
cwd: AbsolutePath,
mainClass: String,
args0: Array[String],
skipJargs: Boolean,
envVars: List[String],
logger: Logger,
opts: CommonOptions,
extraClasspath: Array[AbsolutePath] = Array.empty
): Task[Int] = {
val (userJvmOptions, userArgs) =
if (skipJargs) (Array.empty[String], args0)
else args0.partition(_.startsWith("-J"))

runMain(cwd, mainClass, userArgs, userJvmOptions, envVars, logger, opts, extraClasspath)
}

def runMain(
cwd: AbsolutePath,
mainClass: String,
args: Array[String],
jargs: Array[String],
jvmOptions: Array[String],
envVars: List[String],
logger: Logger,
opts: CommonOptions,
extraClasspath: Array[AbsolutePath]
extraClasspath: Array[AbsolutePath] = Array.empty
): Task[Int]
}

Expand Down Expand Up @@ -117,8 +97,7 @@ final class JvmForker(config: JdkConfig, classpath: Array[AbsolutePath]) extends
opts: CommonOptions,
extraClasspath: Array[AbsolutePath]
): Task[Int] = {

val jvmOptions = jargs.map(_.stripPrefix("-J")) ++ config.javaOptions
val jvmOptions = jargs ++ config.javaOptions
val fullClasspath = classpath ++ extraClasspath
val fullClasspathStr = fullClasspath.map(_.syntax).mkString(File.pathSeparator)
envVars.map(_.split("=")).collect {
Expand Down
17 changes: 13 additions & 4 deletions frontend/src/main/scala/bloop/testing/TestInternals.scala
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ object TestInternals {
* @param classLoader The class loader used for discovering the tests
* @param discovered The test tasks that were discovered, grouped by their `Framework`
* @param args The test arguments to pass to the framework
* @param userJvmOptions The jvm options to run tests with.
* @param jvmOptions The jvm options to run tests with.
* @param testEventHandler Handler that reacts on messages from the testing frameworks
* @param logger Logger receiving test output
* @param opts The options to run the program with
Expand All @@ -118,7 +118,7 @@ object TestInternals {
classLoader: ClassLoader,
discovered: Map[Framework, List[TaskDef]],
args: List[Config.TestArgument],
userJvmOptions: List[String],
jvmOptions: List[String],
testEventHandler: TestSuiteEventHandler,
logger: Logger,
opts: CommonOptions
Expand All @@ -130,7 +130,7 @@ object TestInternals {

val server = new TestServer(logger, testEventHandler, classLoader, discovered, args, opts)
val forkMain = classOf[sbt.ForkMain].getCanonicalName
val arguments = userJvmOptions.toArray ++ Array(server.port.toString)
val arguments = Array(server.port.toString)
val testAgentJars = agentFiles.filter(_.underlying.toString.endsWith(".jar"))
logger.debug("Test agent JARs: " + testAgentJars.mkString(", "))

Expand All @@ -139,7 +139,16 @@ object TestInternals {
val listener = server.listenToTests
val runner = {
val runTask =
forker.runMain(cwd, forkMain, arguments, false, Nil, logger, opts, testAgentJars)
forker.runMain(
cwd,
forkMain,
arguments,
jvmOptions.toArray,
Nil,
logger,
opts,
testAgentJars
)
runTask.map(exitCode => Forker.exitStatus(exitCode).code)
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/test/scala/bloop/ForkerSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ForkerSpec {
cwd,
mainClass,
args,
skipJargs = false,
Array.empty,
envVars = Nil,
logger.asVerbose,
opts,
Expand Down