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

Granular cache invalidation with multiple build files #1663

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5f8208a
More granular cache invalidaton with multiple file
lolgab Jan 11, 2022
033585c
Remove debugging code and use Loose.Agg for classes
lolgab Jan 11, 2022
c53db35
Fix comment in transitiveTargets
lolgab Jan 11, 2022
23f3adc
Add test with invalidation of task in modules
lolgab Jan 11, 2022
0fdac14
Handle inner classes in ammonite scripts
lolgab Jan 11, 2022
9c6f2c7
Make ScriptInvalidationTests run with fork = false
lolgab Jan 11, 2022
b9ba8a3
Handle some cases of `^` ammonite import
lolgab Jan 11, 2022
667b192
Add fork = true variant of tests
lolgab Jan 11, 2022
717695b
Properly normalize ammonite import paths
lolgab Jan 13, 2022
f1b3aa8
Proper implementation of links to graph structure
lolgab Jan 14, 2022
c7e6313
Use the same normalization mechanism everywhere
lolgab Jan 14, 2022
5d635d0
Fix binary incompatibilities
lolgab Jan 14, 2022
31b8a71
Handle mappings in ammonite imports
lolgab Jan 14, 2022
35647d3
Fix some binary compatibility shims
lolgab Jan 15, 2022
cca0a57
Revert Scalafmt changes
lolgab Jan 15, 2022
701c7c9
Add other TODOs and deprecations
lolgab Jan 15, 2022
8ae634e
Revert Scalafmt changes
lolgab Jan 15, 2022
65b1b5a
Do not pass importTree to ReplApplyHandler
lolgab Jan 15, 2022
1b1667c
Deprecate copy method in evaluator
lolgab Jan 15, 2022
abf747d
Remove `protected[this]` from methods
lolgab Jan 15, 2022
56f0445
Fix Evaluator new methods to not have default
lolgab Jan 15, 2022
82e67fb
Add @mill.api.internal to Utils and GraphUtils
lolgab Jan 15, 2022
3ed242e
Merge remote-tracking branch 'origin/main' into granular-cache-invali…
lolgab Feb 15, 2022
8bd0207
Revert scalafmt unrelated changes
lolgab Feb 15, 2022
7a04db8
Remove wrong change
lolgab Feb 15, 2022
2749b9e
Revert scalafmt unrelated changes
lolgab Feb 15, 2022
f87a70b
Revert other changes
lolgab Feb 15, 2022
92f00e6
Rename Utils to AmmoniteUtils
lolgab Feb 15, 2022
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
3 changes: 3 additions & 0 deletions integration/test/resources/invalidation-foreign/build.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def taskC = T {
println("c")
}
12 changes: 12 additions & 0 deletions integration/test/resources/invalidation-foreign/foreignA/build.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import $file.^.foreignB.build
import $file.^.build

def taskA = T {
^.foreignB.build.taskB()
println("a")
}

