Merge pull request #883 from eed3si9n/wip/use_vs_export

Distinguish between using pipelining vs exporting pipelining (earlyOutput)
sbt · Aug 16, 2020 · 6078b60 · 6078b60
2 parents 6edccf6 + b4b68ba
commit 6078b60
Show file tree

Hide file tree

Showing 15 changed files with 103 additions and 39 deletions.
diff --git a/internal/compiler-interface/src/main/contraband-java/xsbti/compile/IncOptions.java b/internal/compiler-interface/src/main/contraband-java/xsbti/compile/IncOptions.java
@@ -451,7 +451,14 @@ public boolean strictMode() {
     public boolean allowMachinePath() {
         return this.allowMachinePath;
     }
-    /** Enables build pipelining at the module level. */
+    /**
+     * Enabled when build pipelining is used for this subproject.
+     * The consumption of early output (pickle JAR) is dependent on the Scala version,
+     * so this flag in Zinc effectively means skip Javac invocation.
+     * 
+     * Note that contribution to pipelining by exporting early output is signaled by
+     * setting an earlyOutput in CompileOptions.
+     */
     public boolean pipelining() {
         return this.pipelining;
     }

diff --git a/internal/compiler-interface/src/main/contraband/incremental.contra b/internal/compiler-interface/src/main/contraband/incremental.contra
@@ -92,7 +92,12 @@ type IncOptions {
   allowMachinePath: Boolean! = raw"xsbti.compile.IncOptions.defaultAllowMachinePath()"
   @since("1.4.0")
 
-  ## Enables build pipelining at the module level.
+  ## Enabled when build pipelining is used for this subproject.
+  ## The consumption of early output (pickle JAR) is dependent on the Scala version,
+  ## so this flag in Zinc effectively means skip Javac invocation.
+  ##
+  ## Note that contribution to pipelining by exporting early output is signaled by
+  ## setting an earlyOutput in CompileOptions.
   pipelining: Boolean! = raw"xsbti.compile.IncOptions.defaultPipelining()"
   @since("1.4.0")
 

diff --git a/internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala b/internal/zinc-core/src/main/scala/sbt/internal/inc/Incremental.scala
@@ -152,7 +152,7 @@ object Incremental {
       previous.relations.classNames(f)
     val externalAPI = getExternalAPI(lookup)
 
-    val earlyJar = for (early <- earlyOutput; jar <- jo2o(early.getSingleOutputAsPath)) yield jar
+    val earlyJar = extractEarlyJar(earlyOutput)
     val pickleJarPair = earlyJar.map { p =>
       val newName = s"${p.getFileName.toString.stripSuffix(".jar")}-${UUID.randomUUID()}.jar"
       val updatesJar = p.resolveSibling(newName)
@@ -210,6 +210,14 @@ object Incremental {
     } finally runProfiler.registerRun()
   }
 
+  def extractEarlyJar(earlyOutput: Option[Output]): Option[Path] =
+    for {
+      early <- earlyOutput
+      jar <- jo2o(early.getSingleOutputAsPath)
+    } yield jar
+
+  def isPickleJava(scalacOptions: Seq[String]): Boolean = scalacOptions.contains("-Ypickle-java")
+
   /**
    * Compile all Java sources, ignoring incrementality.
    * We are using Incremental class because we still need to perform Analysis so other subprojects
@@ -348,21 +356,18 @@ object Incremental {
     val (initialInvClasses, initialInvSources0) =
       incremental.invalidateInitial(previous.relations, initialChanges)
 
-    // During pipelining, if there's any compilation at all, invalidate all Java sources too, so the downstream Scala subprojects would have type information via early output (pickle jar).
+    // During early output, if there's any compilation at all, invalidate all Java sources too, so the downstream Scala subprojects would have type information via early output (pickle jar).
     val javaSources: Set[VirtualFileRef] = sources.collect {
       case s: VirtualFileRef if s.name.endsWith(".java") => s
     }
     val scalacOptions = currentSetup.options.scalacOptions
+    val earlyJar = extractEarlyJar(earlyOutput)
     val isPickleWrite = scalacOptions.contains("-Ypickle-write")
-    if (isPickleWrite) {
+    if (earlyOutput.isDefined || isPickleWrite) {
       val idx = scalacOptions.indexOf("-Ypickle-write")
       val p =
         if (scalacOptions.size <= idx + 1) None
         else Some(Paths.get(scalacOptions(idx + 1)))
-      val earlyJar = for {
-        earlyO <- earlyOutput
-        jar <- jo2o(earlyO.getSingleOutputAsPath())
-      } yield jar
       (p, earlyJar) match {
         case (None, _)            => log.warn(s"-Ypickle-write is specified but <path> is not?")
         case (x1, x2) if x1 == x2 => ()
@@ -371,17 +376,15 @@ object Incremental {
             s"early output must match -Ypickle-write path '$p' but was '$earlyJar' instead"
           )
       }
-    } else if (!isPickleWrite && options.pipelining) {
-      log.warn(s"-Ypickle-write should be included into scalacOptions if pipelining is enabled")
     }
-    val isPickleJava = currentSetup.options.scalacOptions.contains("-Ypickle-java")
-    if (javaSources.nonEmpty && options.pipelining && !isPickleJava) {
+    val pickleJava = isPickleJava(scalacOptions)
+    if (javaSources.nonEmpty && earlyOutput.isDefined && !pickleJava) {
       log.warn(
-        s"-Ypickle-java should be included into scalacOptions if pipelining is enabled with Java sources"
+        s"-Ypickle-java should be included into scalacOptions if early output is enabled with Java sources"
       )
     }
     val initialInvSources =
-      if (isPickleJava && initialInvSources0.nonEmpty) initialInvSources0 ++ javaSources
+      if (pickleJava && initialInvSources0.nonEmpty) initialInvSources0 ++ javaSources
       else initialInvSources0
     if (initialInvClasses.nonEmpty || initialInvSources.nonEmpty) {
       if (initialInvSources == sources)
@@ -410,7 +413,7 @@ object Incremental {
             1
           )
         else {
-          if (options.pipelining)
+          if (earlyOutput.isDefined)
             writeEarlyOut(lookup, progress, earlyOutput, previous, new java.util.HashSet)
           previous
         }
@@ -862,7 +865,7 @@ private final class AnalysisCallback(
       }
     outputJarContent.scalacRunCompleted()
     val a = getAnalysis
-    if (options.pipelining) {
+    if (earlyOutput.isDefined) {
       invalidationResults match {
         case None =>
           val early = incHandler.previousAnalysisPruned
@@ -1004,7 +1007,7 @@ private final class AnalysisCallback(
     // 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.
     incHandlerOpt foreach { incHandler =>
-      if (options.pipelining() && incHandler.isFullCompilation) {
+      if (earlyOutput.isDefined && incHandler.isFullCompilation) {
         val a = getAnalysis
         val CompileCycleResult(continue, invalidations, merged) =
           incHandler.mergeAndInvalidate(a, false)
@@ -1029,7 +1032,7 @@ private final class AnalysisCallback(
       // 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
-      if (options.pipelining && !continue && !lookup.shouldDoEarlyOutput(merged)) {
+      if (earlyOutput.isDefined && !continue && !lookup.shouldDoEarlyOutput(merged)) {
         writeEarlyArtifacts(merged)
       }
     }

diff --git a/internal/zinc-scripted/src/test/scala/sbt/internal/inc/IncHandler.scala b/internal/zinc-scripted/src/test/scala/sbt/internal/inc/IncHandler.scala
@@ -331,6 +331,7 @@ case class ProjectStructure(
         .withStoreApis(storeApis)
     (incO, sco)
   }
+  val exportPipelining = incOptions.pipelining
 
   def prev(useCachedAnalysis: Boolean = true) = {
     val store = if (useCachedAnalysis) cachedStore else fileStore
@@ -539,7 +540,7 @@ case class ProjectStructure(
 
     i.compilations.get(this).getOrElse {
       val notifyEarlyOutput: Promise[Boolean] = Promise[Boolean]()
-      if (incOptions.pipelining) {
+      if (exportPipelining) {
         // Initiate compilation
         val triggerDeps: Map[ProjectStructure, (Future[Analysis], Future[Boolean])] =
           Map(dependsOnRef map { dep =>
@@ -550,13 +551,14 @@ case class ProjectStructure(
         }
         // future of early outputs
         val earlyDeps: Future[Seq[Path]] = Future.traverse(dependsOnRef) { dep =>
-          triggerDeps(dep)._2 flatMap { success =>
-            if (success) {
-              Future.successful { dep.earlyOutput }
-            } else {
-              triggerDeps(dep)._1.map(_ => dep.output)
-            }
-          }
+          if (dep.exportPipelining)
+            triggerDeps(dep)._2 flatMap { success =>
+              if (success) {
+                Future.successful { dep.earlyOutput }
+              } else {
+                triggerDeps(dep)._1.map(_ => dep.output)
+              }
+            } else triggerDeps(dep)._1.map(_ => dep.output)
         }
         val futureScalaAnalysis = earlyDeps.map { internalCp =>
           blocking {
@@ -648,7 +650,8 @@ case class ProjectStructure(
       cp.toArray,
       vs.toArray,
       output,
-      Some(earlyOutput),
+      if (exportPipelining) Some(earlyOutput)
+      else None,
       scalacOptions,
       javacOptions = Array(),
       maxErrors,
@@ -761,12 +764,11 @@ case class ProjectStructure(
       else opts.withRecompileAllFraction(1.0)
     }
     val scalacOptions: List[String] =
-      (Option(map.get("scalac.options")) match {
-        case Some(x) => List(x)
-        case _ if incOptions.pipelining =>
-          List("-Ypickle-java", "-Ypickle-write", earlyOutput.toString)
-        case _ => Nil
-      }).flatMap(_.toString.split(" +").toList)
+      Option(map.get("scalac.options")).toList
+        .flatMap(_.toString.split(" +").toList) ++
+        // for now assume export pipelining for all pipelining subprojects
+        (if (incOptions.pipelining) List("-Ypickle-java", "-Ypickle-write", earlyOutput.toString)
+         else Nil)
     (incOptions, scalacOptions.toArray)
   }
 

diff --git a/zinc/src/main/scala/sbt/internal/inc/IncrementalCompilerImpl.scala b/zinc/src/main/scala/sbt/internal/inc/IncrementalCompilerImpl.scala
@@ -470,7 +470,7 @@ class IncrementalCompilerImpl extends IncrementalCompiler {
         extra
       )
       def prevResult = CompileResult.of(prev, config.currentSetup, false)
-      if (skip && !incrementalOptions.pipelining) prevResult
+      if (skip && earlyOutput.isEmpty) prevResult
       else if (recompileAllJava) {
         if (javaSrcs.isEmpty) prevResult
         else {

diff --git a/zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala b/zinc/src/main/scala/sbt/internal/inc/MixedAnalyzingCompiler.scala
@@ -134,11 +134,11 @@ final class MixedAnalyzingCompiler(
     val incSrc = config.sources.filter(include)
     val (javaSrcs, scalaSrcs) = incSrc.partition(MixedAnalyzingCompiler.javaOnly)
     logInputs(log, javaSrcs.size, scalaSrcs.size, outputDirs)
-    val isPickleJava = config.currentSetup.order == Mixed && config.incOptions.pipelining && javaSrcs.nonEmpty
+    val pickleJava = Incremental.isPickleJava(config.currentSetup.options.scalacOptions)
 
     // Compile Scala sources.
     def compileScala(): Unit =
-      if (scalaSrcs.nonEmpty || isPickleJava) {
+      if (scalaSrcs.nonEmpty || pickleJava) {
         val pickleJarPair = callback.getPickleJarPair.toOption.map(t2 => (t2.get1, t2.get2))
         val scalacOpts = pickleJarPair match {
           case Some((originalJar, updatesJar)) =>
@@ -159,7 +159,7 @@ final class MixedAnalyzingCompiler(
             config.converter.toVirtualFile(x.toAbsolutePath)
           }) ++ absClasspath.toVector
           val cp =
-            if (scalaSrcs.isEmpty && isPickleJava) {
+            if (scalaSrcs.isEmpty && pickleJava) {
               // we are invoking Scala compiler just for the sake of generating pickles for Java, which
               // means that the classpath would not contain scala-library jar from the build tool.
               // to work around this, we inject the scala-library into the classpath
@@ -251,7 +251,7 @@ final class MixedAnalyzingCompiler(
     val combined = scalaMsg ++ javaMsg
     if (combined.nonEmpty) {
       val targets = outputDirs.map(_.toAbsolutePath).mkString(",")
-      log.info(combined.mkString("Compiling ", " and ", s" to $targets ..."))
+      log.info(combined.mkString("compiling ", " and ", s" to $targets ..."))
     }
   }
 }

diff --git a/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/build.json b/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/build.json
@@ -0,0 +1,15 @@
+{
+  "projects": [
+    {
+      "name": "use",
+      "dependsOn": [
+        "dep"
+      ],
+      "scalaVersion": "2.13.3"
+    },
+    {
+      "name": "dep",
+      "scalaVersion": "2.13.3"
+    }
+  ]
+}
diff --git a/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/changes/B2.scala b/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/changes/B2.scala
@@ -0,0 +1,6 @@
+package example
+
+object B {
+  val y = A.x
+  val z = 1
+}
diff --git a/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/changes/Break.scala b/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/changes/Break.scala
@@ -0,0 +1,3 @@
+package example
+
+object Break
diff --git a/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/dep/A.scala b/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/dep/A.scala
@@ -0,0 +1,5 @@
+package example
+
+object A {
+  val x = 3
+}
diff --git a/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/dep/incOptions.properties b/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/dep/incOptions.properties
@@ -0,0 +1 @@
+pipelining = false
diff --git a/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/test b/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/test
@@ -0,0 +1,10 @@
+# done this way because last modified times often have ~1s resolution
+> use/compile
+
+# This tests no-op compilation of dep
+$ copy-file changes/B2.scala use/B.scala
+> use/compile
+
+$ copy-file changes/Break.scala dep/A.scala
+-> use/compile
+
diff --git a/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/use/B.scala b/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/use/B.scala
@@ -0,0 +1,5 @@
+package example
+
+object B {
+  val y = A.x
+}
diff --git a/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/use/incOptions.properties b/zinc/src/sbt-test/source-dependencies/subproject-pipelining-optout/use/incOptions.properties
@@ -0,0 +1 @@
+pipelining = true
diff --git a/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/incOptions.properties b/zinc/src/sbt-test/source-dependencies/trait-trait-211/mirtest/incOptions.properties
@@ -1,2 +1,3 @@
+pipelining = false
 relationsDebug = true
 scalac.options = -deprecation