From 72b8fd102527e2344660503e4c8e86b97c57f880 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Sat, 25 Jul 2015 17:37:52 -0400 Subject: [PATCH 01/10] Reproduce stack overflow using cached resolution with circular dependency --- .../changes/multi.sbt | 48 +++++++++++++++++++ .../cached-resolution-circular/multi.sbt | 45 +++++++++++++++++ .../cached-resolution-circular/test | 13 +++++ .../circular-dependency/changes/multi.sbt | 1 + .../circular-dependency/multi.sbt | 3 +- 5 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 sbt/src/sbt-test/dependency-management/cached-resolution-circular/changes/multi.sbt create mode 100644 sbt/src/sbt-test/dependency-management/cached-resolution-circular/multi.sbt create mode 100644 sbt/src/sbt-test/dependency-management/cached-resolution-circular/test diff --git a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/changes/multi.sbt b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/changes/multi.sbt new file mode 100644 index 000000000..beea32492 --- /dev/null +++ b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/changes/multi.sbt @@ -0,0 +1,48 @@ +lazy val check = taskKey[Unit]("Runs the check") + +val sprayV = "1.1.1" +val playVersion = "2.2.0" +val summingbirdVersion = "0.4.0" +val luceneVersion = "4.0.0" +val akkaVersion = "2.3.1" + +def commonSettings: Seq[Def.Setting[_]] = + Seq( + ivyPaths := new IvyPaths( (baseDirectory in ThisBuild).value, Some((target in LocalRootProject).value / "ivy-cache")), + scalaVersion := "2.10.4", + fullResolvers := fullResolvers.value.filterNot(_.name == "inter-project"), + updateOptions := updateOptions.value.withCachedResolution(true) + ) + +lazy val a = project. + settings(commonSettings: _*). + settings( + name := "a", + libraryDependencies := Seq( + organization.value %% "c" % version.value, + "commons-io" % "commons-io" % "1.3", + "org.apache.spark" %% "spark-core" % "0.9.0-incubating" + ) + ) + +lazy val b = project. + settings(commonSettings: _*). + settings( + name := "b", + // this adds circular dependency + libraryDependencies := Seq(organization.value %% "c" % version.value) + ) + +lazy val c = project. + settings(commonSettings: _*). + settings( + name := "c", + libraryDependencies := Seq(organization.value %% "b" % version.value) + ) + +lazy val root = (project in file(".")). + settings(commonSettings: _*). + settings( + organization in ThisBuild := "org.example", + version in ThisBuild := "1.0-SNAPSHOT" + ) diff --git a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/multi.sbt b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/multi.sbt new file mode 100644 index 000000000..908893a4c --- /dev/null +++ b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/multi.sbt @@ -0,0 +1,45 @@ +lazy val check = taskKey[Unit]("Runs the check") + +val sprayV = "1.1.1" +val playVersion = "2.2.0" +val summingbirdVersion = "0.4.0" +val luceneVersion = "4.0.0" +val akkaVersion = "2.3.1" + +def commonSettings: Seq[Def.Setting[_]] = + Seq( + ivyPaths := new IvyPaths( (baseDirectory in ThisBuild).value, Some((target in LocalRootProject).value / "ivy-cache")), + scalaVersion := "2.10.4", + fullResolvers := fullResolvers.value.filterNot(_.name == "inter-project"), + updateOptions := updateOptions.value.withCachedResolution(true) + ) + +lazy val a = project. + settings(commonSettings: _*). + settings( + name := "a", + libraryDependencies := Seq( + "commons-io" % "commons-io" % "1.3", + "org.apache.spark" %% "spark-core" % "0.9.0-incubating" + ) + ) + +lazy val b = project. + settings(commonSettings: _*). + settings( + name := "b" + ) + +lazy val c = project. + settings(commonSettings: _*). + settings( + name := "c", + libraryDependencies := Seq(organization.value %% "b" % version.value) + ) + +lazy val root = (project in file(".")). + settings(commonSettings: _*). + settings( + organization in ThisBuild := "org.example", + version in ThisBuild := "1.0-SNAPSHOT" + ) diff --git a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/test b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/test new file mode 100644 index 000000000..f02099d70 --- /dev/null +++ b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/test @@ -0,0 +1,13 @@ +> a/publishLocal + +> b/publishLocal + +> c/publishLocal + +$ copy-file changes/multi.sbt multi.sbt + +> reload + +> b/publishLocal + +> a/update diff --git a/sbt/src/sbt-test/dependency-management/circular-dependency/changes/multi.sbt b/sbt/src/sbt-test/dependency-management/circular-dependency/changes/multi.sbt index e2a312aa6..68ccf8107 100644 --- a/sbt/src/sbt-test/dependency-management/circular-dependency/changes/multi.sbt +++ b/sbt/src/sbt-test/dependency-management/circular-dependency/changes/multi.sbt @@ -4,6 +4,7 @@ def commonSettings: Seq[Def.Setting[_]] = Seq( ivyPaths := new IvyPaths( (baseDirectory in ThisBuild).value, Some((target in LocalRootProject).value / "ivy-cache")), scalaVersion := "2.10.4", + fullResolvers := fullResolvers.value.filterNot(_.name == "inter-project"), updateOptions := updateOptions.value.withCircularDependencyLevel(CircularDependencyLevel.Error) ) diff --git a/sbt/src/sbt-test/dependency-management/circular-dependency/multi.sbt b/sbt/src/sbt-test/dependency-management/circular-dependency/multi.sbt index 947217506..0a6454d57 100644 --- a/sbt/src/sbt-test/dependency-management/circular-dependency/multi.sbt +++ b/sbt/src/sbt-test/dependency-management/circular-dependency/multi.sbt @@ -3,7 +3,8 @@ lazy val check = taskKey[Unit]("Runs the check") def commonSettings: Seq[Def.Setting[_]] = Seq( ivyPaths := new IvyPaths( (baseDirectory in ThisBuild).value, Some((target in LocalRootProject).value / "ivy-cache")), - scalaVersion := "2.10.4" + scalaVersion := "2.10.4", + fullResolvers := fullResolvers.value.filterNot(_.name == "inter-project") ) lazy val a = project. From 593850562a920badd4f33e43ce968dd0d9986509 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Sat, 25 Jul 2015 17:38:11 -0400 Subject: [PATCH 02/10] make sortModules tailrec --- .../sbt/ivyint/CachedResolutionResolveEngine.scala | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala index 07a157107..0bf2467b8 100644 --- a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala +++ b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala @@ -407,9 +407,11 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { }: _*) val stackGuard = reports0.size * reports0.size * 2 // sort the all modules such that less called modules comes earlier - def sortModules(cs: ListMap[(String, String), Vector[OrganizationArtifactReport]], + @tailrec def sortModules(cs: ListMap[(String, String), Vector[OrganizationArtifactReport]], + acc: ListMap[(String, String), Vector[OrganizationArtifactReport]], n: Int): ListMap[(String, String), Vector[OrganizationArtifactReport]] = { + println(s"sortModules: $n / $stackGuard") val keys = cs.keySet val (called, notCalled) = cs partition { case (k, oas) => @@ -422,9 +424,8 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { } } } - notCalled ++ - (if (called.isEmpty || n > stackGuard) called - else sortModules(called, n + 1)) + (if (called.isEmpty || n > stackGuard) acc ++ notCalled ++ called + else sortModules(called, acc ++ notCalled, n + 1)) } def resolveConflicts(cs: List[((String, String), Vector[OrganizationArtifactReport])]): List[OrganizationArtifactReport] = cs match { @@ -446,7 +447,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { x :: resolveConflicts(next) }) } - val sorted = sortModules(allModules, 0) + val sorted = sortModules(allModules, ListMap(), 0) val result = resolveConflicts(sorted.toList) result.toVector } From bc5e7d56236d913ae58059c97322fb2f12eb281d Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Sat, 25 Jul 2015 22:40:18 -0400 Subject: [PATCH 03/10] refactored to use less stack space --- .../CachedResolutionResolveEngine.scala | 97 +++++++++++-------- ivy/src/test/scala/CachedResolutionSpec.scala | 3 +- .../changes/multi.sbt | 13 ++- .../cached-resolution-circular/multi.sbt | 5 +- .../cached-resolution-circular/test | 2 +- 5 files changed, 72 insertions(+), 48 deletions(-) diff --git a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala index 0bf2467b8..037ab4664 100644 --- a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala +++ b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala @@ -396,59 +396,68 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { def mergeOrganizationArtifactReports(rootModuleConf: String, reports0: Vector[OrganizationArtifactReport], os: Vector[IvyOverride], log: Logger): Vector[OrganizationArtifactReport] = { // group by takes up too much memory. trading space with time. - val orgNamePairs = (reports0 map { oar => (oar.organization, oar.name) }).distinct + val orgNamePairs: Vector[(String, String)] = (reports0 map { oar => (oar.organization, oar.name) }).distinct // this might take up some memory, but it's limited to a single val reports1 = reports0 map { filterOutCallers } - val allModules: ListMap[(String, String), Vector[OrganizationArtifactReport]] = - ListMap(orgNamePairs map { + val allModules0: Map[(String, String), Vector[OrganizationArtifactReport]] = + Map(orgNamePairs map { case (organization, name) => val xs = reports1 filter { oar => oar.organization == organization && oar.name == name } ((organization, name), xs) }: _*) - val stackGuard = reports0.size * reports0.size * 2 // sort the all modules such that less called modules comes earlier - @tailrec def sortModules(cs: ListMap[(String, String), Vector[OrganizationArtifactReport]], - acc: ListMap[(String, String), Vector[OrganizationArtifactReport]], - n: Int): ListMap[(String, String), Vector[OrganizationArtifactReport]] = + @tailrec def sortModules(cs: Vector[(String, String)], + acc: Vector[(String, String)], + n: Int, guard: Int): Vector[(String, String)] = { - println(s"sortModules: $n / $stackGuard") - val keys = cs.keySet - val (called, notCalled) = cs partition { - case (k, oas) => - oas exists { - _.modules.exists { - _.callers exists { caller => - val m = caller.caller - keys((m.organization, m.name)) - } + // println(s"sortModules: $n / $guard") + val keys = cs.toSet + val (called, notCalled) = cs partition { k => + val reports = allModules0(k) + reports exists { + _.modules.exists { + _.callers exists { caller => + val m = caller.caller + keys((m.organization, m.name)) } } + } } - (if (called.isEmpty || n > stackGuard) acc ++ notCalled ++ called - else sortModules(called, acc ++ notCalled, n + 1)) + lazy val result0 = acc ++ notCalled ++ called + (if (n > guard) { + log.warn(s"""cached resolution detected circular dependencies: ${cs.mkString(",")}""") + result0 + } else if (called.isEmpty || notCalled.isEmpty) result0 + else sortModules(called, acc ++ notCalled, 0, called.size * called.size + 1)) } - def resolveConflicts(cs: List[((String, String), Vector[OrganizationArtifactReport])]): List[OrganizationArtifactReport] = + def resolveConflicts(cs: List[(String, String)], + allModules: Map[(String, String), Vector[OrganizationArtifactReport]]): List[OrganizationArtifactReport] = cs match { case Nil => Nil - case (k, Vector()) :: rest => resolveConflicts(rest) - case (k, Vector(oa)) :: rest if (oa.modules.size == 0) => resolveConflicts(rest) - case (k, Vector(oa)) :: rest if (oa.modules.size == 1 && !oa.modules.head.evicted) => - log.debug(s":: no conflict $rootModuleConf: ${oa.organization}:${oa.name}") - oa :: resolveConflicts(rest) - case ((organization, name), oas) :: rest => - (mergeModuleReports(rootModuleConf, oas flatMap { _.modules }, os, log) match { - case (survivor, newlyEvicted) => - val evicted = (survivor ++ newlyEvicted) filter { m => m.evicted } - val notEvicted = (survivor ++ newlyEvicted) filter { m => !m.evicted } - log.debug("::: adds " + (notEvicted map { _.module }).mkString(", ")) - log.debug("::: evicted " + (evicted map { _.module }).mkString(", ")) - val x = new OrganizationArtifactReport(organization, name, survivor ++ newlyEvicted) - val next = transitivelyEvict(rootModuleConf, rest, evicted, log) - x :: resolveConflicts(next) - }) + case (organization, name) :: rest => + val reports = allModules((organization, name)) + reports match { + case Vector() => resolveConflicts(rest, allModules) + case Vector(oa) if (oa.modules.size == 0) => resolveConflicts(rest, allModules) + case Vector(oa) if (oa.modules.size == 1 && !oa.modules.head.evicted) => + log.debug(s":: no conflict $rootModuleConf: ${oa.organization}:${oa.name}") + oa :: resolveConflicts(rest, allModules) + case oas => + (mergeModuleReports(rootModuleConf, oas flatMap { _.modules }, os, log) match { + case (survivor, newlyEvicted) => + val evicted = (survivor ++ newlyEvicted) filter { m => m.evicted } + val notEvicted = (survivor ++ newlyEvicted) filter { m => !m.evicted } + log.debug("::: adds " + (notEvicted map { _.module }).mkString(", ")) + log.debug("::: evicted " + (evicted map { _.module }).mkString(", ")) + val x = new OrganizationArtifactReport(organization, name, survivor ++ newlyEvicted) + val nextModules = transitivelyEvict(rootModuleConf, rest, allModules, evicted, log) + x :: resolveConflicts(rest, nextModules) + }) + } } - val sorted = sortModules(allModules, ListMap(), 0) - val result = resolveConflicts(sorted.toList) + val guard0 = (orgNamePairs.size * orgNamePairs.size) + 1 + val sorted: Vector[(String, String)] = sortModules(orgNamePairs, Vector(), 0, guard0) + val result = resolveConflicts(sorted.toList, allModules0) result.toVector } def filterOutCallers(report0: OrganizationArtifactReport): OrganizationArtifactReport = @@ -491,13 +500,15 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { /** * This transitively evicts any non-evicted modules whose only callers are newly evicted. */ - def transitivelyEvict(rootModuleConf: String, reports0: List[((String, String), Vector[OrganizationArtifactReport])], - evicted0: Vector[ModuleReport], log: Logger): List[((String, String), Vector[OrganizationArtifactReport])] = + def transitivelyEvict(rootModuleConf: String, pairs: List[(String, String)], + reports0: Map[(String, String), Vector[OrganizationArtifactReport]], + evicted0: Vector[ModuleReport], log: Logger): Map[(String, String), Vector[OrganizationArtifactReport]] = { val em = (evicted0 map { _.module }).toSet def isTransitivelyEvicted(mr: ModuleReport): Boolean = mr.callers forall { c => em(c.caller) } - val reports: List[((String, String), Vector[OrganizationArtifactReport])] = reports0 map { + val reports: Seq[((String, String), Vector[OrganizationArtifactReport])] = reports0.toSeq flatMap { + case (k, v) if !(pairs contains k) => Seq() case ((organization, name), oars0) => val oars = oars0 map { oar => val (affected, unaffected) = oar.modules partition { mr => @@ -511,9 +522,9 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { if (affected.isEmpty) oar else new OrganizationArtifactReport(organization, name, unaffected ++ newlyEvicted) } - ((organization, name), oars) + Seq(((organization, name), oars)) } - reports + Map(reports: _*) } /** * resolves dependency resolution conflicts in which multiple candidates are found for organization+name combos. diff --git a/ivy/src/test/scala/CachedResolutionSpec.scala b/ivy/src/test/scala/CachedResolutionSpec.scala index 16021b15c..1860b3ed8 100644 --- a/ivy/src/test/scala/CachedResolutionSpec.scala +++ b/ivy/src/test/scala/CachedResolutionSpec.scala @@ -77,6 +77,7 @@ class CachedResolutionSpec extends BaseIvySpecification { // second resolution reads from the minigraph val report = ivyUpdate(m) val modules = report.configurations.head.modules - modules must containMatch("""org\.jboss\.netty:netty:3\.2\.0.Final""") + (modules must containMatch("""org\.jboss\.netty:netty:3\.2\.0.Final""")) and + (modules must not containMatch ("""org\.jboss\.netty:netty:3\.2\.1.Final""")) } } diff --git a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/changes/multi.sbt b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/changes/multi.sbt index beea32492..b802f3e21 100644 --- a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/changes/multi.sbt +++ b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/changes/multi.sbt @@ -21,7 +21,10 @@ lazy val a = project. libraryDependencies := Seq( organization.value %% "c" % version.value, "commons-io" % "commons-io" % "1.3", - "org.apache.spark" %% "spark-core" % "0.9.0-incubating" + "org.apache.spark" %% "spark-core" % "0.9.0-incubating", + "org.apache.avro" % "avro" % "1.7.7", + "com.linkedin.pegasus" % "data-avro" % "1.9.40", + "org.jboss.netty" % "netty" % "3.2.0.Final" ) ) @@ -44,5 +47,11 @@ lazy val root = (project in file(".")). settings(commonSettings: _*). settings( organization in ThisBuild := "org.example", - version in ThisBuild := "1.0-SNAPSHOT" + version in ThisBuild := "1.0-SNAPSHOT", + check := { + val acp = (externalDependencyClasspath in Compile in a).value.map {_.data.getName}.sorted + if (!(acp contains "netty-3.2.0.Final.jar")) { + sys.error("netty-3.2.0.Final not found when it should be included: " + acp.toString) + } + } ) diff --git a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/multi.sbt b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/multi.sbt index 908893a4c..8ba4c7f15 100644 --- a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/multi.sbt +++ b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/multi.sbt @@ -20,7 +20,10 @@ lazy val a = project. name := "a", libraryDependencies := Seq( "commons-io" % "commons-io" % "1.3", - "org.apache.spark" %% "spark-core" % "0.9.0-incubating" + "org.apache.spark" %% "spark-core" % "0.9.0-incubating", + "org.apache.avro" % "avro" % "1.7.7", + "com.linkedin.pegasus" % "data-avro" % "1.9.40", + "org.jboss.netty" % "netty" % "3.2.0.Final" ) ) diff --git a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/test b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/test index f02099d70..d09401376 100644 --- a/sbt/src/sbt-test/dependency-management/cached-resolution-circular/test +++ b/sbt/src/sbt-test/dependency-management/cached-resolution-circular/test @@ -10,4 +10,4 @@ $ copy-file changes/multi.sbt multi.sbt > b/publishLocal -> a/update +> check From 792a761599c2f89c86956d2355fd1b4ee01f87ba Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Sun, 26 Jul 2015 23:19:16 -0400 Subject: [PATCH 04/10] try breaking circular dependency and continue sorting --- .../ivyint/CachedResolutionResolveEngine.scala | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala index 037ab4664..7a9c35269 100644 --- a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala +++ b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala @@ -407,7 +407,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { }: _*) // sort the all modules such that less called modules comes earlier @tailrec def sortModules(cs: Vector[(String, String)], - acc: Vector[(String, String)], + acc: Vector[(String, String)], extra: Vector[(String, String)], n: Int, guard: Int): Vector[(String, String)] = { // println(s"sortModules: $n / $guard") @@ -423,12 +423,18 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { } } } - lazy val result0 = acc ++ notCalled ++ called + lazy val result0 = acc ++ notCalled ++ called ++ extra + def warnCircular(): Unit = { + log.warn(s"""avoid circular dependency while using cached resolution: ${cs.mkString(",")}""") + } (if (n > guard) { - log.warn(s"""cached resolution detected circular dependencies: ${cs.mkString(",")}""") + warnCircular result0 - } else if (called.isEmpty || notCalled.isEmpty) result0 - else sortModules(called, acc ++ notCalled, 0, called.size * called.size + 1)) + } else if (called.isEmpty) result0 + else if (notCalled.isEmpty) { + warnCircular + sortModules(cs.tail, acc, extra :+ cs.head, n + 1, guard) + } else sortModules(called, acc ++ notCalled, extra, 0, called.size * called.size + 1)) } def resolveConflicts(cs: List[(String, String)], allModules: Map[(String, String), Vector[OrganizationArtifactReport]]): List[OrganizationArtifactReport] = @@ -456,7 +462,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { } } val guard0 = (orgNamePairs.size * orgNamePairs.size) + 1 - val sorted: Vector[(String, String)] = sortModules(orgNamePairs, Vector(), 0, guard0) + val sorted: Vector[(String, String)] = sortModules(orgNamePairs, Vector(), Vector(), 0, guard0) val result = resolveConflicts(sorted.toList, allModules0) result.toVector } From 431a90264d091d148e94812559f0987dfe2862e4 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Mon, 3 Aug 2015 07:31:32 -0400 Subject: [PATCH 05/10] Fixes #2129. break up circular dependency loops in cached resolution Simple remove-one method to workaround for circular dependency did not work. This fix traverses the entire graph to detect all loops and then breaks them up. --- .../CachedResolutionResolveEngine.scala | 75 ++++++++++++++++++- notes/0.13.9.markdown | 3 +- 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala index 7a9c35269..bae20ec75 100644 --- a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala +++ b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala @@ -405,6 +405,77 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { val xs = reports1 filter { oar => oar.organization == organization && oar.name == name } ((organization, name), xs) }: _*) + // this returns a List of Lists of (org, name). should be deterministic + def detectLoops(allModules: Map[(String, String), Vector[OrganizationArtifactReport]]): List[List[(String, String)]] = + { + val loopSets: mutable.Set[Set[(String, String)]] = mutable.Set.empty + val loopLists: mutable.ListBuffer[List[(String, String)]] = mutable.ListBuffer.empty + def testLoop(m: (String, String), current: (String, String), history: List[(String, String)]): Unit = + { + val callers = + (for { + oar <- allModules.getOrElse(current, Vector()) + mr <- oar.modules + c <- mr.callers + } yield (c.caller.organization, c.caller.name)).distinct + callers foreach { c => + if (history contains c) { + val loop = (c :: history.takeWhile(_ != c)) ::: List(c) + if (!loopSets(loop.toSet)) { + loopSets += loop.toSet + loopLists += loop + val loopStr = (loop map { case (o, n) => s"$o:$n" }).mkString("->") + log.warn(s"""avoid circular dependency while using cached resolution: $loopStr""") + } + } else testLoop(m, c, c :: history) + } + } + orgNamePairs map { orgname => + testLoop(orgname, orgname, List(orgname)) + } + loopLists.toList + } + @tailrec def breakLoops(loops: List[List[(String, String)]], + allModules1: Map[(String, String), Vector[OrganizationArtifactReport]]): Map[(String, String), Vector[OrganizationArtifactReport]] = + loops match { + case Nil => + allModules1 + case loop :: rest => + loop match { + case Nil => + breakLoops(rest, allModules1) + case loop => + val sortedLoop = loop sortBy { x => + (for { + oar <- allModules0(x) + mr <- oar.modules + c <- mr.callers + } yield c).size + } + val moduleWithMostCallers = sortedLoop.reverse.head + val next: (String, String) = loop(loop.indexOf(moduleWithMostCallers) + 1) + // remove the module with most callers as the caller of next. + // so, A -> C, B -> C, and C -> A. C has the most callers, and C -> A will be removed. + val nextMap = allModules1 map { + case (k: (String, String), oars0) if k == next => + val oars: Vector[OrganizationArtifactReport] = oars0 map { oar => + val mrs = oar.modules map { mr => + val callers0 = mr.callers + val callers = callers0 filterNot { c => (c.caller.organization, c.caller.name) == moduleWithMostCallers } + if (callers.size == callers0.size) mr + else mr.copy(callers = callers) + } + OrganizationArtifactReport(oar.organization, oar.name, mrs) + } + (k, oars) + case (k, v) => (k, v) + } + breakLoops(rest, nextMap) + } + } + val loop = detectLoops(allModules0) + log.debug(s":: $rootModuleConf: loop: $loop") + val allModules2 = breakLoops(loop, allModules0) // sort the all modules such that less called modules comes earlier @tailrec def sortModules(cs: Vector[(String, String)], acc: Vector[(String, String)], extra: Vector[(String, String)], @@ -413,7 +484,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { // println(s"sortModules: $n / $guard") val keys = cs.toSet val (called, notCalled) = cs partition { k => - val reports = allModules0(k) + val reports = allModules2(k) reports exists { _.modules.exists { _.callers exists { caller => @@ -425,7 +496,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { } lazy val result0 = acc ++ notCalled ++ called ++ extra def warnCircular(): Unit = { - log.warn(s"""avoid circular dependency while using cached resolution: ${cs.mkString(",")}""") + log.warn(s"""unexpected circular dependency while using cached resolution: ${cs.mkString(",")}""") } (if (n > guard) { warnCircular diff --git a/notes/0.13.9.markdown b/notes/0.13.9.markdown index ad21d1c20..bbfc01ec7 100644 --- a/notes/0.13.9.markdown +++ b/notes/0.13.9.markdown @@ -50,6 +50,7 @@ [2009]: https://github.com/sbt/sbt/pull/2009 [2046]: https://github.com/sbt/sbt/issues/2046 [2097]: https://github.com/sbt/sbt/pull/2097 + [2129]: https://github.com/sbt/sbt/pull/2129 ### Fixes with compatibility implications @@ -130,7 +131,7 @@ Under some circumstances, libraries that shouldn't have been evicted was being e This occured when library `A1` depended on `B2`, but a newer `A2` dropped the dependency, and `A2` and `B1` are also is in the graph. This is fixed by sorting the graph prior to eviction. -[#2030][2030]/[#1721][1721]/[#2014][2014]/[#2046][2046]/[#2097][2097] by [@eed3si9n][@eed3si9n] +[#2030][2030]/[#1721][1721]/[#2014][2014]/[#2046][2046]/[#2097][2097]/[#2129][2129] by [@eed3si9n][@eed3si9n] ### Force GC From ff729643e39b9bcc9109b5c8c968ee176919cf34 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Mon, 3 Aug 2015 08:17:48 -0400 Subject: [PATCH 06/10] Add debug logs --- .../scala/sbt/ivyint/CachedResolutionResolveEngine.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala index bae20ec75..73b0ec6fc 100644 --- a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala +++ b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala @@ -463,7 +463,10 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { val callers0 = mr.callers val callers = callers0 filterNot { c => (c.caller.organization, c.caller.name) == moduleWithMostCallers } if (callers.size == callers0.size) mr - else mr.copy(callers = callers) + else { + log.debug(s":: $rootModuleConf: removing caller $moduleWithMostCallers -> $next for sorting") + mr.copy(callers = callers) + } } OrganizationArtifactReport(oar.organization, oar.name, mrs) } @@ -534,6 +537,8 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { } val guard0 = (orgNamePairs.size * orgNamePairs.size) + 1 val sorted: Vector[(String, String)] = sortModules(orgNamePairs, Vector(), Vector(), 0, guard0) + val sortedStr = (sorted map { case (o, n) => s"$o:$n" }).mkString(", ") + log.debug(s":: sort result: $sortedStr") val result = resolveConflicts(sorted.toList, allModules0) result.toVector } From dd94cb90d997d559da1148cedd9371946f839570 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Wed, 5 Aug 2015 07:00:39 -0400 Subject: [PATCH 07/10] cached resolution: don't include callers from evicted modules --- .../sbt/ivyint/CachedResolutionResolveEngine.scala | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala index 73b0ec6fc..96c7d88b4 100644 --- a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala +++ b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala @@ -395,6 +395,18 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { */ def mergeOrganizationArtifactReports(rootModuleConf: String, reports0: Vector[OrganizationArtifactReport], os: Vector[IvyOverride], log: Logger): Vector[OrganizationArtifactReport] = { + // filter out evicted modules from further logic + def filterReports(report0: OrganizationArtifactReport): Option[OrganizationArtifactReport] = + report0.modules.toVector flatMap { mr => + if (mr.evicted || mr.problem.nonEmpty) None + else + // https://github.com/sbt/sbt/issues/1763 + Some(mr.copy(callers = JsonUtil.filterOutArtificialCallers(mr.callers))) + } match { + case Vector() => None + case ms => Some(OrganizationArtifactReport(report0.organization, report0.name, ms)) + } + // group by takes up too much memory. trading space with time. val orgNamePairs: Vector[(String, String)] = (reports0 map { oar => (oar.organization, oar.name) }).distinct // this might take up some memory, but it's limited to a single From 868740a2ae4d6e29bb8b7c7ecd9858b5e12b24b2 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Wed, 5 Aug 2015 07:01:21 -0400 Subject: [PATCH 08/10] cached resolution: use mutable map to speed up breakLoops --- .../CachedResolutionResolveEngine.scala | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala index 96c7d88b4..4ff745276 100644 --- a/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala +++ b/ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala @@ -410,7 +410,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { // group by takes up too much memory. trading space with time. val orgNamePairs: Vector[(String, String)] = (reports0 map { oar => (oar.organization, oar.name) }).distinct // this might take up some memory, but it's limited to a single - val reports1 = reports0 map { filterOutCallers } + val reports1 = reports0 flatMap { filterReports } val allModules0: Map[(String, String), Vector[OrganizationArtifactReport]] = Map(orgNamePairs map { case (organization, name) => @@ -447,15 +447,15 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { } loopLists.toList } - @tailrec def breakLoops(loops: List[List[(String, String)]], - allModules1: Map[(String, String), Vector[OrganizationArtifactReport]]): Map[(String, String), Vector[OrganizationArtifactReport]] = + val allModules2: mutable.Map[(String, String), Vector[OrganizationArtifactReport]] = + mutable.Map(allModules0.toSeq: _*) + @tailrec def breakLoops(loops: List[List[(String, String)]]): Unit = loops match { - case Nil => - allModules1 + case Nil => () case loop :: rest => loop match { case Nil => - breakLoops(rest, allModules1) + breakLoops(rest) case loop => val sortedLoop = loop sortBy { x => (for { @@ -468,7 +468,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { val next: (String, String) = loop(loop.indexOf(moduleWithMostCallers) + 1) // remove the module with most callers as the caller of next. // so, A -> C, B -> C, and C -> A. C has the most callers, and C -> A will be removed. - val nextMap = allModules1 map { + allModules2 foreach { case (k: (String, String), oars0) if k == next => val oars: Vector[OrganizationArtifactReport] = oars0 map { oar => val mrs = oar.modules map { mr => @@ -482,15 +482,17 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { } OrganizationArtifactReport(oar.organization, oar.name, mrs) } - (k, oars) - case (k, v) => (k, v) + allModules2(k) = oars + case (k, v) => // do nothing } - breakLoops(rest, nextMap) + + breakLoops(rest) } } val loop = detectLoops(allModules0) log.debug(s":: $rootModuleConf: loop: $loop") - val allModules2 = breakLoops(loop, allModules0) + breakLoops(loop) + // sort the all modules such that less called modules comes earlier @tailrec def sortModules(cs: Vector[(String, String)], acc: Vector[(String, String)], extra: Vector[(String, String)], @@ -554,14 +556,6 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine { val result = resolveConflicts(sorted.toList, allModules0) result.toVector } - def filterOutCallers(report0: OrganizationArtifactReport): OrganizationArtifactReport = - OrganizationArtifactReport( - report0.organization, - report0.name, - report0.modules map { mr => - // https://github.com/sbt/sbt/issues/1763 - mr.copy(callers = JsonUtil.filterOutArtificialCallers(mr.callers)) - }) /** * Merges ModuleReports, which represents orgnization, name, and version. * Returns a touple of (surviving modules ++ non-conflicting modules, newly evicted modules). From 10afc1f0a19a8d5a7eede56fdf84f579c4a897ad Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Mon, 10 Aug 2015 20:32:24 -0400 Subject: [PATCH 09/10] 0.13.9 --- build.sbt | 2 +- src/main/conscript/sbt/launchconfig | 4 +++- src/main/conscript/scalas/launchconfig | 4 +++- src/main/conscript/screpl/launchconfig | 4 +++- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/build.sbt b/build.sbt index 0f802c394..94392be9a 100644 --- a/build.sbt +++ b/build.sbt @@ -11,7 +11,7 @@ import Sxr.sxr // but can be shared across the multi projects. def buildLevelSettings: Seq[Setting[_]] = Seq( organization in ThisBuild := "org.scala-sbt", - version in ThisBuild := "0.13.9-SNAPSHOT", + version in ThisBuild := "0.13.9", // bintrayOrganization in ThisBuild := None, // bintrayRepository in ThisBuild := "test-test-test", bintrayOrganization in ThisBuild := { diff --git a/src/main/conscript/sbt/launchconfig b/src/main/conscript/sbt/launchconfig index f884aebcc..7394c4fd5 100644 --- a/src/main/conscript/sbt/launchconfig +++ b/src/main/conscript/sbt/launchconfig @@ -4,7 +4,7 @@ [app] org: ${sbt.organization-org.scala-sbt} name: sbt - version: ${sbt.version-read(sbt.version)[0.13.8]} + version: ${sbt.version-read(sbt.version)[0.13.9]} class: sbt.xMain components: xsbti,extra cross-versioned: ${sbt.cross.versioned-false} @@ -12,9 +12,11 @@ [repositories] local + jcenter: https://jcenter.bintray.com/ typesafe-ivy-releases: https://repo.typesafe.com/typesafe/ivy-releases/, [organization]/[module]/[revision]/[type]s/[artifact](-[classifier]).[ext], bootOnly maven-central + [boot] directory: ${sbt.boot.directory-${sbt.global.base-${user.home}/.sbt}/boot/} diff --git a/src/main/conscript/scalas/launchconfig b/src/main/conscript/scalas/launchconfig index b3a9cc26d..ab0bdeacd 100644 --- a/src/main/conscript/scalas/launchconfig +++ b/src/main/conscript/scalas/launchconfig @@ -4,7 +4,7 @@ [app] org: ${sbt.organization-org.scala-sbt} name: sbt - version: ${sbt.version-read(sbt.version)[0.13.8]} + version: ${sbt.version-read(sbt.version)[0.13.9]} class: sbt.ScriptMain components: xsbti,extra cross-versioned: ${sbt.cross.versioned-false} @@ -12,9 +12,11 @@ [repositories] local + jcenter: https://jcenter.bintray.com/ typesafe-ivy-releases: https://repo.typesafe.com/typesafe/ivy-releases/, [organization]/[module]/[revision]/[type]s/[artifact](-[classifier]).[ext], bootOnly maven-central + [boot] directory: ${sbt.boot.directory-${sbt.global.base-${user.home}/.sbt}/boot/} diff --git a/src/main/conscript/screpl/launchconfig b/src/main/conscript/screpl/launchconfig index c73788eed..4bd4e8d0c 100644 --- a/src/main/conscript/screpl/launchconfig +++ b/src/main/conscript/screpl/launchconfig @@ -4,7 +4,7 @@ [app] org: ${sbt.organization-org.scala-sbt} name: sbt - version: ${sbt.version-read(sbt.version)[0.13.8]} + version: ${sbt.version-read(sbt.version)[0.13.9]} class: sbt.ConsoleMain components: xsbti,extra cross-versioned: ${sbt.cross.versioned-false} @@ -12,9 +12,11 @@ [repositories] local + jcenter: https://jcenter.bintray.com/ typesafe-ivy-releases: https://repo.typesafe.com/typesafe/ivy-releases/, [organization]/[module]/[revision]/[type]s/[artifact](-[classifier]).[ext], bootOnly maven-central + [boot] directory: ${sbt.boot.directory-${sbt.global.base-${user.home}/.sbt}/boot/} From 946ee17081a93329d195cc460f342ac599fda668 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Tue, 11 Aug 2015 17:24:55 -0400 Subject: [PATCH 10/10] Notes --- notes/0.13.9.markdown | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/notes/0.13.9.markdown b/notes/0.13.9.markdown index bbfc01ec7..dab3fcd0f 100644 --- a/notes/0.13.9.markdown +++ b/notes/0.13.9.markdown @@ -157,3 +157,7 @@ This problem surfaced as: - Potentially other `SNAPSHOT` related issues. sbt 0.13.9 fixes this by relaxing the Maven compatiblity check, so it will read `maven-metadata.xml`. [#2075][2075] by [@eed3si9n][@eed3si9n] + +### Contributors + +Special thanks to the contributors for making this release a success. Compared to 0.13.8, there were 127 (non-merge) commits, by 14 contributors: Eugene Yokota, Dale Wijnand, Josh Suereth, Andrew Johnson, David Perez, Matthew Farwell, Antonio Cunei, Andrzej Jozwik, James Roper, Vitalii Voloshyn, Benjy, Kamil Kloch, Max Worgan, Andreas Flierl. Thank you!