From 2c3f40eb02fe108bf99a139b988adbb7a6735ae0 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Mon, 2 Jun 2014 15:10:17 -0400 Subject: [PATCH 1/7] Improve test failure message for #1390. --- project/Sbt.scala | 14 +++++++------- .../test/scala/sbt/classpath/ConcurrentCache.scala | 13 ++++++++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/project/Sbt.scala b/project/Sbt.scala index f22136d94..1633d6d71 100644 --- a/project/Sbt.scala +++ b/project/Sbt.scala @@ -25,7 +25,7 @@ object Sbt extends Build { javacOptions in compile ++= Seq("-target", "6", "-source", "6", "-Xlint", "-Xlint:-serial"), incOptions := incOptions.value.withNameHashing(true), commands += Command.command("setupBuildScala211") { state => - """set scalaVersion in ThisBuild := "2.11.0" """ :: + """set scalaVersion in ThisBuild := "2.11.1" """ :: "set Util.includeTestDependencies in ThisBuild := true" :: state }, @@ -46,12 +46,12 @@ object Sbt extends Build { def lameAgregateTask(task: String): String = s"all control/$task collections/$task io/$task completion/$task" "setupBuildScala211" :: - /// First test - lameAgregateTask("test") :: - // Note: You need the sbt-pgp plugin installed to release. - lameAgregateTask("publishSigned") :: - // Now restore the defaults. - "reload" :: state + /// First test + lameAgregateTask("test") :: + // Note: You need the sbt-pgp plugin installed to release. + lameAgregateTask("publishSigned") :: + // Now restore the defaults. + "reload" :: state }, commands += Command.command("release-sbt") { state => // TODO - Any sort of validation diff --git a/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala b/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala index ab2fa9ac9..470c1c8d5 100644 --- a/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala +++ b/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala @@ -9,11 +9,20 @@ object ConcurrentCache extends Properties("ClassLoaderCache concurrent access") implicit lazy val concurrentArb: Arbitrary[Int] = Arbitrary(Gen.choose(1, 1000)) implicit lazy val filenameArb: Arbitrary[String] = Arbitrary(Gen.alphaStr) + private[this] object DifferentClassloader { + def unapply(loaders: Seq[ClassLoader]): Option[(ClassLoader, ClassLoader)] = + if (loaders.size > 1) loaders.sliding(2).find { case Seq(x, y) => x != y } map { case Seq(x, y) => (x, y) } + else None + } + property("Same class loader for same classpaths concurrently processed") = forAll { (names: List[String], concurrent: Int) => withcp(names.distinct) { files => val cache = new ClassLoaderCache(null) val loaders = (1 to concurrent).par.map(_ => cache(files)).toList - sameClassLoader(loaders) + loaders match { + case DifferentClassloader(left, right) => false :| s"$left != $right" + case _ => true :| "" + } } } @@ -25,6 +34,4 @@ object ConcurrentCache extends Properties("ClassLoaderCache concurrent access") } f(files) } - private[this] def sameClassLoader(loaders: Seq[ClassLoader]): Boolean = loaders.size < 2 || - loaders.sliding(2).forall { case Seq(x, y) => x == y } } \ No newline at end of file From 87306524a88cf312482ffa85c5df77fc280bdbac Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Tue, 3 Jun 2014 13:57:27 -0400 Subject: [PATCH 2/7] Improve error message further. --- .../src/test/scala/sbt/classpath/ConcurrentCache.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala b/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala index 470c1c8d5..8ff943b8d 100644 --- a/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala +++ b/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala @@ -15,12 +15,17 @@ object ConcurrentCache extends Properties("ClassLoaderCache concurrent access") else None } + private def showCp(cp: ClassLoader): String = cp match { + case u: java.net.URLClassLoader => u.getURLs.mkString("UrlClassLoader(", ", ", ")") + case _ => cp.toString + } + property("Same class loader for same classpaths concurrently processed") = forAll { (names: List[String], concurrent: Int) => withcp(names.distinct) { files => val cache = new ClassLoaderCache(null) val loaders = (1 to concurrent).par.map(_ => cache(files)).toList loaders match { - case DifferentClassloader(left, right) => false :| s"$left != $right" + case DifferentClassloader(left, right) => false :| s"${showCp(left)} != ${showCp(right)}" case _ => true :| "" } } From 273024a238c8ae0f56907e2cf2c4c8498d87317c Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Wed, 4 Jun 2014 11:06:49 -0400 Subject: [PATCH 3/7] Add special hook to diagnose travis failing test. --- .../src/main/scala/sbt/classpath/ClassLoaderCache.scala | 8 +++++--- .../src/test/scala/sbt/classpath/ConcurrentCache.scala | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/util/classpath/src/main/scala/sbt/classpath/ClassLoaderCache.scala b/util/classpath/src/main/scala/sbt/classpath/ClassLoaderCache.scala index 3886fe4d5..645f45747 100644 --- a/util/classpath/src/main/scala/sbt/classpath/ClassLoaderCache.scala +++ b/util/classpath/src/main/scala/sbt/classpath/ClassLoaderCache.scala @@ -5,7 +5,8 @@ import java.io.File import java.net.URLClassLoader import java.util.HashMap -private[sbt] final class ClassLoaderCache(val commonParent: ClassLoader) { +// Hack for testing only +private[sbt] final class ClassLoaderCache(val commonParent: ClassLoader, errorEvicted: Boolean = false) { private[this] val delegate = new HashMap[List[File], Reference[CachedClassLoader]] /** @@ -25,9 +26,10 @@ private[sbt] final class ClassLoaderCache(val commonParent: ClassLoader) { get(files, stamps, existingRef.get) private[this] def get(files: List[File], stamps: List[Long], existing: CachedClassLoader): ClassLoader = - if (existing == null || stamps != existing.timestamps) + if (existing == null || stamps != existing.timestamps) { + if (existing == null && errorEvicted) sys.error(s"Evicted classloader for [${files mkString ", "}]!!!! Not allowed, Travis!") newEntry(files, stamps) - else + } else existing.loader private[this] def newEntry(files: List[File], stamps: List[Long]): ClassLoader = diff --git a/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala b/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala index 8ff943b8d..9ac26af5c 100644 --- a/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala +++ b/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala @@ -22,7 +22,7 @@ object ConcurrentCache extends Properties("ClassLoaderCache concurrent access") property("Same class loader for same classpaths concurrently processed") = forAll { (names: List[String], concurrent: Int) => withcp(names.distinct) { files => - val cache = new ClassLoaderCache(null) + val cache = new ClassLoaderCache(null, errorEvicted = true) val loaders = (1 to concurrent).par.map(_ => cache(files)).toList loaders match { case DifferentClassloader(left, right) => false :| s"${showCp(left)} != ${showCp(right)}" @@ -32,7 +32,7 @@ object ConcurrentCache extends Properties("ClassLoaderCache concurrent access") } private[this] def withcp[T](names: List[String])(f: List[File] => T): T = IO.withTemporaryDirectory { tmp => - val files = names.map { name => + val files = names.sorted.map { name => val file = new File(tmp, name) IO.touch(file) file From 63d9b638c3e9c830a119f7471983456847e4cafc Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Thu, 5 Jun 2014 11:07:37 -0400 Subject: [PATCH 4/7] Wait 5 seconds instead of 2 seconds to read stderr from launched server --- launch/src/main/scala/xsbt/boot/ServerApplication.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/launch/src/main/scala/xsbt/boot/ServerApplication.scala b/launch/src/main/scala/xsbt/boot/ServerApplication.scala index 343b37185..6d4077e5b 100644 --- a/launch/src/main/scala/xsbt/boot/ServerApplication.scala +++ b/launch/src/main/scala/xsbt/boot/ServerApplication.scala @@ -112,7 +112,7 @@ class StreamDumper(in: java.io.BufferedReader, out: java.io.PrintStream) extends // just wait a couple seconds to read more stuff if there is // any stuff. if (waitForErrors) { - endTime.set(System.currentTimeMillis + 2000) + endTime.set(System.currentTimeMillis + 5000) // at this point we'd rather the dumper thread run // before we check whether to sleep Thread.`yield`() From a72448b793c17faffb5bafd2f70e11f223ed1ef6 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Thu, 5 Jun 2014 11:09:59 -0400 Subject: [PATCH 5/7] Flush output every time we write to it in StreamDumper To try harder to get our errors displayed. --- launch/src/main/scala/xsbt/boot/ServerApplication.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/launch/src/main/scala/xsbt/boot/ServerApplication.scala b/launch/src/main/scala/xsbt/boot/ServerApplication.scala index 6d4077e5b..05010fff9 100644 --- a/launch/src/main/scala/xsbt/boot/ServerApplication.scala +++ b/launch/src/main/scala/xsbt/boot/ServerApplication.scala @@ -101,6 +101,7 @@ class StreamDumper(in: java.io.BufferedReader, out: java.io.PrintStream) extends case null => () case line => out.println(line) + out.flush() read() } read() From 506d06bf67d05fab95120050820dcdf5f17ea273 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Thu, 5 Jun 2014 11:13:29 -0400 Subject: [PATCH 6/7] Don't delete the server config file if SBT_SERVER_SAVE_TEMPS ne null This allows people to reproduce failures by running the command line by hand. Fixes #1394 --- launch/src/main/scala/xsbt/boot/ServerApplication.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/launch/src/main/scala/xsbt/boot/ServerApplication.scala b/launch/src/main/scala/xsbt/boot/ServerApplication.scala index 05010fff9..b1d90fe38 100644 --- a/launch/src/main/scala/xsbt/boot/ServerApplication.scala +++ b/launch/src/main/scala/xsbt/boot/ServerApplication.scala @@ -133,7 +133,8 @@ object ServerLauncher { case None => throw new RuntimeException("Logic Failure: Attempting to start a server that isn't configured to be a server. Please report a bug.") } val launchConfig = java.io.File.createTempFile("sbtlaunch", "config") - launchConfig.deleteOnExit() + if (System.getenv("SBT_SERVER_SAVE_TEMPS") eq null) + launchConfig.deleteOnExit() LaunchConfiguration.save(config, launchConfig) val jvmArgs: List[String] = serverConfig.jvmArgs map readLines match { case Some(args) => args From 4311f87d64af303511fcadc17f69b2d5428c61b4 Mon Sep 17 00:00:00 2001 From: Josh Suereth Date: Thu, 5 Jun 2014 12:39:37 -0400 Subject: [PATCH 7/7] Remove flaky test. Fixes #1390 --- .../sbt/classpath/ClassLoaderCache.scala | 3 +- .../scala/sbt/classpath/ConcurrentCache.scala | 42 ------------------- 2 files changed, 1 insertion(+), 44 deletions(-) delete mode 100644 util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala diff --git a/util/classpath/src/main/scala/sbt/classpath/ClassLoaderCache.scala b/util/classpath/src/main/scala/sbt/classpath/ClassLoaderCache.scala index 645f45747..2e489eb6f 100644 --- a/util/classpath/src/main/scala/sbt/classpath/ClassLoaderCache.scala +++ b/util/classpath/src/main/scala/sbt/classpath/ClassLoaderCache.scala @@ -6,7 +6,7 @@ import java.net.URLClassLoader import java.util.HashMap // Hack for testing only -private[sbt] final class ClassLoaderCache(val commonParent: ClassLoader, errorEvicted: Boolean = false) { +private[sbt] final class ClassLoaderCache(val commonParent: ClassLoader) { private[this] val delegate = new HashMap[List[File], Reference[CachedClassLoader]] /** @@ -27,7 +27,6 @@ private[sbt] final class ClassLoaderCache(val commonParent: ClassLoader, errorEv private[this] def get(files: List[File], stamps: List[Long], existing: CachedClassLoader): ClassLoader = if (existing == null || stamps != existing.timestamps) { - if (existing == null && errorEvicted) sys.error(s"Evicted classloader for [${files mkString ", "}]!!!! Not allowed, Travis!") newEntry(files, stamps) } else existing.loader diff --git a/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala b/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala deleted file mode 100644 index 9ac26af5c..000000000 --- a/util/classpath/src/test/scala/sbt/classpath/ConcurrentCache.scala +++ /dev/null @@ -1,42 +0,0 @@ -package sbt -package classpath - -import org.scalacheck._ -import Prop._ -import java.io.File - -object ConcurrentCache extends Properties("ClassLoaderCache concurrent access") { - implicit lazy val concurrentArb: Arbitrary[Int] = Arbitrary(Gen.choose(1, 1000)) - implicit lazy val filenameArb: Arbitrary[String] = Arbitrary(Gen.alphaStr) - - private[this] object DifferentClassloader { - def unapply(loaders: Seq[ClassLoader]): Option[(ClassLoader, ClassLoader)] = - if (loaders.size > 1) loaders.sliding(2).find { case Seq(x, y) => x != y } map { case Seq(x, y) => (x, y) } - else None - } - - private def showCp(cp: ClassLoader): String = cp match { - case u: java.net.URLClassLoader => u.getURLs.mkString("UrlClassLoader(", ", ", ")") - case _ => cp.toString - } - - property("Same class loader for same classpaths concurrently processed") = forAll { (names: List[String], concurrent: Int) => - withcp(names.distinct) { files => - val cache = new ClassLoaderCache(null, errorEvicted = true) - val loaders = (1 to concurrent).par.map(_ => cache(files)).toList - loaders match { - case DifferentClassloader(left, right) => false :| s"${showCp(left)} != ${showCp(right)}" - case _ => true :| "" - } - } - } - - private[this] def withcp[T](names: List[String])(f: List[File] => T): T = IO.withTemporaryDirectory { tmp => - val files = names.sorted.map { name => - val file = new File(tmp, name) - IO.touch(file) - file - } - f(files) - } -} \ No newline at end of file