Use sbt for PR validation

szeiger · szeiger · commit f2d0f1e85d8b · 2016-06-14T15:54:01.000+02:00
- Support directories in `-doc-external-doc`: It is documented as accepting a “classpath_entry_path” for the keys but this only worked for JARs and not for individual class files. When checking for external-doc mappings for a Symbol, we now find the root directory relative to a class file instead of using the full class file path. The corresponding tests for SI-191 and SI8557 are also fixed to support individual class files instead of JARs in partest. This is required for the sbt build which runs partest on “quick” instead of “pack”. - Fix version and repository handling for bootstrapping. The bootstrap `scalaInstance` can now be resolved from any repository added to the project (not just the bootstrap repositories) by using a different workaround for sbt/sbt#1872. - Workaround for sbt/sbt#2640 (putting the wrong `scalaInstance` on partest’s classpath). The required `ScalaInstance` constructor is deprecated, so we have to disable deprecation warnings and fatal warnings until there is a better fix. - Add MiMa to the sbt build (port of the old `test.bc` ant task). The sbt-mima plugin doesn’t have all the features we need, so we do it manually in a similar way to what the plugin does. Checks are done in both directions for the `library` and `compiler` projects. The base version has to be set in `build.sbt`. When set to `None`, MiMa checks are skipped. MiMa checks are run sequentially to avoid spurious errors (see lightbend-labs/mima#115). - Port the OSGi tests to the sbt build. The set of JARs that gets copied into build/osgi as bundles is a bit different from the ant build. We omit the source JARs but add additional modules that are part of the Scala distribution, which seems more correct. - Get rid up `pull-binary-libs.sh` for the sbt build. Add artifacts are resolved from the special bootstrap repository through Ivy. The special `code.jar` and `instrumented.jar` artifacts are copied to the location where partest expects them (because these paths are hardcoded in partest). Other extra JARs for partest in `test/files/lib` are referenced directly from the Ivy cache. - Move common settings that should be available with unqualified names in local `.sbt` files and on the command line into an auto-plugin. - Add an `antStyle` setting to sbt to allow users to easily enable ant-style incremental compilation instead of sbt’s standard name hashing with `set antStyle := true`. - Disable verbose `info`-level logging during sbt startup for both, `validate/test` and `validate/publish-core` jobs. Update logging is no longer disabled when running locally (where it is useful and doesn’t generate excessive output). - Pass optimization flags for scalac down to partest, using the new partest version 1.0.15\6. - Call the new sbt-based PR validation from `scripts/jobs/validate/test`. - Disable the tests `run/t7843-jsr223-service` and `run/t7933` from scala#4959 for now. We need to set up a new test project (either partest or junit) that can run them on a packaged version of Scala, or possibly move them into a separate project that would naturally run from a packaged Scala as part of the community build.
diff --git a/build.sbt b/build.sbt
diff --git a/project/BuildSettings.scala b/project/BuildSettings.scala
@@ -0,0 +1,11 @@
+import sbt._
+
+/** This object defines keys that should be visible with an unqualified name in all .sbt files and the command line */
+object BuildSettings extends AutoPlugin {
+  object autoImport {
+    lazy val antStyle = settingKey[Boolean]("Use ant-style incremental builds instead of name-hashing")
+    lazy val baseVersion = settingKey[String]("The base version number from which all others are derived")
+    lazy val baseVersionSuffix = settingKey[String]("Identifies the kind of version to build")
+    lazy val mimaReferenceVersion = settingKey[Option[String]]("Scala version number to run MiMa against")
+  }
+}
diff --git a/project/MiMa.scala b/project/MiMa.scala
@@ -0,0 +1,95 @@
+// It would be nice to use sbt-mima-plugin here, but the plugin is missing
+// at least two features we need:
+// * ability to run MiMa twice, swapping `curr` and `prev`, to detect
+//   both forwards and backwards incompatibilities (possibly fixed as of
+//   https://github.com/typesafehub/migration-manager/commit/2844ffa48b6d2255aa64bd687703aec21dadd55e)
+// * ability to pass a filter file (https://github.com/typesafehub/migration-manager/issues/102)
+// So we invoke the MiMa CLI directly; it's also what the Ant build did.
+
+import sbt._
+import sbt.Keys._
+import BuildSettings.autoImport._
+
+object MiMa {
+  lazy val mima =
+    taskKey[Unit]("run Migration Manager to detect binary incompatibilities")
+
+  lazy val settings =
+    Seq(
+      mima := {
+        val log = streams.value.log
+        mimaReferenceVersion.value.fold {
+          log.info(s"No reference version defined - skipping binary compatibility checks")
+        } { refVersion =>
+          def runOnce(prev: java.io.File, curr: java.io.File, isForward: Boolean): Unit = {
+            val direction = if (isForward) "forward" else "backward"
+            log.info(s"Checking $direction binary compatibility")
+            log.debug(s"prev = $prev, curr = $curr")
+            runMima(
+              prev = if (isForward) curr else prev,
+              curr = if (isForward) prev else curr,
+              // TODO: it would be nicer if each subproject had its own whitelist, but for now
+              // for compatibility with how Ant did things, there's just one at the root.
+              // once Ant is gone we'd be free to split it up.
+              filter = (baseDirectory in ThisBuild).value / s"bincompat-$direction.whitelist.conf",
+              log)
+          }
+          val artifact =
+            getPreviousArtifact(
+              "org.scala-lang" % s"${name.value}" % refVersion,
+              ivySbt.value, streams.value)
+          for (isForward <- Seq(false, true))
+            runOnce(artifact, (packageBin in Compile).value, isForward)
+        }
+      }
+    )
+
+  def runMima(prev: java.io.File, curr: java.io.File, filter: java.io.File, log: Logger): Unit = {
+    val args = Array(
+      "--prev", prev.getAbsolutePath,
+      "--curr", curr.getAbsolutePath,
+      "--filters", filter.getAbsolutePath,
+      "--generate-filters"
+    )
+    val exitCode = TrapExit(com.typesafe.tools.mima.cli.Main.main(args), log)
+    if (exitCode != 0)
+      throw new RuntimeException(s"MiMa failed with exit code $exitCode")
+  }
+
+  // cribbed from https://github.com/typesafehub/migration-manager/blob/master/sbtplugin/src/main/scala/com/typesafe/tools/mima/plugin/SbtMima.scala
+  def getPreviousArtifact(m: ModuleID, ivy: IvySbt, s: TaskStreams): File = {
+    val moduleSettings = InlineConfiguration(
+      "dummy" % "test" % "version",
+      ModuleInfo("dummy-test-project-for-resolving"),
+      dependencies = Seq(m))
+    val module = new ivy.Module(moduleSettings)
+    val report = Deprecated.Inner.ivyUpdate(ivy)(module, s)
+    val optFile = (for {
+      config <- report.configurations
+      module <- config.modules
+      (artifact, file) <- module.artifacts
+      // TODO - Hardcode this?
+      if artifact.name == m.name
+    } yield file).headOption
+    optFile getOrElse sys.error("Could not resolve previous artifact: " + m)
+  }
+
+}
+
+// use the SI-7934 workaround to silence a deprecation warning on an sbt API
+// we have no choice but to call.  on the lack of any suitable alternative,
+// see https://gitter.im/sbt/sbt-dev?at=5616e2681b0e279854bd74a4 :
+// "it's my intention to eventually come up with a public API" says Eugene Y
+object Deprecated {
+  @deprecated("", "") class Inner {
+    def ivyUpdate(ivy: IvySbt)(module: ivy.Module, s: TaskStreams) =
+      IvyActions.update(
+        module,
+        new UpdateConfiguration(
+          retrieve = None,
+          missingOk = false,
+          logging = UpdateLogging.DownloadOnly),
+        s.log)
+  }
+  object Inner extends Inner
+}
diff --git a/project/Osgi.scala b/project/Osgi.scala
@@ -28,8 +28,8 @@ object Osgi {
         "Bundle-Name" -> bundleName.value,
         "Bundle-SymbolicName" -> bundleSymbolicName.value,
         "ver" -> v,
-        "Export-Package" -> ("*;version=${ver}"),
-        "Import-Package" -> ("scala.*;version=\"${range;[==,=+);${ver}}\",*"),
+        "Export-Package" -> "*;version=${ver}",
+        "Import-Package" -> "scala.*;version=\"${range;[==,=+);${ver}}\",*",
         "Bundle-Version" -> v,
         "Bundle-RequiredExecutionEnvironment" -> "JavaSE-1.6, JavaSE-1.7",
         "-eclipse" -> "false"
diff --git a/project/Quiet.scala b/project/Quiet.scala
@@ -28,6 +28,4 @@ object Quiet {
       case x => x
     }
   }
