Upgrade zinc's sbt dependency to 1.0.0: JVM portion

This review is based off Stu's unsubmitted previous effort: https://rbcommons.com/s/twitter/r/3658/

The python portion is in https://rbcommons.com/s/twitter/r/4342/

It has everything from rb/3658:

* Update for new sbt/zinc repo, that also includes a few fixes for us:
  ** empty analysis sbt/zinc#144
  ** class file analysis trigger static initializers run sbt/zinc#151
* Exclude the existing io/logging deps, and re-include them explicitly as forced (with the appropriate classifiers)
* Update all imports/dependencies to new locations
* Require an explicit -cache-dir (which will be placed inside the pants cache directory in the review that incorporates this * version)
* Remove the -name-hashing flag, as name hashing is required for the now-default class-based dependency tracking.
* Rename sbt-interface to compiler-interface, and compiler-interface to compiler-bridge. Confused yet?

Plus a few other changes:

* Analysis format recently changed to a zip with two entries. This review keeps the plain txt format pants parser uses. Long term we probably should switch to some internal format that's more stable and lighter weight.
* An option to turn off zinc provided file manager, see sbt/zinc#185
* re-enable the optimization to check class existence from analysis (Significant performance impact, esp. for incremental compile)

Testing Done:
https://travis-ci.org/peiyuwang/pants/builds/172980195
https://travis-ci.org/peiyuwang/pants/builds/174415697

Bugs closed: 3962, 4021

Reviewed at https://rbcommons.com/s/twitter/r/4340/

3rdparty/BUILD

