When .mill-jvm-opts doesn't end with a newline, the last parameter is ignored

[phfroidmont]

This is a follow up from scalameta/metals#4557.

It is easily verified as removing the last line from .mill-jvm-opts in test resources, makes the test fail.
On the other hand, I don't see any obvious issue in the parser code.

[lefou]

I added a test case which only shows that parsing that file works.

• Test opts file reader to cover issue #2140 #2145

Can you confirm, that it properly works for you, when you add a final newline to that file?

[phfroidmont]

I confirm it works in your test with or without the newline. Actually I did something similar when trying to fix the issue myself.
Where it doesn't work, is when I run the integration tests. If I remove the last line in .mill-jvm-opts and run ./mill integration.forked.test, the relevant test fails.
That is very odd to me, there has to be some magic happening that I don't know about.

[lefou]

If I remove the last line in .mill-jvm-opts and run ./mill integration.forked.test, the relevant test fails.

I'm reading "remove the last line" but I think you meant the "remove the last newline", don't you?

Can you override the compile target and print out the system properties? Then, place a sys property on the last line with and without a final newline and see if it is included or not.

[phfroidmont]

Yes, I meant the last newline.
I still have the issue but I found a precision on the context.

In an empty directory, I created 2 files:

.mill-jvm-opts (No newline at the end, hexdump is exactly as in #2145)

# comment after an empty line
-DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file
-Xss120m

build.sc

import mill._
import java.lang.management.ManagementFactory
import scala.jdk.CollectionConverters._


def checkJvmOpts() = T.command {
  val runtime = ManagementFactory.getRuntimeMXBean()
  val args = runtime.getInputArguments().asScala.toSet
  println("###### Properties ####")
  println(System.getProperties.keySet().asScala.toSet.mkString("\n"))
  println("###### JVM args ######")
  println(args.mkString("\n"))
  ()
}

And then, I ran:

$ mill version
[1/1] version
0.10.8

$ mill -i checkJvmOpts
[1/1] checkJvmOpts
###### Properties ####
<...>
PROPERTY_PROPERLY_SET_VIA_JVM_OPTS
<...>
###### JVM args ######
-Dmill.jvm_opts_applied=true
-DMILL_VERSION=0.10.8
-DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file
-Djna.nosys=true
-DMILL_CLASSPATH=/nix/store/cajzcgzv8n2bikadg90kfnplqglp12yq-mill-0.10.8/bin/.mill-wrapped

$ mill checkJvmOpts
[1/1] checkJvmOpts
###### Properties ####
<...>
PROPERTY_PROPERLY_SET_VIA_JVM_OPTS
<...>
###### JVM args ######
-DMILL_VERSION=0.10.8
-DPROPERTY_PROPERLY_SET_VIA_JVM_OPTS=value-from-file
-Xss120m

So the problem only occurs when using the -i flag.

[lefou]

That's it! There is a different place where we read that file:

mill/build.sc

Line 1164 in f03c3c3

| while IFS= read line

This is probably the culprit. Nice detective work!

[lefou]

@phfroidmont I implemented a fix in #2162. But I didn't include a test case, because of effort/complexity to refactor this script for test-ability. Do you mind to test it for your case?

[phfroidmont]

I confirm it works.
Thanks for all your work!