Merge pull request #2097 from sbt/wip/2046

Fixes #2046. Cached resolution: Fixes double eviction

ivy/src/main/scala/sbt/JsonUtil.scala

@@ -5,8 +5,12 @@ import java.net.URL
 import org.apache.ivy.core
 import core.module.descriptor.ModuleDescriptor
 import sbt.serialization._
+import java.net.{ URLEncoder, URLDecoder }
 
 private[sbt] object JsonUtil {
+  def sbtOrgTemp = "org.scala-sbt.temp"
+  def fakeCallerOrganization = "org.scala-sbt.temp-callers"
+
   def parseUpdateReport(md: ModuleDescriptor, path: File, cachedDescriptor: File, log: Logger): UpdateReport =
     {
       try {
@@ -33,24 +37,23 @@ private[sbt] object JsonUtil {
             mr.evicted, mr.evictedData, mr.evictedReason,
             mr.problem, mr.homepage, mr.extraAttributes,
             mr.isDefault, mr.branch, mr.configurations, mr.licenses,
-            summarizeCallers(mr.callers))
+            filterOutArtificialCallers(mr.callers))
         })
       })
     })
   // #1763/#2030. Caller takes up 97% of space, so we need to shrink it down,
   // but there are semantics associated with some of them.
-  def summarizeCallers(callers: Seq[Caller]): Seq[Caller] =
+  def filterOutArtificialCallers(callers: Seq[Caller]): Seq[Caller] =
     if (callers.isEmpty) callers
     else {
-      // Use the first element to represent all callers
+      val nonArtificial = callers filter { c =>
-      val head = callers.head
+        (c.caller.organization != sbtOrgTemp) &&
-      val caller = new Caller(
+          (c.caller.organization != fakeCallerOrganization)
-        head.caller, head.callerConfigurations, head.callerExtraAttributes,
+      }
-        callers exists { _.isForceDependency },
+      val interProj = (callers filter { c =>
-        callers exists { _.isChangingDependency },
+        (c.caller.organization == sbtOrgTemp)
-        callers exists { _.isTransitiveDependency },
+      }).headOption.toList
-        callers exists { _.isDirectlyForceDependency })
+      interProj ::: nonArtificial.toList
-      Seq(caller)
     }
 
   def fromLite(lite: UpdateReportLite, cachedDescriptor: File): UpdateReport =

ivy/src/main/scala/sbt/ivyint/CachedResolutionResolveEngine.scala

