JavaModule's run command arguments should be a Task

[daddykotex]

Currently, we can't define a def a: Task[String] = ??? and use that to call the run command from the JavaModule. See the minimal repo:

import mill._
import mill.scalalib._

object proj extends JavaModule {

/*

/private/tmp/def/build.sc:10: Target#apply() call cannot use `value args` defined within the T{...} block
    run(args)()
        ^
/private/tmp/def/build.sc:8: `T.sources` definitions must have 0 parameter lists
  def aCommand() = T.command {
      ^
Compilation Failed
*/
  def myArgs: T[String] = T { "arg" }

  def aCommand() = T.command {
    val args = myArgs()
    run(args)()
  }
}

As per a conversation on gitter with @lefou , a straightforward fix for that is to turn def run(args: String*): Command[Unit] into def run(args: Task[Seq[String]]): Command[Unit]

[lefou]

• See also Consolidating `Tasks` and `Task`s as command parameters (RFC) #1857

[lihaoyi]

I tried a prototype in #2452. Thinking about this, it appears there are a few options here:

1. def run(args: Task[String]*): That should work already I think, but it limits the number way arguments can be passed to run since the number of arguments is fixed up front. This is an awkward limitation, and even apart from that would probably result in boilerplate at the callsite to massage the incoming Task[Seq[String]] into the Seq[Task[String]] that we can pass in

2. def run(args: Task[String*]) is not allowed by the Scala type system

3. def run(args: Task[mainargs.Leftover[String]]) would require some patches to the mainargs library, because currently we hard-code support for mainargs.Leftover[T]. We'd need to un-hard-code it somehow to make the Task[Leftover[String]] param signature work, and then maybe provide an implicit constructor so people can pass in run(Seq("foo")) rather than run(mainargs.Leftover(Seq("foo"))) at the callsite

4. Leave def run(args: String*) and just make it forward to an anonymous task def run(args: Task[Seq[String]]) = T.task{...}. This is probably the easiest thing to do to solve the immediate issue without major surgery, but doesn't generalize to other T.commands unless we repeat the boilerplate of T.command/T.task pairing for each one

It feels like either (3) or (4) are the best options here, depending on how much we want to generalize this v.s. how difficult it is to make the upstream mainargs changes necessary to support (3)

[lefou]

Beside what this means for the codebase, I think what we want is a args: Task[Something]. For different reasons.

• Something can be a Seq[T] to support variable count
• Something can be a case class or the like, which is easier to evolve in a binary compatible way

The JavaModule.run is a good example for a Seq-like parameter.

If we look at a more complex use case like JavaModule.ivyDepsTree, we see currently the following signature:

def ivyDepsTree(args: IvyDepsTreeArgs = IvyDepsTreeArgs()): Command[Unit]

with IvyDepsTreeArgs, which is annotated with various mainargs annotation to be parseable.

[lihaoyi]

com-lihaoyi/mainargs#62 has the mainargs necessary to make (3) work. It would be a binary compatibility breakage for Mainargs, but IMO it's probably worth doing sooner or later, as apart from supporting this feature it would also clean up a lot of cruft within the MainArgs codebase, and if we're breaking bin-compat in Mill 0.11.0 then it could be worth doing now

[lihaoyi]

I've got #2452 updated to use the mainargs PR above (publishLocaled), and everything works end to end. Still pretty janky, but I'm guessing it the syntax and use sites can be cleaned up without too much work.

There's also some concern about how to receive the output of the bar.run command, since it doesn't have the T.dest folder of the calling task for it to write its files to. Probably solvable but might need a bit of thought