-
-  def silenceIvyUpdateInfoLogging = logLevel in update := Level.Warn
 }
diff --git a/project/ScalaTool.scala b/project/ScalaTool.scala
@@ -27,12 +27,12 @@ case class ScalaTool(mainClass: String,
       } else classpath.mkString(":").replace('\\', '/').replaceAll(varRegex, """\${$1}""")
 
     val variables = Map(
-      ("@@"           -> "@"), // for backwards compatibility
-      ("@class@"      -> mainClass),
-      ("@properties@" -> (properties map { case (k, v) => s"""-D$k="$v""""} mkString " ")),
-      ("@javaflags@"  -> javaOpts),
-      ("@toolflags@"  -> toolFlags),
-      ("@classpath@"  -> platformClasspath)
+      "@@"           -> "@", // for backwards compatibility
+      "@class@"      -> mainClass,
+      "@properties@" -> (properties map { case (k, v) => s"""-D$k="$v""""} mkString " "),
+      "@javaflags@"  -> javaOpts,
+      "@toolflags@"  -> toolFlags,
+      "@classpath@"  -> platformClasspath
     )
 
     val (from, to) = variables.unzip
diff --git a/project/ScriptCommands.scala b/project/ScriptCommands.scala
@@ -1,19 +1,32 @@
 import sbt._
 import Keys._
-import complete.DefaultParsers._
+import BuildSettings.autoImport._
 
 /** Custom commands for use by the Jenkins scripts. This keeps the surface area and call syntax small. */
 object ScriptCommands {
-  def all = Seq(setupPublishCore)
+  def all = Seq(setupPublishCore, setupValidateTest)
 
   /** Set up the environment for `validate/publish-core`. The argument is the Artifactory snapshot repository URL. */
   def setupPublishCore = Command.single("setupPublishCore") { case (state, url) =>
       Project.extract(state).append(Seq(
-        VersionUtil.baseVersionSuffix in Global := "SHA-SNAPSHOT",
+        baseVersionSuffix in Global := "SHA-SNAPSHOT",
         // Append build.timestamp to Artifactory URL to get consistent build numbers (see https://github.com/sbt/sbt/issues/2088):
         publishTo in Global := Some("scala-pr" at url.replaceAll("/$", "") + ";build.timestamp=" + System.currentTimeMillis),
         publishArtifact in (Compile, packageDoc) in ThisBuild := false,
-        scalacOptions in Compile in ThisBuild += "-optimise"
+        scalacOptions in Compile in ThisBuild += "-optimise",
+        logLevel in ThisBuild := Level.Info,
+        logLevel in update in ThisBuild := Level.Warn
       ), state)
     }
+
+  /** Set up the environment for `validate/test`. The argument is the Artifactory snapshot repository URL. */
+  def setupValidateTest = Command.single("setupValidateTest") { case (state, url) =>
+    //TODO When ant is gone, pass starr version as an argument to this command instead of using version.properties
+    Project.extract(state).append(Seq(
+      resolvers in Global += "scala-pr" at url,
+      scalacOptions in Compile in ThisBuild += "-optimise",
+      logLevel in ThisBuild := Level.Info,
+      logLevel in update in ThisBuild := Level.Warn
+    ), state)
+  }
 }
diff --git a/project/VersionUtil.scala b/project/VersionUtil.scala
@@ -1,12 +1,11 @@
 import sbt._
 import Keys._
 import java.util.Properties
-import java.io.FileInputStream
+import java.io.{File, FileInputStream}
 import scala.collection.JavaConverters._
+import BuildSettings.autoImport._
 
 object VersionUtil {
-  lazy val baseVersion = settingKey[String]("The base version number from which all others are derived")
-  lazy val baseVersionSuffix = settingKey[String]("Identifies the kind of version to build")
   lazy val copyrightString = settingKey[String]("Copyright string.")
   lazy val versionProperties = settingKey[Versions]("Version properties.")
   lazy val generateVersionPropertiesFile = taskKey[File]("Generating version properties file.")
@@ -123,4 +122,33 @@ object VersionUtil {
   /** Get a subproject version number from `versionProps` */
   def versionNumber(name: String): String =
     versionProps(s"$name.version.number")
+
+  /** Build a dependency to a Scala module with the given group and artifact ID */
+  def scalaDep(group: String, artifact: String, versionProp: String = null, scope: String = null, compatibility: String = "binary") = {
+    val vp = if(versionProp eq null) artifact else versionProp
+    val m = group % (artifact + "_" + versionProps(s"scala.$compatibility.version")) % versionNumber(vp)
+    val m2 = if(scope eq null) m else m % scope
+    // exclusion of the scala-library transitive dependency avoids eviction warnings during `update`:
+    m2.exclude("org.scala-lang", "*")
+  }
+
+  private def bootstrapOrganization(path: String) =
+    "org.scala-lang.scala-sha-bootstrap." + path.replace('/', '.')
+
+  /** Build a dependency to a JAR file in the bootstrap repository */
+  def bootstrapDep(baseDir: File, path: String, libName: String): ModuleID = {
+    val sha = IO.read(baseDir / path / s"$libName.jar.desired.sha1").split(' ')(0)
+    bootstrapOrganization(path) % libName % sha from
+      s"https://dl.bintray.com/typesafe/scala-sha-bootstrap/org/scala-lang/bootstrap/$sha/$path/$libName.jar"
+  }
+
+  /** Copy a boostrap dependency JAR that is on the classpath to a file */
+  def copyBootstrapJar(cp: Seq[Attributed[File]], baseDir: File, path: String, libName: String): Unit = {
+    val org = bootstrapOrganization(path)
+    val resolved = cp.find { a =>
+      val mod = a.get(moduleID.key)
+      mod.map(_.organization) == Some(org) && mod.map(_.name) == Some(libName)
+    }.map(_.data).get
+    IO.copyFile(resolved, baseDir / path / s"$libName.jar")
+  }
 }
diff --git a/project/plugins.sbt b/project/plugins.sbt
@@ -1,3 +1,6 @@
+scalacOptions ++= Seq("-unchecked", "-feature", /*"-deprecation",*/
+  "-Xlint" /*, "-Xfatal-warnings"*/)
+
 libraryDependencies += "org.apache.commons" % "commons-lang3" % "3.3.2"
 
 libraryDependencies += "org.pantsbuild" % "jarjar" % "1.6.3"
@@ -15,3 +18,5 @@ buildClasspath := (externalDependencyClasspath in Compile).value.map(_.data).mkS
 buildInfoKeys := Seq[BuildInfoKey](buildClasspath)
 
 buildInfoPackage := "scalabuild"
+
+libraryDependencies += "com.typesafe" %% "mima-reporter" % "0.1.8"
diff --git a/scripts/jobs/validate/publish-core b/scripts/jobs/validate/publish-core
@@ -16,7 +16,7 @@ case $prDryRun in
     ;;
   *)
     echo ">>> Getting Scala version number."
-    $SBT_CMD "setupPublishCore $prRepoUrl" generateBuildCharacterPropertiesFile
+    $SBT_CMD --warn "setupPublishCore $prRepoUrl" generateBuildCharacterPropertiesFile
     parseScalaProperties buildcharacter.properties # produce maven_version_number
 
     echo ">>> Checking availability of Scala ${maven_version_number} in $prRepoUrl."
@@ -27,7 +27,7 @@ case $prDryRun in
     if $libraryAvailable && $reflectAvailable && $compilerAvailable; then
       echo "Scala core already built!"
     else
-      $SBT_CMD "setupPublishCore $prRepoUrl" $antBuildArgs publish
+      $SBT_CMD --warn "setupPublishCore $prRepoUrl" publish
     fi
 
     mv buildcharacter.properties jenkins.properties # parsed by the jenkins job
diff --git a/scripts/jobs/validate/test b/scripts/jobs/validate/test
@@ -1,17 +1,36 @@
-#!/bin/bash -e
+#!/bin/bash -e -v -x
+
+baseDir=${WORKSPACE-`pwd`}
+scriptsDir="$baseDir/scripts"
+. $scriptsDir/common
 
 case $prDryRun in
+
   yep)
     echo "DRY RUN"
     ;;
+
   *)
-    ./pull-binary-libs.sh
 
     # build quick using STARR built upstream, as specified by scalaVersion
-    # (in that sense it's locker, since it was built with starr by that upstream job)
-    ant -Dstarr.version=$scalaVersion \
-       -Dscalac.args.optimise=-optimise \
-       -Dlocker.skip=1 -Dextra.repo.url=$prRepoUrl \
-       $testExtraArgs ${testTarget-test.core docs.done}
+    # (in that sense it's locker, since it was built with starr by that upstream job);
+    # and run JUnit tests, partest, OSGi tests, MiMa and scaladoc
+    $SBT_CMD \
+       -Dstarr.version=$scalaVersion \
+       --warn \
+       "setupValidateTest $prRepoUrl" \
+       $testExtraArgs \
+       "test" \
+       "partest run pos neg jvm" \
+       "partest res scalap specialized scalacheck" \
+       "partest instrumented presentation" \
+       "partest --srcpath scaladoc" \
+       osgiTestFelix/test \
+       osgiTestEclipse/test \
+       library/mima \
+       reflect/mima \
+       doc
+
     ;;
-esac
+
+esac
diff --git a/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala b/src/scaladoc/scala/tools/nsc/doc/model/MemberLookup.scala
@@ -44,8 +44,22 @@ trait MemberLookup extends base.MemberLookupBase {
         /* Get package object which has associatedFile ne null */
         sym.info.member(newTermName("package"))
       else sym
-    Option(sym1.associatedFile) flatMap (_.underlyingSource) flatMap { src =>
-      val path = src.canonicalPath
+    def classpathEntryFor(s: Symbol): Option[String] = {
+      Option(s.associatedFile).flatMap(_.underlyingSource).map { src =>
+        val path = src.canonicalPath
+        if(path.endsWith(".class")) { // Individual class file -> Classpath entry is root dir
+          var nesting = s.ownerChain.count(_.hasPackageFlag)
+          if(nesting > 0) {
+            val p = 0.until(nesting).foldLeft(src) {
+              case (null, _) => null
+              case (f, _) => f.container
+            }
+            if(p eq null) path else p.canonicalPath
+          } else path
+        } else path // JAR file (and fallback option)
+      }
+    }
+    classpathEntryFor(sym1) flatMap { path =>
       settings.extUrlMapping get path map { url =>
         LinkToExternal(name, url + "#" + name)
       }
diff --git a/test/disabled/run/t7843-jsr223-service.check b/test/disabled/run/t7843-jsr223-service.check
diff --git a/test/disabled/run/t7843-jsr223-service.scala b/test/disabled/run/t7843-jsr223-service.scala
diff --git a/test/disabled/run/t7933.check b/test/disabled/run/t7933.check
diff --git a/test/disabled/run/t7933.scala b/test/disabled/run/t7933.scala
diff --git a/test/osgi/src/logback.xml b/test/osgi/src/logback.xml
@@ -0,0 +1,10 @@
+<configuration>
+    <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
+        <encoder>
+            <pattern>*** \(%logger{30}\)%green(%X{debugId}) %msg%n</pattern>
+        </encoder>
+    </appender>
+    <root level="${log.root:-warn}">
+        <appender-ref ref="STDOUT" />
+    </root>
+</configuration>
diff --git a/test/scaladoc/.gitignore b/test/scaladoc/.gitignore
@@ -0,0 +1,2 @@
+*.log
+*.obj/
diff --git a/test/scaladoc/run/SI-191.scala b/test/scaladoc/run/SI-191.scala
@@ -33,10 +33,14 @@ object Test extends ScaladocModelTest {
   def scalaURL = "http://bog.us"
 
   override def scaladocSettings = {
-    val scalaLibUri = getClass.getClassLoader.getResource("scala/Function1.class").getPath.split("!")(0)
-    val scalaLibPath = new URI(scalaLibUri).getPath
-    val externalArg = s"$scalaLibPath#$scalaURL"
-    "-no-link-warnings -doc-external-doc " + externalArg
+    val samplePath = getClass.getClassLoader.getResource("scala/Function1.class").getPath
+    val scalaLibPath = if(samplePath.contains("!")) { // in scala-library.jar
+      val scalaLibUri = samplePath.split("!")(0)
+      new URI(scalaLibUri).getPath
+    } else { // individual class files on disk
+      samplePath.replace('\\', '/').dropRight("scala/Function1.class".length)
+    }
+    s"-no-link-warnings -doc-external-doc $scalaLibPath#$scalaURL"
   }
 
   def testModel(rootPackage: Package) {
diff --git a/test/scaladoc/run/t8557.scala b/test/scaladoc/run/t8557.scala
@@ -1,3 +1,5 @@
+import java.net.URI
+
 import scala.tools.nsc.doc.base._
 import scala.tools.nsc.doc.model._
 import scala.tools.partest.ScaladocModelTest
@@ -15,10 +17,22 @@ object Test extends ScaladocModelTest {
       class A
     """
 
+  def scalaURL = "http://www.scala-lang.org/api/current/"
+
   // a non-canonical path to scala-library.jar should still work
-  // this is a bit fragile (depends on the current directory being the root of the repo ;
-  // ant & partest seem to do that properly)
-  def scaladocSettings = "-doc-external-doc build/pack/bin/../lib/scala-library.jar#http://www.scala-lang.org/api/current/"
+  override def scaladocSettings = {
+    val samplePath = getClass.getClassLoader.getResource("scala/Function1.class").getPath.replace('\\', '/')
+    val scalaLibPath = if(samplePath.contains("!")) { // in scala-library.jar
+      val scalaLibUri = samplePath.split("!")(0)
+      val p = new URI(scalaLibUri).getPath
+      // this is a bit fragile (depends on the scala library being in build/pack as produced by ant)
+      p.replace("/pack/lib/scala-library.jar", "/pack/bin/../lib/scala-library.jar")
+    } else { // individual class files on disk
+      val p = samplePath.dropRight("scala/Function1.class".length + 1)
+      p + "/.." + p.takeRight(p.length - p.lastIndexOf('/'))
+    }
+    s"-doc-external-doc $scalaLibPath#$scalaURL"
+  }
 
   def testModel(rootPackage: Package) = {
     // get the quick access implicit defs in scope (_package(s), _class(es), _trait(s), object(s) _method(s), _value(s))
diff --git a/versions.properties b/versions.properties

Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,4 @@ object Quiet {`
`28`	`28`	`case x => x`
`29`	`29`	`}`
`30`	`30`	`}`
`31`		`-`
`32`		`- def silenceIvyUpdateInfoLogging = logLevel in update := Level.Warn`
`33`	`31`	`}`