Metals: Import build fails when using .jvmopts

[bytecodeguru]

Hello 👋

Describe the bug
Metals fails to import build if there are more than one JVM arguments in .jvmopts.

To Reproduce
Create a .jvmopts file in the sbt project root with, for example, the following JVM args:

-Xms2g -Xmx4g

Trying to import build via metals-vscode extension, it fails.

If you remove all but one JVM argument, the import is successful:

-Xms2g

Expected behavior
Import build is successful when there are more than one legal JVM arguments in .jvmopts.

Screenshots

Installation:

• Operating system: macOS
• Editor: Visual Studio Code
• Metals version: v0.3.3 and v0.3.3+161-e6b9f172-SNAPSHOT

Additional context
From .metals/metals.log:

INFO  running 'sbt metalsEnable bloopInstall'
ERROR Invalid initial heap size: -Xms2g -Xmx4g
ERROR Error: Could not create the Java Virtual Machine.
Error: A fatal exception has occurred. Program will exit.
INFO  sbt exit: 1
INFO  time: ran 'sbt bloopInstall' in 25ms
ERROR sbt command failed: /Library/Java/JavaVirtualMachines/jdk1.8.0_202.jdk/Contents/Home/jre/bin/java -Djline.terminal=jline.UnsupportedTerminal -Dsbt.log.noformat=true -Dfile.encoding=UTF-8 -Xms2g -Xmx4g -Xss2m -XX:MetaspaceSize=512m -jar /var/folders/84/ywwck9pn3pqbylnqks7pfbww0000gn/T/metals23080048761981920/sbt-launch.jar metalsEnable bloopInstall
INFO  bloop exit: 1

Search terms
vscode import build sbt jvmopts

[gabro]

Hi @bytecodeguru, thanks for reporting!

This is almost certainly due to the parsing of options we do here:

https://github.com/scalameta/metals/blob/b3a4801e4cebefc22fc20dffa489fc1bdaee3b3c/metals/src/main/scala/scala/meta/internal/metals/JvmOpts.scala#L20

where we assume one option per line.

I'm not very familiar with the .jvmopts format, but I think we can improve the parser to allow multiple options on the same line.

In the meantime I would reccomend to work around this by putting one option per line:

-Xms2g
-Xmx4g

[bytecodeguru]

It works 🎉 thank you @gabro for the explanation.
I supposed it was a legal format for .jvmopts because sbt was not complaining at all.

[olafurpg]

Thanks for reporting! We use the same .jvmopts parser as in IntelliJ Scala so I'm surprised this has not been a problem for them 🤔

Can JVM options have spaces in them? Like -Xconfig="a b".

[olafurpg]

System properties can have spaces

$ scala -Dfoo="a b"
Welcome to Scala 2.12.8 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_192).
Type in expressions for evaluation. Or try :help.

scala> sys.props.get("foo")
res0: Option[String] = Some("a b")

It looks like sbt doesn't handle this correctly

$ cat .jvmopts
-Xss4m
-Xms1G
-Xmx4G
-Dfoo="a b"
$ sbt
Error: Could not find or load main class b

I'm inclined to keep the current behavior and require each argument to be on a separate line like IntelliJ expects.

[gabro]

I agree it's safer to require one argument per line. It's easier to work around than the opposite behavior.

Since IntelliJ already does this, I expect this is the most common scenario already.

Closing the issue, @bytecodeguru feel free to comment if we've missed something.