From 431a90264d091d148e94812559f0987dfe2862e4 Mon Sep 17 00:00:00 2001 From: Eugene Yokota Date: Mon, 3 Aug 2015 07:31:32 -0400 Subject: [PATCH 1/2] 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 2/2] 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 }