Overcompilation for non-API changes with circular dependencies

[jackkoenig]

This is a regression in Zinc 1.10.0-M1, the behavior is correct in 1.9.6.

steps

Apply the following patch to the zinc git repository:

diff --git a/zinc/src/test/scala/sbt/inc/IncrementalCompilerSpec.scala b/zinc/src/test/scala/sbt/inc/IncrementalCompilerSpec.scala
index dc7a552fa..fa5f4994b 100644
--- a/zinc/src/test/scala/sbt/inc/IncrementalCompilerSpec.scala
+++ b/zinc/src/test/scala/sbt/inc/IncrementalCompilerSpec.scala
@@ -147,6 +147,36 @@ class IncrementalCompilerSpec extends BaseCompilerSpec {
       }
   }

+  it should "not trigger recompile circular dependencies for non-API changes" in withTmpDir {
+    tmp =>
+      val project = VirtualSubproject(tmp.toPath / "p1")
+      val comp = project.setup.createCompiler()
+      try {
+        val s1 =
+          """|class A {
+             |  def b = new B
+             |}
+          |""".stripMargin
+        val s1b =
+          """|class A {
+             |  println("Hello World!")
+             |  def b = new B
+             |}
+          |""".stripMargin
+        val s2 = "class B { def b = new A }"
+
+        val f1 = StringVirtualFile("A.scala", s1)
+        val f1b = StringVirtualFile("A.scala", s1b)
+        val f2 = StringVirtualFile("B.scala", s2)
+
+        comp.compile(f1, f2)
+        val result = comp.compile(f1b, f2)
+        assert(lastClasses(result.analysis.asInstanceOf[Analysis]) == Set("A"))
+      } finally {
+        comp.close()
+      }
+  }
+
   it should "track dependencies from nested inner Java classes" in withTmpDir { tmp =>
     val project = VirtualSubproject(tmp.toPath / "p1")
     val comp = project.setup.createCompiler()

Run this test

$ sbt
> zinc / testOnly sbt.inc.IncrementalCompilerSpec -- -z "not trigger recompile circular dependencies"

problem

In the test, only A should be recompiled. This is the case (ie. correct) in Zinc 1.9.6, but not (ie. broken) in 1.10.0-M1 and 1.10.0-M2.

I bisected the issue to this commit: https://github.com/sbt/zinc/commit/8f40c4154e240c3cc3760b79969123eaadda587e Considering the commit message is Include mutual dependency in initial invalidation, then maybe this is intended behavior, but it certainly is an over-compilation in cases like this one.

expectation

It would be better if non-public API changes did not cause downstream recompilations even if there are circular dependencies.

notes

[jackkoenig]

I thought this was causing a lot of problems in my work codebase, but I don't actually think it is. I think it's something else in between 1.10.0-M1 and 1.10.0-M2. This particular issue is probably just "working as intended".

[eed3si9n]

This particular issue is probably just "working as intended".

So is it ok to close this issue?