def taskD = T {
^.build.taskC()
println("d")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def taskB = T {
println("b")
}
3 changes: 3 additions & 0 deletions integration/test/resources/invalidation/a/inputA.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def input = T {
println("a")
}
5 changes: 5 additions & 0 deletions integration/test/resources/invalidation/b/inputB.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import $file.inputD
def input = T {
inputD.method()
println("b")
}
3 changes: 3 additions & 0 deletions integration/test/resources/invalidation/b/inputD.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def method() = {
println("d")
}
25 changes: 25 additions & 0 deletions integration/test/resources/invalidation/build.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import $file.a.inputA
import $file.b.{inputB => inputBRenamed}
import $file.inputC
import $ivy.`org.scalaj::scalaj-http:2.4.2`
import $file.e.inputE

def task = T {
inputA.input()
inputBRenamed.input()
inputC.input()
}

object module extends Module {
def task = T {
println("task")
inputA.input()
inputBRenamed.input()
inputC.input()
}
}

def taskE = T {
println("taskE")
inputE.input()
}
6 changes: 6 additions & 0 deletions integration/test/resources/invalidation/e/inputE.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import $file.^.a.inputA

def input = T {
println("e")
inputA.input()
}
3 changes: 3 additions & 0 deletions integration/test/resources/invalidation/inputC.sc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
def input = T {
println("c")
}
71 changes: 71 additions & 0 deletions integration/test/src/ScriptsInvalidationForeignTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package mill.integration

import mill.util.ScriptTestSuite
import utest._

import scala.collection.mutable

class ScriptsInvalidationForeignTests(fork: Boolean) extends ScriptTestSuite(fork) {
def workspaceSlug: String = "invalidation-foreign"
def scriptSourcePath: os.Path = os.pwd / "integration" / "test" / "resources" / workspaceSlug
override def buildPath = os.sub / "foreignA" / "build.sc"

def runTask(task: String) = {
val (successful, stdout) = evalStdout(task)
assert(successful)
stdout.map(_.trim)
}

val tests = Tests {
test("should handle foreign modules") {
test("first run") {
initWorkspace()

val result = runTask("taskA")

val expected = Seq("b", "a")

assert(result == expected)
}

test("second run modifying script") {
val oldContent = os.read(scriptSourcePath / buildPath)
val newContent = s"""$oldContent
|def newTask = T { }
|""".stripMargin
os.write.over(workspacePath / buildPath, newContent)

val result = runTask("taskA")

val expected = Seq("a")

assert(result == expected)
}
}
test("should handle imports in higher level than top level") {
test("first run") {
initWorkspace()

val result = runTask("taskD")

val expected = Seq("c", "d")

assert(result == expected)
}

test("second run modifying script") {
val oldContent = os.read(scriptSourcePath / buildPath)
val newContent = s"""$oldContent
|def newTask = T { }
|""".stripMargin
os.write.over(workspacePath / buildPath, newContent)

val result = runTask("taskD")

val expected = Seq("d")

assert(result == expected)
}
}
}
}
113 changes: 113 additions & 0 deletions integration/test/src/ScriptsInvalidationTests.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package mill.integration

import mill.util.ScriptTestSuite
import utest._

import scala.collection.mutable

class ScriptsInvalidationTests(fork: Boolean) extends ScriptTestSuite(fork) {
def workspaceSlug: String = "invalidation"
def scriptSourcePath: os.Path = os.pwd / "integration" / "test" / "resources" / workspaceSlug

def runTask(task: String) = {
val (successful, stdout) = evalStdout(task)
assert(successful)
stdout.map(_.trim)
}

val tests = Tests {
test("should not invalidate tasks in different untouched sc files") {
test("first run") {
initWorkspace()

val result = runTask("task")

val expected = Seq("a", "d", "b", "c")

assert(result == expected)
}

test("second run modifying script") {
val oldContent = os.read(scriptSourcePath / buildPath)
val newContent = s"""$oldContent
|def newTask = T { }
|""".stripMargin
os.write.over(workspacePath / buildPath, newContent)

val stdout = runTask("task")

assert(stdout.isEmpty)
}
}
test("should invalidate tasks if leaf file is changed") {
test("first run") {
initWorkspace()

val result = runTask("task")
val expected = Seq("a", "d", "b", "c")

assert(result == expected)
}

test("second run modifying script") {
val inputD = os.sub / "b" / "inputD.sc"
val oldContent = os.read(scriptSourcePath / inputD)
val newContent = s"""$oldContent
|def newTask = T { }
|""".stripMargin
os.write.over(workspacePath / inputD, newContent)

val result = runTask("task")
val expected = Seq("d", "b")

assert(result == expected)
}
}
test("should handle submodules in scripts") {
test("first run") {
initWorkspace()

val result = runTask("module.task")
val expected = Seq("a", "d", "b", "c", "task")

assert(result == expected)
}

test("second run modifying script") {
val oldContent = os.read(scriptSourcePath / buildPath)
val newContent = s"""$oldContent
|def newTask = T { }
|""".stripMargin
os.write.over(workspacePath / buildPath, newContent)

val result = runTask("module.task")
val expected = Seq("task")

assert(result == expected)
}
}
test("should handle ammonite ^ imports") {
test("first run") {
initWorkspace()

val result = runTask("taskE")
val expected = Seq("a", "e", "taskE")

assert(result == expected)
}

test("second run modifying script") {
val oldContent = os.read(scriptSourcePath / buildPath)
val newContent = s"""$oldContent
|def newTask = T { }
|""".stripMargin
os.write.over(workspacePath / buildPath, newContent)

val result = runTask("taskE")
val expected = Seq("taskE")

assert(result == expected)
}
}
}
}
3 changes: 3 additions & 0 deletions integration/test/src/forked/Tests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ object UpickleTests extends mill.integration.UpickleTests(fork = true)
object PlayJsonTests extends mill.integration.PlayJsonTests(fork = true)
object CaffeineTests extends mill.integration.CaffeineTests(fork = true)
object DocAnnotationsTests extends mill.integration.DocAnnotationsTests(fork = true)
object ScriptsInvalidationTests extends mill.integration.ScriptsInvalidationTests(fork = true)
object ScriptsInvalidationForeignTests
extends mill.integration.ScriptsInvalidationForeignTests(fork = true)
3 changes: 3 additions & 0 deletions integration/test/src/local/Tests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ object UpickleTests extends mill.integration.UpickleTests(fork = false)
object PlayJsonTests extends mill.integration.PlayJsonTests(fork = false)
object CaffeineTests extends mill.integration.CaffeineTests(fork = false)
object DocAnnotationsTests extends mill.integration.DocAnnotationsTests(fork = false)
object ScriptsInvalidationTests extends mill.integration.ScriptsInvalidationTests(fork = false)
lefou marked this conversation as resolved.
Show resolved Hide resolved
object ScriptsInvalidationForeignTests
extends mill.integration.ScriptsInvalidationForeignTests(fork = false)
20 changes: 14 additions & 6 deletions main/core/src/mill/define/Graph.scala
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,25 @@ object Graph {
* including the given targets.
*/
def transitiveTargets(sourceTargets: Agg[Task[_]]): Agg[Task[_]] = {
val transitiveTargets = new Agg.Mutable[Task[_]]
def rec(t: Task[_]): Unit = {
if (transitiveTargets.contains(t)) () // do nothing
transitiveNodes(sourceTargets)
}

/**
* Collects all transitive dependencies (nodes) of the given nodes,
* including the given nodes.
*/
def transitiveNodes[T <: GraphNode[T]](sourceNodes: Agg[T]): Agg[T] = {
val transitiveNodes = new Agg.Mutable[T]
def rec(t: T): Unit = {
if (transitiveNodes.contains(t)) () // do nothing
else {
transitiveTargets.append(t)
transitiveNodes.append(t)
t.inputs.foreach(rec)
}
}

sourceTargets.items.foreach(rec)
transitiveTargets
sourceNodes.items.foreach(rec)
transitiveNodes
}

/**
Expand Down
9 changes: 9 additions & 0 deletions main/core/src/mill/define/GraphNode.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package mill.define

trait GraphNode[T] {

/**
* What other nodes does this node depend on?
*/
def inputs: Seq[T]
}
3 changes: 3 additions & 0 deletions main/core/src/mill/define/ScriptNode.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package mill.define

case class ScriptNode(cls: String, inputs: Seq[ScriptNode]) extends GraphNode[ScriptNode]
2 changes: 1 addition & 1 deletion main/core/src/mill/define/Task.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import scala.reflect.macros.blackbox.Context
* Generally not instantiated manually, but instead constructed via the
* [[Target.apply]] & similar macros.
*/
abstract class Task[+T] extends Task.Ops[T] with Applyable[Task, T] {
abstract class Task[+T] extends Task.Ops[T] with Applyable[Task, T] with GraphNode[Task[_]] {

/**
* What other Targets does this Target depend on?
Expand Down
Loading