@@ -33,7 +33,8 @@ jar_library(name='jansi',
 jar_library(name='jsr305',
             jars=[
               jar('com.google.code.findbugs', 'jsr305', '3.0.0'),
-            ])
+            ],
+            scope='compile')
 
 jar_library(name='checkstyle',
             jars = [

3rdparty/jvm/org/scala-sbt/BUILD

@@ -0,0 +1,28 @@
+jar_library(
+  name='zinc',
+  jars=[
+    scala_jar(org='org.scala-sbt', name='zinc', rev='1.0.0-X5',
+              excludes=[
+                exclude(org='org.scala-sbt', name='io_2.10'),
+                exclude(org='org.scala-sbt', name='util-logging_2.10'),
+              ]),
+  ],
+)
+
+jar_library(
+  name='util-logging',
+  jars=[
+    # TODO: `zinc` only declares a dep on the `tests` classifier for
+    # util-logging for some reason. We redefine the dep here to get the full package.
+    scala_jar(org='org.scala-sbt', name='util-logging', rev='0.1.0-M13', force=True),
+  ],
+)
+
+jar_library(
+  name='io',
+  jars=[
+    # TODO: `zinc` only declares a dep on the `tests` classifier for
+    # io. We redefine the dep here to get the full package.
+    scala_jar(org='org.scala-sbt', name='io', rev='1.0.0-M6', force=True),
+  ],
+)

contrib/scrooge/src/python/pants/contrib/scrooge/tasks/thrift_linter.py

@@ -129,4 +129,4 @@ def execute(self):
             vt.update()
 
         if errors:
-          raise TaskError('\n'.join(errors))
+          raise TaskError('\n'.join(errors))

src/python/pants/bin/target_roots.py

@@ -7,11 +7,11 @@
 
 import logging
 
-from pants.base.specs import SingleAddress
 from twitter.common.collections import OrderedSet
 
 from pants.base.build_environment import get_buildroot
 from pants.base.cmd_line_spec_parser import CmdLineSpecParser
+from pants.base.specs import SingleAddress
 from pants.bin.options_initializer import OptionsInitializer
 from pants.option.options_bootstrapper import OptionsBootstrapper
 from pants.scm.subsystems.changed import ChangedRequest

src/scala/org/pantsbuild/zinc/AnalysisMap.scala

@@ -5,17 +5,18 @@
 
 package org.pantsbuild.zinc
 
-import java.io.{
-  File,
-  IOException
-}
+import java.io.{File, IOException}
 import java.nio.file.Files
 import java.nio.file.StandardCopyOption
-
-import sbt.{CompileSetup, Logger}
-import sbt.inc.{Analysis, AnalysisStore, FileBasedStore, Locate}
+import xsbti.Maybe
+import xsbti.compile.{CompileAnalysis, DefinesClass, MiniSetup, PerClasspathEntryLookup}
+import sbt.internal.inc.{Analysis, AnalysisStore, CompanionsStore, Locate, TextAnalysisFormat}
+import sbt.io.{IO, Using}
+import sbt.util.Logger
+import sbt.util.Logger.o2m
 import org.pantsbuild.zinc.cache.{Cache, FileFPrint}
 import org.pantsbuild.zinc.cache.Cache.Implicits
+import xsbti.api.Companions
 
 /**
  * A facade around the analysis cache to:
@@ -33,32 +34,54 @@ case class AnalysisMap private[AnalysisMap] (
   // log
   log: Logger
 ) {
-  /**
-   * An implementation of definesClass that will use analysis for an input directory to determine
-   * whether it defines a particular class.
-   *
-   * TODO: This optimization is unnecessary for jars on the classpath, which are already indexed.
-   * Can remove after the sbt jar output patch lands.
-   */
-  def definesClass(classpathEntry: File): String => Boolean =
-    getAnalysis(classpathEntry).map { analysis =>
-      log.debug(s"Hit analysis cache for class definitions with ${classpathEntry}")
-      // strongly hold the classNames, and transform them to ensure that they are unlinked from
-      // the remainder of the analysis
-      analysis.relations.classes.reverseMap.keys.toList.toSet
-    }.map { classes =>
-      (s: String) => classes(s)
-    }.getOrElse {
-      // no analysis: return a function that will scan instead
-      Locate.definesClass(classpathEntry)
+
+  def getPCELookup = new PerClasspathEntryLookup {
+    /**
+     * Gets analysis for a classpath entry (if it exists) by translating its path to a potential
+     * cache location and then checking the cache.
+     */
+    def analysis(classpathEntry: File): Maybe[CompileAnalysis] =
+      o2m(analysisLocations.get(classpathEntry).flatMap(AnalysisMap.get))
+
+    /**
+     * An implementation of definesClass that will use analysis for an input directory to determine
+     * whether it defines a particular class.
+     *
+     * TODO: This optimization is unnecessary for jars on the classpath, which are already indexed.
+     * Can remove after the sbt jar output patch lands.
+     */
+    def definesClass(classpathEntry: File): DefinesClass = {
+      getAnalysis(classpathEntry).map { analysis =>
+        log.debug(s"Hit analysis cache for class definitions with ${classpathEntry}")
+        // strongly hold the classNames, and transform them to ensure that they are unlinked from
+        // the remainder of the analysis
+        val classNames = analysis.asInstanceOf[Analysis].relations.srcProd.reverseMap.keys.toList.toSet.map(
+          (f: File) => filePathToClassName(f))
+        new ClassNamesDefinesClass(classNames)
+      }.getOrElse {
+        // no analysis: return a function that will scan instead
+        Locate.definesClass(classpathEntry)
+      }
     }
 
-  /**
-   * Gets analysis for a classpath entry (if it exists) by translating its path to a potential
-   * cache location and then checking the cache.
-   */
-  def getAnalysis(classpathEntry: File): Option[Analysis] =
-    analysisLocations.get(classpathEntry).flatMap(AnalysisMap.get)
+    private class ClassNamesDefinesClass(classes: Set[String]) extends DefinesClass {
+      override def apply(className: String): Boolean = classes(className)
+    }
+
+    private def filePathToClassName(file: File): String = {
+      // Extract className from path, for example:
+      //   .../.pants.d/compile/zinc/.../current/classes/org/pantsbuild/example/hello/exe/Exe.class
+      //   => org.pantsbuild.example.hello.exe.Exe
+      file.getAbsolutePath.split("current/classes")(1).drop(1).replace(".class", "").replaceAll("/", ".")
+    }
+
+    /**
+     * Gets analysis for a classpath entry (if it exists) by translating its path to a potential
+     * cache location and then checking the cache.
+     */
+    def getAnalysis(classpathEntry: File): Option[CompileAnalysis] =
+       analysisLocations.get(classpathEntry).flatMap(AnalysisMap.get)
+  }
 }
 
 object AnalysisMap {
@@ -67,7 +90,7 @@ object AnalysisMap {
    * know if, on a cache miss, the underlying file will yield a valid Analysis.
    */
   private val analysisCache =
-    Cache[FileFPrint, Option[(Analysis, CompileSetup)]](Setup.Defaults.analysisCacheLimit)
+    Cache[FileFPrint, Option[(CompileAnalysis, MiniSetup)]](Setup.Defaults.analysisCacheLimit)
 
   def create(
     // a map of classpath entries to cache file locations, excluding the current compile destination
@@ -83,13 +106,13 @@ object AnalysisMap {
       log
     )
 
-  private def get(cacheFPrint: FileFPrint): Option[Analysis] =
+  private def get(cacheFPrint: FileFPrint): Option[CompileAnalysis] =
     analysisCache.getOrElseUpdate(cacheFPrint) {
       // re-fingerprint the file on miss, to ensure that analysis hasn't changed since we started
       if (!FileFPrint.fprint(cacheFPrint.file).exists(_ == cacheFPrint)) {
         throw new IOException(s"Analysis at $cacheFPrint has changed since startup!")
       }
-      FileBasedStore(cacheFPrint.file).get
+      AnalysisStore.cached(SafeFileBasedStore(cacheFPrint.file)).get()
     }.map(_._1)
 
   /**
@@ -99,11 +122,11 @@ object AnalysisMap {
     val fileStore = AnalysisStore.cached(SafeFileBasedStore(cacheFile))
 
     val fprintStore = new AnalysisStore {
-      def set(analysis: Analysis, setup: CompileSetup) {
+      def set(analysis: CompileAnalysis, setup: MiniSetup) {
         fileStore.set(analysis, setup)
         FileFPrint.fprint(cacheFile) foreach { analysisCache.put(_, Some((analysis, setup))) }
       }
-      def get(): Option[(Analysis, CompileSetup)] = {
+      def get(): Option[(CompileAnalysis, MiniSetup)] = {
         FileFPrint.fprint(cacheFile) flatMap { fprint =>
           analysisCache.getOrElseUpdate(fprint) {
             fileStore.get
@@ -124,14 +147,40 @@ object AnalysisMap {
  */
 object SafeFileBasedStore {
   def apply(file: File): AnalysisStore = new AnalysisStore {
-    def set(analysis: Analysis, setup: CompileSetup) {
+    override def set(analysis: CompileAnalysis, setup: MiniSetup): Unit = {
       val tmpAnalysisFile = File.createTempFile(file.getName, ".tmp")
-      val analysisStore = FileBasedStore(tmpAnalysisFile)
+      val analysisStore = PlainTextFileBasedStore(tmpAnalysisFile)
       analysisStore.set(analysis, setup)
       Files.move(tmpAnalysisFile.toPath, file.toPath, StandardCopyOption.REPLACE_EXISTING)
     }
 
-    def get(): Option[(Analysis, CompileSetup)] =
-      FileBasedStore(file).get
+    override def get(): Option[(CompileAnalysis, MiniSetup)] =
+      PlainTextFileBasedStore(file).get
+  }
+}
+
+/**
+ * Zinc 1.0 changes its analysis file format to zip, and split into two files.
+ * The following provides a plain text adaptor for pants parser. Long term though,
+ * we should consider define an internal analysis format that's 1) more stable
+ * 2) better performance because we can pick and choose only the fields we care about
+ * - string processing in rebase can be slow for example.
+ * https://github.com/pantsbuild/pants/issues/4039
+ */
+object PlainTextFileBasedStore {
+  def apply(file: File): AnalysisStore = new AnalysisStore {
+    override def set(analysis: CompileAnalysis, setup: MiniSetup): Unit = {
+      Using.fileWriter(IO.utf8)(file) { writer => TextAnalysisFormat.write(writer, analysis, setup) }
+    }
+
+    override def get(): Option[(CompileAnalysis, MiniSetup)] =
+      try { Some(getUncaught()) } catch { case _: Exception => None }
+    def getUncaught(): (CompileAnalysis, MiniSetup) =
+      Using.fileReader(IO.utf8)(file) { reader => TextAnalysisFormat.read(reader, noopCompanionsStore) }
   }
-}
+
+  val noopCompanionsStore = new CompanionsStore {
+    override def get(): Option[(Map[String, Companions], Map[String, Companions])] = Some(getUncaught())
+    override def getUncaught(): (Map[String, Companions], Map[String, Companions]) = (Map(), Map())
+  }
+}

src/scala/org/pantsbuild/zinc/BUILD

@@ -6,12 +6,13 @@ scala_library(
     publication_metadata=pants_library('The SBT incremental compiler for nailgun')
   ),
   dependencies=[
-    '3rdparty/jvm/com/typesafe/sbt:incremental-compiler',
+    '3rdparty/jvm/org/scala-sbt:io',
+    '3rdparty/jvm/org/scala-sbt:util-logging',
+    '3rdparty/jvm/org/scala-sbt:zinc',
     '3rdparty:guava',
+    '3rdparty:jsr305',
     'src/scala/org/pantsbuild/zinc/cache',
     'src/scala/org/pantsbuild/zinc/logging',
-    'src/scala/sbt/compiler/javac',
-    'src/scala/sbt/inc',
   ],
   strict_deps=True,
   platform='java7',