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

Make Mill commands require named CLI parameters by default #3431

Merged
merged 9 commits into from
Aug 29, 2024
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
58 changes: 55 additions & 3 deletions example/depth/tasks/2-primary-tasks/build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ def lineCount: T[Int] = T {
// on other targets are defined using `foo()` to extract the value from them.
// Apart from the `foo()` calls, the `T {...}` block contains arbitrary code that
// does some work and returns a result.
//
// The `os.walk` and `os.read.lines` statements above are from the
// https://github.com/com-lihaoyi/os-lib[OS-Lib] library, which provides all common
// filesystem and subprocess operations for Mill builds. You can see the OS-Lib library
// documentation for more details:
//
// * https://github.com/com-lihaoyi/os-lib[OS-Lib Library Documentation]

// If a target's inputs change but its output does not, e.g. someone changes a
// comment within the source files that doesn't affect the classfiles, then
Expand Down Expand Up @@ -102,7 +109,9 @@ Computing line count
// The return-value of targets has to be JSON-serializable via
// {upickle-github-url}[uPickle]. You can run targets directly from the command
// line, or use `show` if you want to see the JSON content or pipe it to
// external tools.
// external tools. See the uPickle library documentation for more details:
//
// * {upickle-github-url}[uPickle Library Documentation]

// ==== T.dest
//
Expand Down Expand Up @@ -257,11 +266,11 @@ def summarizeClassFileStats = T{

// === Commands

def run(args: String*) = T.command {
def run(mainClass: String, args: String*) = T.command {
os.proc(
"java",
"-cp", s"${classFiles().path}:${resources().path}",
"foo.Foo",
mainClass,
args
)
.call(stdout = os.Inherit)
Expand All @@ -285,6 +294,49 @@ def run(args: String*) = T.command {
// re-evaluate every time even if none of their inputs have changed.
// A command with no parameter is defined as `def myCommand() = T.command {...}`.
// It is a compile error if `()` is missing.
//
// Targets can take command line params, parsed by the https://github.com/com-lihaoyi/mainargs[MainArgs]
// library. Thus the signature `def run(mainClass: String, args: String*)` takes
// params of the form `--main-class <str> <arg1> <arg2> ... <argn>`:

/** Usage

> ./mill run --main-class foo.Foo hello world
Foo.value: 31337
args: hello world
foo.txt resource: My Example Text

*/

// Command line arguments can take most primitive types: `String`, `Int`, `Boolean`, etc.,
// along with `Option[T]` representing optional values and `Seq[T]` representing repeatable values,
// and `mainargs.Flag` representing flags and `mainargs.Leftover[T]` representing any command line
// arguments not parsed earlier. Default values for command line arguments are also supported.
// See the mainargs documentation for more details:
//
// * [MainArgs Library Documentation](https://github.com/com-lihaoyi/mainargs[MainArgs])
//
// By default, all command parameters need to be named, except for variadic parameters
// of type `T*` or `mainargs.Leftover[T]`. You can use the flag `--allow-positional-command-args`
// to allow arbitrary arguments to be passed positionally, as shown below:

/** Usage

> ./mill run foo.Foo hello world # this raises an error because `--main-class` is not given
error: Missing argument: --mainClass <str>
Expected Signature: run
--mainClass <str>
args <str>...
...

> ./mill --allow-positional-command-args run foo.Foo hello world # this succeeds due to --allow-positional-command-args
Foo.value: 31337
args: hello world
foo.txt resource: My Example Text

*/


//
// Like <<_targets>>, a command only evaluates after all its upstream
// dependencies have completed, and will not begin to run if any upstream
Expand Down
4 changes: 2 additions & 2 deletions example/depth/tasks/3-anonymous-tasks/build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ def printFileData(fileName: String) = T.command {
> ./mill show helloFileData
"Hello"

> ./mill printFileData hello.txt
> ./mill printFileData --file-name hello.txt
Hello

> ./mill printFileData world.txt
> ./mill printFileData --file-name world.txt
World!

*/
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ object ModuleInitErrorTests extends IntegrationTestSuite {
test("rootCommand") {
// If we specify a target in the root module, we are not
// affected by the sub-modules failing to initialize
val res = eval(("rootCommand", "hello"))
val res = eval(("rootCommand", "-s", "hello"))
assert(res.isSuccess == true)
assert(res.out.contains("""Running rootCommand hello"""))
}
Expand All @@ -66,7 +66,7 @@ object ModuleInitErrorTests extends IntegrationTestSuite {
assert(fansi.Str(res.err).plainText.linesIterator.size < 20)
}
test("fooCommand") {
val res = eval(("foo.fooCommand", "hello"))
val res = eval(("foo.fooCommand", "-s", "hello"))
assert(res.isSuccess == false)
assert(fansi.Str(res.err).plainText.contains("""java.lang.Exception: Foo Boom"""))
assert(fansi.Str(res.err).plainText.linesIterator.size < 20)
Expand All @@ -77,7 +77,7 @@ object ModuleInitErrorTests extends IntegrationTestSuite {
assert(res.out.contains("""Running barTarget"""))
}
test("barCommand") {
val res = eval(("bar.barCommand", "hello"))
val res = eval(("bar.barCommand", "-s", "hello"))
assert(res.isSuccess == true)
assert(res.out.contains("""Running barCommand hello"""))
}
Expand All @@ -88,7 +88,7 @@ object ModuleInitErrorTests extends IntegrationTestSuite {
assert(fansi.Str(res.err).plainText.linesIterator.size < 20)
}
test("quxCommand") {
val res = eval(("bar.qux.quxCommand", "hello"))
val res = eval(("bar.qux.quxCommand", "-s", "hello"))
assert(res.isSuccess == false)
assert(fansi.Str(res.err).plainText.contains("""java.lang.Exception: Qux Boom"""))
assert(fansi.Str(res.err).plainText.linesIterator.size < 20)
Expand Down
2 changes: 1 addition & 1 deletion main/eval/src/mill/eval/Evaluator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ trait Evaluator {

def withBaseLogger(newBaseLogger: ColorLogger): Evaluator
def withFailFast(newFailFast: Boolean): Evaluator

def allowPositionalCommandArgs: Boolean = false
def plan(goals: Agg[Task[_]]): (MultiBiMap[Terminal, Task[_]], Agg[Task[_]])

/**
Expand Down
3 changes: 2 additions & 1 deletion main/eval/src/mill/eval/EvaluatorImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ private[mill] case class EvaluatorImpl(
threadCount: Option[Int] = Some(1),
scriptImportGraph: Map[os.Path, (Int, Seq[os.Path])] = Map.empty,
methodCodeHashSignatures: Map[String, Int],
override val disableCallgraphInvalidation: Boolean
override val disableCallgraphInvalidation: Boolean,
override val allowPositionalCommandArgs: Boolean
) extends Evaluator with EvaluatorCore {
import EvaluatorImpl._

Expand Down
75 changes: 60 additions & 15 deletions main/resolve/src/mill/resolve/Resolve.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ object Resolve {
resolved: Seq[Resolved],
args: Seq[String],
selector: Segments,
nullCommandDefaults: Boolean
nullCommandDefaults: Boolean,
allowPositionalCommandArgs: Boolean
) = {
Right(resolved.map(_.segments))
}
Expand All @@ -37,7 +38,8 @@ object Resolve {
resolved: Seq[Resolved],
args: Seq[String],
selector: Segments,
nullCommandDefaults: Boolean
nullCommandDefaults: Boolean,
allowPositionalCommandArgs: Boolean
) = {
val taskList = resolved.map {
case r: Resolved.Target =>
Expand All @@ -51,7 +53,14 @@ object Resolve {
val instantiated = ResolveCore
.instantiateModule0(baseModules, r.segments.init)
.flatMap { case (mod, rootMod) =>
instantiateCommand(rootMod, r, mod, args, nullCommandDefaults)
instantiateCommand(
rootMod,
r,
mod,
args,
nullCommandDefaults,
allowPositionalCommandArgs
)
}

instantiated.map(Some(_))
Expand All @@ -75,7 +84,8 @@ object Resolve {
r,
value,
args,
nullCommandDefaults
nullCommandDefaults,
allowPositionalCommandArgs
).map(Some(_))
}
)
Expand Down Expand Up @@ -110,15 +120,17 @@ object Resolve {
r: Resolved.Command,
p: Module,
args: Seq[String],
nullCommandDefaults: Boolean
nullCommandDefaults: Boolean,
allowPositionalCommandArgs: Boolean
) = {
ResolveCore.catchWrapException {
val invoked = invokeCommand0(
p,
r.segments.parts.last,
rootModule.millDiscover.asInstanceOf[Discover[mill.define.Module]],
args,
nullCommandDefaults
nullCommandDefaults,
allowPositionalCommandArgs
)

invoked.head
Expand All @@ -130,7 +142,8 @@ object Resolve {
name: String,
discover: Discover[mill.define.Module],
rest: Seq[String],
nullCommandDefaults: Boolean
nullCommandDefaults: Boolean,
allowPositionalCommandArgs: Boolean
): Iterable[Either[String, Command[_]]] = for {
(cls, (names, entryPoints)) <- discover.value
if cls.isAssignableFrom(target.getClass)
Expand All @@ -154,7 +167,7 @@ object Resolve {
mainargs.TokenGrouping.groupArgs(
rest,
flattenedArgSigsWithDefaults,
allowPositional = true,
allowPositional = allowPositionalCommandArgs,
allowRepeats = false,
allowLeftover = ep.argSigs0.exists(_.reader.isLeftover),
nameMapper = mainargs.Util.kebabCaseNameMapper
Expand Down Expand Up @@ -195,35 +208,59 @@ trait Resolve[T] {
resolved: Seq[Resolved],
args: Seq[String],
segments: Segments,
nullCommandDefaults: Boolean
nullCommandDefaults: Boolean,
allowPositionalCommandArgs: Boolean
): Either[String, Seq[T]]

def resolve(
rootModule: BaseModule,
scriptArgs: Seq[String],
selectMode: SelectMode,
allowPositionalCommandArgs: Boolean = false
): Either[String, List[T]] = {
resolve0(Seq(rootModule), scriptArgs, selectMode, allowPositionalCommandArgs)
}
def resolve(
rootModule: BaseModule,
scriptArgs: Seq[String],
selectMode: SelectMode
): Either[String, List[T]] = {
resolve0(Seq(rootModule), scriptArgs, selectMode)
resolve0(Seq(rootModule), scriptArgs, selectMode, false)
}
def resolve(
rootModules: Seq[BaseModule],
scriptArgs: Seq[String],
selectMode: SelectMode,
allowPositionalCommandArgs: Boolean
): Either[String, List[T]] = {
resolve0(rootModules, scriptArgs, selectMode, allowPositionalCommandArgs)
}
def resolve(
rootModules: Seq[BaseModule],
scriptArgs: Seq[String],
selectMode: SelectMode
): Either[String, List[T]] = {
resolve0(rootModules, scriptArgs, selectMode)
resolve0(rootModules, scriptArgs, selectMode, false)
}

private[mill] def resolve0(
baseModules: Seq[BaseModule],
scriptArgs: Seq[String],
selectMode: SelectMode
selectMode: SelectMode,
allowPositionalCommandArgs: Boolean
): Either[String, List[T]] = {
val nullCommandDefaults = selectMode == SelectMode.Multi
val resolvedGroups = ParseArgs(scriptArgs, selectMode).flatMap { groups =>
val resolved = groups.map { case (selectors, args) =>
val selected = selectors.map { case (scopedSel, sel) =>
resolveRootModule(baseModules, scopedSel).map { rootModuleSels =>
resolveNonEmptyAndHandle(args, rootModuleSels, sel, nullCommandDefaults)
resolveNonEmptyAndHandle(
args,
rootModuleSels,
sel,
nullCommandDefaults,
allowPositionalCommandArgs
)
}
}

Expand All @@ -243,7 +280,8 @@ trait Resolve[T] {
args: Seq[String],
baseModules: BaseModuleTree,
sel: Segments,
nullCommandDefaults: Boolean
nullCommandDefaults: Boolean,
allowPositionalCommandArgs: Boolean
): Either[String, Seq[T]] = {
val rootResolved = ResolveCore.Resolved.Module(Segments(), baseModules.rootModule.getClass)
val resolved =
Expand All @@ -269,7 +307,14 @@ trait Resolve[T] {

resolved
.map(_.toSeq.sortBy(_.segments.render))
.flatMap(handleResolved(baseModules, _, args, sel, nullCommandDefaults))
.flatMap(handleResolved(
baseModules,
_,
args,
sel,
nullCommandDefaults,
allowPositionalCommandArgs
))
}

private[mill] def deduplicate(items: List[T]): List[T] = items
Expand Down
6 changes: 4 additions & 2 deletions main/resolve/test/src/mill/main/ResolveTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,17 @@ object ResolveTests extends TestSuite {
Resolve.Tasks.resolve0(
Seq(module),
selectorStrings,
SelectMode.Separated
SelectMode.Separated,
false
)
}

def resolveMetadata(selectorStrings: Seq[String]) = {
Resolve.Segments.resolve0(
Seq(module),
selectorStrings,
SelectMode.Separated
SelectMode.Separated,
false
).map(_.map(_.render))
}
}
Expand Down
7 changes: 6 additions & 1 deletion main/src/mill/main/MainModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,12 @@ trait MainModule extends BaseModule0 {
* If there are multiple dependency paths between `src` and `dest`, the path
* chosen is arbitrary.
*/
def path(evaluator: Evaluator, src: String, dest: String): Command[List[String]] =

def path(
evaluator: Evaluator,
@mainargs.arg(positional = true) src: String,
@mainargs.arg(positional = true) dest: String
): Command[List[String]] =
Target.command {
val resolved = Resolve.Tasks.resolve(
evaluator.rootModules,
Expand Down
7 changes: 6 additions & 1 deletion main/src/mill/main/RunScript.scala
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ object RunScript {
(Seq[Watchable], Either[String, Seq[(Any, Option[(TaskName, ujson.Value)])]])
] = {
val resolved = mill.eval.Evaluator.currentEvaluator.withValue(evaluator) {
Resolve.Tasks.resolve(evaluator.rootModules, scriptArgs, selectMode)
Resolve.Tasks.resolve(
evaluator.rootModules,
scriptArgs,
selectMode,
evaluator.allowPositionalCommandArgs
)
}
for (targets <- resolved) yield evaluateNamed(evaluator, Agg.from(targets))
}
Expand Down
Loading
Loading