Command argument parsing behavior changed with Mill 0.11.7

[lefou]

This is most likely caused by the mainargs version update from 0.5.4 to 0.6.1

I have the following command, that stopped working as before and expected:

    def smokeTest(
        @mainargs.arg(doc = "Start the App inside a Xephyr server")
        xephyr: mainargs.Flag = mainargs.Flag(),
        @mainargs.arg(doc = "Start the App insie a Xvfb server")
        xvfb: mainargs.Flag = mainargs.Flag(),
        @mainargs.arg(doc = "The display server port to use")
        xport: String = ":90",
        @mainargs.arg(doc = "Arguments for the app launcher (e.g. `-console -consoleLog -noExit`)")
        args: mainargs.Leftover[String],
        @mainargs.arg(doc = "Timeout in seconds to cancel the test as failed")
        timeout: Int = 40
    ) = T.command { ... }

I typically start it as follows:

> mill my.app.smokeTest --xvfb --timeout 60 -consoleLog 

The consoleLog is supposed to be parsed into args parameter, since it is a mainargs.Leftover[String].

Since the version bump to Mill 0.11.7, it fails with the following error message:

> mill my.app.smokeTest --xvfb --timeout 60 -consoleLog 
...
Unknown argument: "-c"
Expected Signature: smokeTest
  --timeout <int>  Timeout in seconds to cancel the test as failed
  --xephyr         Start the App inside a Xephyr server
  --xport <str>    The display server port to use
  --xvfb           Start the App insie a Xvfb server
  args <str>...    Arguments for the app launcher (e.g. `-console -consoleLog -noExit`)

I also tried to force the parser to stop reading options with --, but it didn't work.

> mill my.app.smokeTest --xvfb --timeout 60 -- -consoleLog 
...
Unknown argument: "-c"
Expected Signature: smokeTest
  --timeout <int>  Timeout in seconds to cancel the test as failed
  --xephyr         Start the App inside a Xephyr server
  --xport <str>    The display server port to use
  --xvfb           Start the App insie a Xvfb server
  args <str>...    Arguments for the app launcher (e.g. `-console -consoleLog -noExit`)

[lihaoyi]

I think this is expected due to the syntax for -consoleLog now meaning -c -o -n -s -o -l -e -L -o -g. That is a behavioral change we missed in the original PR to mainargs. Most programs have their multi-character flags take two dashes --, though there are certainly exceptions (including common things like java -classpath)

Maybe we can validate the entire -combine token first in MainArgs, and only use it as short token if everything matches, but otherwise throw the whole token to Leftover? That would probably be strictly better than the status quo of erroring out, which is probably unnecessarily pessimistic.

What I described would still be a behavioral change. e.g. if you have flags -a -b -c, passing a token -abc would still delegate to -a -b -c, where previously it would have been passed to Leftover. I think this is probably an OK tradeoff

We could add a configuration flag to disable the combined-short-options behavior for cases where that is desired, though exposing such a flag to people defining Mill T.commands would take more plumbing

[lefou]

I agree. It should only go in the report error mode, if there is no Leftover that can take it.

[lefou]

Interestingly, I can work around with specifying --args before the args. Although, that means I can use Mill 0.11.7 for now, but I'm a bit surprised this works at all.

> mill my.app.smokeTest --xvfb --timeout 60 --args -consoleLog

This isn't free of issues though, as the resulting args.value is: List(--args, -consoleLog)

[lefou]

Also, I think mainargs shouldn't even try to parse combined short options in cases like this, where I don't have a single short option defined.

[lihaoyi]

The over-eager erroring out is probably here:

https://github.com/com-lihaoyi/mainargs/pull/102/files#diff-43ca7abfb0abd6d68548dfffb2c189d5ff46628f78c39c61913200aff83165f2R103

Where we immediately return the error, rather than going through the normal complete codepath for finishing a parse that has a chance to handle Leftover[T] etc..

Interestingly, I can work around with specifying --args before the args. Although, that means I can use Mill 0.11.7 for now, but I'm a bit surprised this works at all.

This is because we specifically look for a single-dash-followed-by-not-a-dash prefix before we start parsing combined short options https://github.com/com-lihaoyi/mainargs/pull/102/files#diff-43ca7abfb0abd6d68548dfffb2c189d5ff46628f78c39c61913200aff83165f2R101

Also, I think mainargs shouldn't even try to parse combined short options in cases like this, where I don't have a single short option defined.

If we delegate to Leftover whenever parsing combined short options fails, it doesn't really matter whether it tries or not: it'll try parsing the first short option, fail, and then delegate the token and any subsequent tokes to Leftover. A bit of wasted work, not efficiency probably doesn't matter too much here

[lihaoyi]

com-lihaoyi/mainargs#112 should fix this I think

[megri]

@lefou I think I'm hitting this error but I'm really confused as to what version of mill I'm actually running:

I am using asdf to manage versions in my project, and my project structure is as follows:

• root/.tool-versions - here mill is set to 0.11.7
• root/projects/app/.mill-version - sets mill to 0.11.7

Running mill version from the app directory reports version 0.11.7

When running mill app.run from directory root/projects/app I get the error

Unknown argument: "-D"
Expected Signature: run
  args <str>...

Either of these changes lets the app run:

1. Changing mill version in root/.tool-versions to 0.11.6: mill version still reports version 0.11.7;
2. Changing root/projects/app/.mill-version to 0.11.6: mill version now says 0.11.6.

So it seems like mill version simply outputs the version written in .mill-version but mill in fact uses mill defined in root/.tool-versions. Do you think this assumption is correct?

edit: Adding a line mill 0.11.6 to root/projects/app/.tool-versions also lets the app run, despite the file .mill-version being set to 0.11.7. Again, in this scenario mill version says 0.11.7.

[lefou]

I don't know asdf and how it manages the Mill version. If you use one of the Mill wrapper scripts (either lefou/millw or the one in Mill's repo), you will run with what is in .mill-version or .config/mill-version or env MILL_VERSION. Mill will also check if a version file exists and will report if that version differs from the runtime version. You can find out about the runtime version with mill --version or mill version.