com-lihaoyi / mill

Mill is a fast JVM build tool that supports Java and Scala. 2-3x faster than Gradle and 5-10x faster than Maven for common workflows, Mill aims to make your project’s build process performant, maintainable, and flexible
https://mill-build.org/
MIT License
2.03k stars 319 forks source link

Redesign tasks that get an evaluator instance #502

Open lefou opened 5 years ago

lefou commented 5 years ago

See https://github.com/lihaoyi/mill/pull/480#issuecomment-436927941 and https://github.com/lihaoyi/mill/issues/491#issuecomment-444006357

Currently, all, show, resolve, clean and other built-in tasks take an evaluator instance and are part of the task graph itself, which is somewhat in-elegant. They take part of caching, force the existence of the out folder (even the clean task) and trigger dependency resolution. Also, the interaction and ordering in case of parallel task execution is unclear.

lihaoyi commented 5 years ago

I think all those tasks should not be Tasks. That means:

Essentially, I'd like them to be "dumb" main/entrypoint methods. If they need to do stuff with the Evaluator, they can take an Evaluator instance and call evaluate themselves.

lefou commented 5 years ago

Do we want to support an Evaluator magic task parameter at all? How do we want to contribute (by plugins) tasks like mill.scalalib.GenIdea/ideain the future?

lihaoyi commented 5 years ago

I think Evaluator would need to not be a magic-task-parameter, but a magic-main-method-parameter; we can set up the implicits so tasks and main methods can share most of the command-line deserializers, but can also have some that are unique to each (though for now I can only think of Evaluator)

mill.scalalib.GenIdea/idea would be a main-method rather than a T.command, and users could define their own in their build.sc or in ExternalModules

lefou commented 5 years ago

To call a main-method via cmdline, we need some proper syntax.

As buildfiles and external modules are just valid Scala, there should be no name collision when using the same syntax as for targets. When parsing the cmdline, we need to decide if the first parameter is a main-method or a target. But I have no idea how to elegantly solve this for built-in mains like clean, all, etc. Without some prefix or namespace, the user would expect those to be either targets or main-methods in build.sc.

A magic Evaluator parameter is only accepted, if the def is not a target but a simple scala method (aka main-method).

How could we map built-ins to avoid name collisions:

I like the mapping to options, either generic/automatic or manual. This would be more idiomatic in the sense of same-as-in-other-tools.

lihaoyi commented 5 years ago

When parsing the cmdline, we need to decide if the first parameter is a main-method or a target.

I think for now we can assume there is only one method/target/etc. of the same name in the same module.

While Scala allows you to overload things, and you can have e.g. multiple T.commands of the same name but different parameters, a lot of our code assumes we only have one-thing-per-name. I don't even know what happens if there are overloads, but I assume it doesn't treat it elegantly.

So for now, no need to worry about naming collisions; that's an orthogonal issue to changing the semantics of main methods and can be fixed separately.

But I have no idea how to elegantly solve this for built-in mains like clean, all, etc. Without some prefix or namespace, the user would expect those to be either targets or main-methods in build.sc.

They can always ask:

lihaoyi mill$ mill inspect clean
[1/1] inspect
clean(MainModule.scala:196)
    Deletes the given targets from the out directory. Providing no targets
    will clean everything.

Inputs:

lihaoyi mill$ mill inspect release
[1/1] inspect
release(build.sc:455)

Inputs:
    publishVersion
    dev.runClasspath

This is no different from any other "where did i inherit something from" question, which is an existing issue separate from main method semantics.

How could we map built-ins to avoid name collisions

Again, this is an existing problem, so no need to fix it in the context of this issue. We've already discussed this, and I'm inclined to leave things in the same namespace for now. We can move forward on the things we agree on (changing main method semantics to allow parallelization) without waiting for consensus on unrelated things (cleaning up namespaces) that we can continue to discuss