@@ -26,8 +26,8 @@ import scala.concurrent.duration._
 private[sbt] object CachedResolutionResolveCache {
   def createID(organization: String, name: String, revision: String) =
     ModuleRevisionId.newInstance(organization, name, revision)
-  def sbtOrgTemp = "org.scala-sbt.temp"
+  def sbtOrgTemp = JsonUtil.sbtOrgTemp
-  def graphVersion = "0.13.9"
+  def graphVersion = "0.13.9B"
   val buildStartup: Long = System.currentTimeMillis
   lazy val todayStr: String = toYyyymmdd(buildStartup)
   lazy val tomorrowStr: String = toYyyymmdd(buildStartup + (1 day).toMillis)
@@ -367,6 +367,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine {
       val cachedReports = reports filter { !_.stats.cached }
       val stats = new UpdateStats(resolveTime, (cachedReports map { _.stats.downloadTime }).sum, (cachedReports map { _.stats.downloadSize }).sum, false)
       val configReports = rootModuleConfigs map { conf =>
+        log.debug("::: -----------")
         val crs = reports flatMap { _.configurations filter { _.configuration == conf.getName } }
         mergeConfigurationReports(conf.getName, crs, os, log)
       }
@@ -392,70 +393,84 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine {
   /**
    * Returns a tuple of (merged org + name combo, newly evicted modules)
    */
-  def mergeOrganizationArtifactReports(rootModuleConf: String, reports0: Seq[OrganizationArtifactReport], os: Vector[IvyOverride], log: Logger): Vector[OrganizationArtifactReport] =
+  def mergeOrganizationArtifactReports(rootModuleConf: String, reports0: Vector[OrganizationArtifactReport], os: Vector[IvyOverride], log: Logger): Vector[OrganizationArtifactReport] =
     {
-      val evicteds: mutable.ListBuffer[ModuleReport] = mutable.ListBuffer()
-      val results: mutable.ListBuffer[OrganizationArtifactReport] = mutable.ListBuffer()
       // group by takes up too much memory. trading space with time.
       val orgNamePairs = (reports0 map { oar => (oar.organization, oar.name) }).distinct
-      orgNamePairs foreach {
+      // this might take up some memory, but it's limited to a single
-        case (organization, name) =>
+      val reports1 = reports0 map { filterOutCallers }
-          // hand rolling groupBy to avoid memory allocation
+      val allModules: ListMap[(String, String), Vector[OrganizationArtifactReport]] =
-          val xs = reports0 filter { oar => oar.organization == organization && oar.name == name }
+        ListMap(orgNamePairs map {
-          if (xs.size == 0) () // do nothing
+          case (organization, name) =>
-          else if (xs.size == 1) results += xs.head
+            val xs = reports1 filter { oar => oar.organization == organization && oar.name == name }
-          else
+            ((organization, name), xs)
-            results += (mergeModuleReports(rootModuleConf, xs flatMap { _.modules }, os, log) match {
+        }: _*)
-              case (survivor, newlyEvicted) =>
+      val stackGuard = reports0.size * reports0.size * 2
-                evicteds ++= newlyEvicted
+      // sort the all modules such that less called modules comes earlier
-                new OrganizationArtifactReport(organization, name, survivor ++ newlyEvicted)
+      def sortModules(cs: ListMap[(String, String), Vector[OrganizationArtifactReport]],
-            })
+        n: Int): ListMap[(String, String), Vector[OrganizationArtifactReport]] =
-      }
+        {
-      transitivelyEvict(rootModuleConf, results.toList.toVector, evicteds.toList, log)
+          val keys = cs.keySet
-    }
+          val (called, notCalled) = cs partition {
-  /**
+            case (k, oas) =>
-   * This transitively evicts any non-evicted modules whose only callers are newly evicted.
+              oas exists {
-   */
+                _.modules.exists {
-  @tailrec
+                  _.callers exists { caller =>
-  private final def transitivelyEvict(rootModuleConf: String, reports0: Vector[OrganizationArtifactReport],
+                    val m = caller.caller
-    evicted0: List[ModuleReport], log: Logger): Vector[OrganizationArtifactReport] =
+                    keys((m.organization, m.name))
-    {
+                  }
-      val em = evicted0 map { _.module }
+                }
-      def isTransitivelyEvicted(mr: ModuleReport): Boolean =
+              }
-        mr.callers forall { c => em contains { c.caller } }
-      val evicteds: mutable.ListBuffer[ModuleReport] = mutable.ListBuffer()
-      // Ordering of the OrganizationArtifactReport matters
-      val reports: Vector[OrganizationArtifactReport] = reports0 map { oar =>
-        val organization = oar.organization
-        val name = oar.name
-        val (affected, unaffected) = oar.modules partition { mr =>
-          val x = !mr.evicted && mr.problem.isEmpty && isTransitivelyEvicted(mr)
-          if (x) {
-            log.debug(s""":::: transitively evicted $rootModuleConf: $organization:$name""")
           }
-          x
+          notCalled ++
+            (if (called.isEmpty || n > stackGuard) called
+            else sortModules(called, n + 1))
         }
-        val newlyEvicted = affected map { _.copy(evicted = true, evictedReason = Some("transitive-evict")) }
+      def resolveConflicts(cs: List[((String, String), Vector[OrganizationArtifactReport])]): List[OrganizationArtifactReport] =
-        if (affected.isEmpty) oar
+        cs match {
-        else {
+          case Nil => Nil
-          evicteds ++= newlyEvicted
+          case (k, Vector()) :: rest => resolveConflicts(rest)
-          new OrganizationArtifactReport(organization, name, unaffected ++ newlyEvicted)
+          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)
+            })
         }
-      }
+      val sorted = sortModules(allModules, 0)
-      if (evicteds.isEmpty) reports
+      val result = resolveConflicts(sorted.toList)
-      else transitivelyEvict(rootModuleConf, reports, evicteds.toList, log)
+      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).
    */
   def mergeModuleReports(rootModuleConf: String, modules: Seq[ModuleReport], os: Vector[IvyOverride], log: Logger): (Vector[ModuleReport], Vector[ModuleReport]) =
     {
+      if (modules.nonEmpty) {
+        log.debug(s":: merging module reports for $rootModuleConf: ${modules.head.module.organization}:${modules.head.module.name}")
+      }
       def mergeModuleReports(org: String, name: String, version: String, xs: Seq[ModuleReport]): ModuleReport = {
         val completelyEvicted = xs forall { _.evicted }
         val allCallers = xs flatMap { _.callers }
         val allArtifacts = (xs flatMap { _.artifacts }).distinct
-        log.debug(s":: merging module report for $org:$name:$version - $allArtifacts")
         xs.head.copy(artifacts = allArtifacts, evicted = completelyEvicted, callers = allCallers)
       }
       val merged = (modules groupBy { m => (m.module.organization, m.module.name, m.module.revision) }).toSeq.toVector flatMap {
@@ -470,6 +485,33 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine {
           (survivor ++ (merged filter { m => m.evicted || m.problem.isDefined }), evicted)
       }
     }
+  /**
+   * 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])] =
+    {
+      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 {
+        case ((organization, name), oars0) =>
+          val oars = oars0 map { oar =>
+            val (affected, unaffected) = oar.modules partition { mr =>
+              val x = !mr.evicted && mr.problem.isEmpty && isTransitivelyEvicted(mr)
+              if (x) {
+                log.debug(s""":::: transitively evicted $rootModuleConf: ${mr.module}""")
+              }
+              x
+            }
+            val newlyEvicted = affected map { _.copy(evicted = true, evictedReason = Some("transitive-evict")) }
+            if (affected.isEmpty) oar
+            else new OrganizationArtifactReport(organization, name, unaffected ++ newlyEvicted)
+          }
+          ((organization, name), oars)
+      }
+      reports
+    }
   /**
    * resolves dependency resolution conflicts in which multiple candidates are found for organization+name combos.
    * The main input is conflicts, which is a Vector of ModuleReport, which contains full info on the modulerevision, including its callers.
@@ -487,7 +529,7 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine {
       val head = conflicts.head
       val organization = head.module.organization
       val name = head.module.name
-      log.debug(s"- conflict in $rootModuleConf:$organization:$name " + (conflicts map { _.module }).mkString("(", ", ", ")"))
+      log.debug(s"::: resolving conflict in $rootModuleConf:$organization:$name " + (conflicts map { _.module }).mkString("(", ", ", ")"))
       def useLatest(lcm: LatestConflictManager): (Vector[ModuleReport], Vector[ModuleReport], String) =
         (conflicts find { m =>
           m.callers.exists { _.isDirectlyForceDependency }
@@ -536,7 +578,8 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine {
       if (conflicts.size == 2 && os.isEmpty) {
         val (cf0, cf1) = (conflicts(0).module, conflicts(1).module)
         val cache = cachedResolutionResolveCache
-        cache.getOrElseUpdateConflict(cf0, cf1, conflicts) { doResolveConflict }
+        val (surviving, evicted) = cache.getOrElseUpdateConflict(cf0, cf1, conflicts) { doResolveConflict }
+        (surviving, evicted)
       } else {
         val (surviving, evicted, mgr) = doResolveConflict
         (surviving, evicted)

ivy/src/test/scala/BaseIvySpecification.scala

@@ -59,6 +59,11 @@ trait BaseIvySpecification extends Specification {
     IvyActions.updateEither(module, config, UnresolvedWarningConfiguration(), LogicalClock.unknown, Some(currentDependency), log)
   }
 
+  def cleanCachedResolutionCache(module: IvySbt#Module): Unit =
+    {
+      IvyActions.cleanCachedResolutionCache(module, log)
+    }
+
   def ivyUpdate(module: IvySbt#Module) =
     ivyUpdateEither(module) match {
       case Right(r) => r

ivy/src/test/scala/CachedResolutionSpec.scala

@@ -12,27 +12,38 @@ class CachedResolutionSpec extends BaseIvySpecification {
 
   Resolving the unsolvable module should
     not work                                                    $e2
+
+  Resolving a module with a pseudo-conflict should
+    work                                                        $e3
                                                                 """
 
   def commonsIo13 = ModuleID("commons-io", "commons-io", "1.3", Some("compile"))
   def mavenCayennePlugin302 = ModuleID("org.apache.cayenne.plugins", "maven-cayenne-plugin", "3.0.2", Some("compile"))
+  def avro177 = ModuleID("org.apache.avro", "avro", "1.7.7", Some("compile"))
+  def dataAvro1940 = ModuleID("com.linkedin.pegasus", "data-avro", "1.9.40", Some("compile"))
+  def netty320 = ModuleID("org.jboss.netty", "netty", "3.2.0.Final", Some("compile"))
 
   def defaultOptions = EvictionWarningOptions.default
 
   import ShowLines._
 
   def e1 = {
-    val m = module(ModuleID("com.example", "foo", "0.1.0", Some("compile")), Seq(commonsIo13), Some("2.10.2"), UpdateOptions().withCachedResolution(true))
+    val m = module(ModuleID("com.example", "foo", "0.1.0", Some("compile")),
+      Seq(commonsIo13), Some("2.10.2"), UpdateOptions().withCachedResolution(true))
     val report = ivyUpdate(m)
+    cleanCachedResolutionCache(m)
     val report2 = ivyUpdate(m)
+    // first resolution creates the minigraph
     println(report)
+    // second resolution reads from the minigraph
     println(report.configurations.head.modules.head.artifacts)
     report.configurations.size must_== 3
   }
 
   def e2 = {
-    log.setLevel(Level.Debug)
+    // log.setLevel(Level.Debug)
-    val m = module(ModuleID("com.example", "foo", "0.2.0", Some("compile")), Seq(mavenCayennePlugin302), Some("2.10.2"), UpdateOptions().withCachedResolution(true))
+    val m = module(ModuleID("com.example", "foo", "0.2.0", Some("compile")),
+      Seq(mavenCayennePlugin302), Some("2.10.2"), UpdateOptions().withCachedResolution(true))
     ivyUpdateEither(m) match {
       case Right(_) => sys.error("this should've failed")
       case Left(uw) =>
@@ -48,4 +59,22 @@ class CachedResolutionSpec extends BaseIvySpecification {
           "\t\t  +- com.example:foo:0.2.0"))
     }
   }
+
+  // https://github.com/sbt/sbt/issues/2046
+  // data-avro:1.9.40 depends on avro:1.4.0, which depends on netty:3.2.1.Final.
+  // avro:1.4.0 will be evicted by avro:1.7.7.
+  // #2046 says that netty:3.2.0.Final is incorrectly evicted by netty:3.2.1.Final
+  def e3 = {
+    log.setLevel(Level.Debug)
+    val m = module(ModuleID("com.example", "foo", "0.3.0", Some("compile")),
+      Seq(avro177, dataAvro1940, netty320),
+      Some("2.10.2"), UpdateOptions().withCachedResolution(true))
+    // first resolution creates the minigraph
+    val report0 = ivyUpdate(m)
+    cleanCachedResolutionCache(m)
+    // 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""")
+  }
 }