Ensure we register a cycle if we've got scala sources but pipelining …

…is disabled

internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala

@@ -70,14 +70,15 @@ object Incremental {
      * Merge latest analysis as of pickling into pruned previous analysis, compute invalidations
      * and decide whether we need another cycle.
      */
-    def mergeAndInvalidate(partialAnalysis: Analysis, completingCycle: Boolean): CompileCycleResult
+    def mergeAndInvalidate(partialAnalysis: Analysis, registerCycle: Boolean): CompileCycleResult
 
     /**
      * Merge latest analysis as of analyzer into pruned previous analysis and inform file manager.
      */
     def completeCycle(
         prev: Option[CompileCycleResult],
-        partialAnalysis: Analysis
+        partialAnalysis: Analysis,
+        registerCycle: Boolean
     ): CompileCycleResult
 
     def previousAnalysisPruned: Analysis
@@ -938,7 +939,13 @@ private final class AnalysisCallback(
         if (!writtenEarlyArtifacts) // writing implies the updates merge has happened
           mergeUpdates() // must merge updates each cycle or else scalac will clobber it
       }
-      incHandler.completeCycle(invalidationResults, getAnalysis)
+
+      val partialAnalysis = getAnalysis
+      //val hasScala = Analysis.sources(partialAnalysis).scala.nonEmpty
+      // If we had early output and scala sources, then the cycle has already been registered
+      val registerCycle = true
+
+      incHandler.completeCycle(invalidationResults, partialAnalysis, registerCycle)
     } else {
       throw new IllegalStateException(
         "can't call AnalysisCallback#getCycleResultOnce more than once"
@@ -1090,12 +1097,12 @@ private final class AnalysisCallback(
 
   override def apiPhaseCompleted(): Unit = {
     // If we know we're done with cycles (presumably because all sources were invalidated) we can store early analysis
-    // and picke data now.  Otherwise, we need to wait for dependency information to decide if there are more cycles.
+    // and pickle data now.  Otherwise, we need to wait for dependency information to decide if there are more cycles.
     incHandlerOpt foreach { incHandler =>
       if (earlyOutput.isDefined && incHandler.isFullCompilation) {
         val a = getAnalysis
         val CompileCycleResult(continue, invalidations, merged) =
-          incHandler.mergeAndInvalidate(a, false)
+          incHandler.mergeAndInvalidate(a, true)
         if (lookup.shouldDoEarlyOutput(merged)) {
           assert(
             !continue && invalidations.isEmpty,
@@ -1113,7 +1120,7 @@ private final class AnalysisCallback(
     if (earlyOutput.isDefined && invalidationResults.isEmpty) {
       val a = getAnalysis
       val CompileCycleResult(continue, invalidations, merged) =
-        incHandler.mergeAndInvalidate(a, false)
+        incHandler.mergeAndInvalidate(a, true)
       // Store invalidations and continuation decision; the analysis will be computed again after Analyze phase.
       invalidationResults = Some(CompileCycleResult(continue, invalidations, Analysis.empty))
       // If there will be no more compilation cycles, store the early analysis file and update the pickle jar

internal/zinc-core/src/main/scala/sbt/internal/inc/IncrementalCommon.scala

@@ -158,7 +158,7 @@ private[inc] abstract class IncrementalCommon(
 
       override def mergeAndInvalidate(
           partialAnalysis: Analysis,
-          completingCycle: Boolean
+          registerCycle: Boolean,
       ): CompileCycleResult = {
         val analysis =
           if (isFullCompilation)
@@ -190,23 +190,21 @@ private[inc] abstract class IncrementalCommon(
         val continue = nextInvalidations.nonEmpty &&
           lookup.shouldDoIncrementalCompilation(nextInvalidations, analysis)
 
-        val hasScala = Analysis.sources(partialAnalysis).scala.nonEmpty
-
-        // If we're completing the cycle and we had scala sources, then mergeAndInvalidate has already been called
-        if (!completingCycle || !hasScala) {
-          registerCycle(recompiledClasses, newApiChanges, nextInvalidations, continue)
+        if (registerCycle) {
+          this.registerCycle(recompiledClasses, newApiChanges, nextInvalidations, continue)
         }
         CompileCycleResult(continue, nextInvalidations, analysis)
       }
 
       override def completeCycle(
           prev: Option[CompileCycleResult],
-          partialAnalysis: Analysis
+          partialAnalysis: Analysis,
+          registerCycle: Boolean
       ): CompileCycleResult = {
         classFileManager.generated(partialAnalysis.relations.allProducts.map(toVf).toArray)
         prev match {
           case Some(prev) => prev.copy(analysis = pruned ++ partialAnalysis)
-          case _          => mergeAndInvalidate(partialAnalysis, true)
+          case _          => mergeAndInvalidate(partialAnalysis, registerCycle)
         }
       }
     }

internal/zinc-scripted/src/test/scala/sbt/internal/inc/IncHandler.scala

@@ -26,11 +26,14 @@ import xsbti.{ FileConverter, Problem, Severity, VirtualFileRef, VirtualFile }
 import xsbti.compile.{
   AnalysisContents,
   AnalysisStore,
+  ClassFileManager => XClassFileManager,
   ClasspathOptionsUtil,
   CompileAnalysis,
   CompileOrder,
   CompileProgress,
+  DefaultExternalHooks,
   DefinesClass,
+  ExternalHooks,
   IncOptions,
   IncOptionsUtil,
   PerClasspathEntryLookup,
@@ -222,6 +225,9 @@ class IncHandler(directory: Path, cacheDir: Path, scriptedLog: ManagedLogger, co
     onArgs("checkIterations") {
       case (p, x :: Nil, i) => p.checkNumberOfCompilerIterations(i, x.toInt)
     },
+    onArgs("checkCycles") {
+      case (p, x :: Nil, i) => p.checkNumberOfCycles(i, x.toInt)
+    },
     // note that this can only tell us the *last* round a class got compiled in.
     // it can't tell us *every* round something got compiled in, since only
     // still-extant classfiles are available for inspection
@@ -328,6 +334,7 @@ case class ProjectStructure(
   val earlyCacheFile = baseDirectory / "target" / "early" / "inc_compile.zip"
   val earlyAnalysisStore = FileAnalysisStore.binary(earlyCacheFile.toFile)
   // val earlyCachedStore = AnalysisStore.cached(fileStore)
+  val profiler = new ZincInvalidationProfiler
 
   // We specify the class file manager explicitly even though it's noew possible
   // to specify it in the incremental option property file (this is the default for sbt)
@@ -424,6 +431,15 @@ case class ProjectStructure(
       ()
     }
 
+  def checkNumberOfCycles(i: IncState, expected: Int): Future[Unit] =
+    compile(i).map { _ =>
+      import scala.collection.JavaConverters._
+      val count = profiler.toProfile.getRunsList.asScala.map(_.getCyclesList.size).sum
+      val msg = s"Expected $expected cycles, got $count"
+      assert(count == expected, msg)
+      ()
+    }
+
   def checkClasses(i: IncState, src: String, expected: List[String]): Future[Unit] =
     compile(i).map { analysis =>
       def classes(src: String): Set[String] =
@@ -783,10 +799,13 @@ case class ProjectStructure(
     import scala.collection.JavaConverters._
     val map = new java.util.HashMap[String, String]
     properties.asScala foreach { case (k: String, v: String) => map.put(k, v) }
+    val externalHooks = new DefaultExternalHooks(Optional.empty[ExternalHooks.Lookup], Optional.empty[XClassFileManager])
+      .withInvalidationProfiler(profiler)
     val base = IncOptions
       .of()
       .withPipelining(defaultPipelining)
       .withApiDebug(true)
+      .withExternalHooks(externalHooks)
     // .withRelationsDebug(true)
     val incOptions = {
       val opts = IncOptionsUtil.fromStringMap(base, map, scriptedLog)

zinc/src/sbt-test/profiler/check-cycles-no-pipelining/A.scala

@@ -0,0 +1,13 @@
+/*
+ * Zinc - The incremental compiler for Scala.
+ * Copyright Scala Center, Lightbend, and Mark Harrah
+ *
+ * Licensed under Apache License 2.0
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+class A()
+

zinc/src/sbt-test/profiler/check-cycles-no-pipelining/changes/A.scala

@@ -0,0 +1,16 @@
+/*
+ * Zinc - The incremental compiler for Scala.
+ * Copyright Scala Center, Lightbend, and Mark Harrah
+ *
+ * Licensed under Apache License 2.0
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+class A()
+
+
+
+// random placeholder change

zinc/src/sbt-test/profiler/check-cycles-no-pipelining/incOptions.properties

@@ -0,0 +1 @@
+pipelining = false

zinc/src/sbt-test/profiler/check-cycles-no-pipelining/test

@@ -0,0 +1,5 @@
+> compile
+
+$ copy-file changes/A.scala A.scala
+
+> checkCycles 2

zinc/src/sbt-test/profiler/check-cycles/A.scala

@@ -0,0 +1,13 @@
+/*
+ * Zinc - The incremental compiler for Scala.
+ * Copyright Scala Center, Lightbend, and Mark Harrah
+ *
+ * Licensed under Apache License 2.0
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+class A()
+

zinc/src/sbt-test/profiler/check-cycles/changes/A.scala

@@ -0,0 +1,16 @@
+/*
+ * Zinc - The incremental compiler for Scala.
+ * Copyright Scala Center, Lightbend, and Mark Harrah
+ *
+ * Licensed under Apache License 2.0
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * See the NOTICE file distributed with this work for
+ * additional information regarding copyright ownership.
+ */
+
+class A()
+
+
+
+// random placeholder change

zinc/src/sbt-test/profiler/check-cycles/test

@@ -0,0 +1,5 @@
+> compile
+
+$ copy-file changes/A.scala A.scala
+
+> checkCycles 2