Skip to content

Commit

Permalink
Make input tasks only require a JSON writer, not reader (com-lihaoyi#…
Browse files Browse the repository at this point in the history
…1601)

Resolves com-lihaoyi#598. Input tasks results are re-evaluated every time and thus never read from the disk, and so a `upickle.default.Reader` is not required. Nevertheless, we still require that a `upickle.default.Writer` in order to generate the `*.json` files used for `./mill show` and other debugging purposes.

Targeting com-lihaoyi#1600 to avoid merge conflicts

Let's see what CI says...

TBH not sure if this is worth doing, or whether we should just specify that `Input` tasks require a `ReadWriter` just to keep things consistent. We don't currently have any use case for de-serializing the input task JSON (e.g. even `mill show` takes the JSON from disk and spits it out verbatim without de-serializing) but that doesn't mean we won't find such use cases in future. But if such use cases do re-appear, we can always add that functionality back. `Command`s are write-only, so making `Input`s write-only would also not be unprecedented
  • Loading branch information
lihaoyi authored and pbuszka committed Dec 26, 2021
1 parent 97ad7ad commit 7ed9953
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 16 deletions.
2 changes: 1 addition & 1 deletion docs/antora/modules/ROOT/pages/Tasks.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ different Task types:
| |Target |Command |Source/Input |Anonymous Task |Persistent Target |Worker

|Cached to Disk |X |X | | |X |
|Must be JSON Writable |X |X | | |X |
|Must be JSON Writable |X |X |X| |X |
|Must be JSON Readable |X | | | |X |
|Runnable from the Command Line |X |X | | |X |
|Can Take Arguments | |X | |X | |
Expand Down
16 changes: 6 additions & 10 deletions main/core/src/mill/define/Task.scala
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,11 @@ object Target extends TargetGenerated with Applicative.Applyer[Task, Task, Resul
)
)
}
def input[T](value: Result[T])(implicit rw: RW[T], ctx: mill.define.Ctx): Input[T] =
def input[T](value: Result[T])(implicit w: upickle.default.Writer[T], ctx: mill.define.Ctx): Input[T] =
macro inputImpl[T]

def inputImpl[T: c.WeakTypeTag](c: Context)(value: c.Expr[T])(
rw: c.Expr[RW[T]],
w: c.Expr[upickle.default.Writer[T]],
ctx: c.Expr[mill.define.Ctx]
): c.Expr[Input[T]] = {
import c.universe._
Expand All @@ -199,7 +199,7 @@ object Target extends TargetGenerated with Applicative.Applyer[Task, Task, Resul
new Input[T](
Applicative.impl[Task, T, mill.api.Ctx](c)(value).splice,
ctx.splice,
rw.splice
w.splice
)
)
)
Expand Down Expand Up @@ -328,20 +328,16 @@ class Persistent[+T](t: Task[T], ctx0: mill.define.Ctx, readWrite: RW[_])
override def flushDest = false
}

class Input[T](t: Task[T], ctx0: mill.define.Ctx, val readWrite: RW[_])
extends NamedTaskImpl[T](ctx0, t)
with Target[T] {
class Input[T](t: Task[T], ctx0: mill.define.Ctx, val writer: upickle.default.Writer[_])
extends NamedTaskImpl[T](ctx0, t) {
override def sideHash = util.Random.nextInt()
}

class Sources(t: Task[Seq[PathRef]], ctx0: mill.define.Ctx)
extends Input[Seq[PathRef]](
t,
ctx0,
RW.join(
upickle.default.SeqLikeReader[Seq, PathRef],
upickle.default.SeqLikeWriter[Seq, PathRef]
)
upickle.default.SeqLikeWriter[Seq, PathRef]
)

class Source(t: Task[PathRef], ctx0: mill.define.Ctx)
Expand Down
3 changes: 2 additions & 1 deletion main/core/src/mill/eval/Evaluator.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ import scala.jdk.CollectionConverters._
import scala.reflect.ClassTag
import scala.util.control.NonFatal

case class Labelled[T](task: NamedTask[T], segments: Segments) {
case class Labelled[T](task: NamedTask[T], segments: Segments){
def format = task match {
case t: Target[T] => Some(t.readWrite.asInstanceOf[upickle.default.ReadWriter[T]])
case _ => None
}
def writer = task match {
case t: mill.define.Command[T] => Some(t.writer.asInstanceOf[upickle.default.Writer[T]])
case t: mill.define.Input[T] => Some(t.writer.asInstanceOf[upickle.default.Writer[T]])
case t: Target[T] => Some(t.readWrite.asInstanceOf[upickle.default.ReadWriter[T]])
case _ => None
}
Expand Down
8 changes: 4 additions & 4 deletions main/src/mill/main/Resolve.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ object ResolveMetadata extends Resolve[String] {
val targets =
obj
.millInternal
.reflectAll[Target[_]]
.reflectAll[NamedTask[_]]
.map(_.toString)
val commands =
for {
Expand Down Expand Up @@ -192,15 +192,15 @@ object ResolveTasks extends Resolve[NamedTask[Any]] {
Right(
obj.millInternal.modules
.filter(_ != obj)
.flatMap(m => m.millInternal.reflectAll[Target[_]])
.flatMap(m => m.millInternal.reflectAll[NamedTask[_]])
)
case "_" => Right(obj.millInternal.reflectAll[Target[_]])
case "_" => Right(obj.millInternal.reflectAll[NamedTask[_]])

case _ =>
val target =
obj
.millInternal
.reflectSingle[Target[_]](last)
.reflectSingle[NamedTask[_]](last)
.map(Right(_))

val command = Resolve.invokeCommand(
Expand Down

0 comments on commit 7ed9953

Please sign in to comment.