Merge pull request #1760 from sbt/fix/1760

cached resolution: transitive dependency survives even after its caller is evicted

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

@@ -18,6 +18,7 @@ import org.apache.ivy.util.{ Message, MessageLogger }
 import org.apache.ivy.plugins.latest.{ ArtifactInfo => IvyArtifactInfo }
 import org.apache.ivy.plugins.matcher.{ MapMatcher, PatternMatcher }
 import Configurations.{ System => _, _ }
+import annotation.tailrec
 
 private[sbt] object CachedResolutionResolveCache {
   def createID(organization: String, name: String, revision: String) =
@@ -348,14 +349,59 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine {
       } map { _.module }
       new ConfigurationReport(rootModuleConf, modules, details, evicted)
     }
+  /**
+   * Returns a tuple of (merged org + name combo, newly evicted modules)
+   */
   def mergeOrganizationArtifactReports(rootModuleConf: String, reports0: Vector[OrganizationArtifactReport], os: Vector[IvyOverride], log: Logger): Vector[OrganizationArtifactReport] =
-    (reports0 groupBy { oar => (oar.organization, oar.name) }).toSeq.toVector flatMap {
-      case ((org, name), xs) =>
-        log.debug(s""":::: $rootModuleConf: $org:$name""")
-        if (xs.size < 2) xs
-        else Vector(new OrganizationArtifactReport(org, name, mergeModuleReports(rootModuleConf, xs flatMap { _.modules }, os, log)))
+    {
+      val pairs = (reports0 groupBy { oar => (oar.organization, oar.name) }).toSeq.toVector map {
+        case ((org, name), xs) =>
+          log.debug(s""":::: $rootModuleConf: $org:$name""")
+          if (xs.size < 2) (xs.head, Vector())
+          else
+            mergeModuleReports(rootModuleConf, xs flatMap { _.modules }, os, log) match {
+              case (survivor, newlyEvicted) =>
+                (new OrganizationArtifactReport(org, name, survivor ++ newlyEvicted), newlyEvicted)
+            }
+      }
+      transitivelyEvict(rootModuleConf, pairs map { _._1 }, pairs flatMap { _._2 }, log)
     }
-  def mergeModuleReports(rootModuleConf: String, modules: Vector[ModuleReport], os: Vector[IvyOverride], log: Logger): Vector[ModuleReport] =
+  /**
+   * This transitively evicts any non-evicted modules whose only callers are newly evicted.
+   */
+  @tailrec
+  private final def transitivelyEvict(rootModuleConf: String, reports0: Vector[OrganizationArtifactReport],
+    evicted0: Vector[ModuleReport], log: Logger): Vector[OrganizationArtifactReport] =
+    {
+      val em = evicted0 map { _.module }
+      def isTransitivelyEvicted(mr: ModuleReport): Boolean =
+        mr.callers forall { c => em contains { c.caller } }
+      // Ordering of the OrganizationArtifactReport matters
+      val pairs: Vector[(OrganizationArtifactReport, Vector[ModuleReport])] = reports0 map { oar =>
+        val organization = oar.organization
+        val name = oar.name
+        val (affected, unaffected) = oar.modules.toVector partition { mr =>
+          val x = !mr.evicted && mr.problem.isEmpty && isTransitivelyEvicted(mr)
+          if (x) {
+            log.debug(s""":::: transitively evicted $rootModuleConf: $organization:$name""")
+          }
+          x
+        }
+        val newlyEvicted = affected map { _.copy(evicted = true, evictedReason = Some("transitive-evict")) }
+        if (affected.isEmpty) (oar, Vector())
+        else
+          (new OrganizationArtifactReport(organization, name, unaffected ++ newlyEvicted), newlyEvicted)
+      }
+      val reports = pairs map { _._1 }
+      val evicted = pairs flatMap { _._2 }
+      if (evicted.isEmpty) reports
+      else transitivelyEvict(rootModuleConf, reports, evicted, log)
+    }
+  /**
+   * 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: Vector[ModuleReport], os: Vector[IvyOverride], log: Logger): (Vector[ModuleReport], Vector[ModuleReport]) =
     {
       def mergeModuleReports(org: String, name: String, version: String, xs: Vector[ModuleReport]): ModuleReport = {
         val completelyEvicted = xs forall { _.evicted }
@@ -370,10 +416,10 @@ private[sbt] trait CachedResolutionResolveEngine extends ResolveEngine {
           else Vector(mergeModuleReports(org, name, version, xs))
       }
       val conflicts = merged filter { m => !m.evicted && m.problem.isEmpty }
-      if (conflicts.size < 2) merged
+      if (conflicts.size < 2) (merged, Vector())
       else resolveConflict(rootModuleConf, conflicts, os, log) match {
         case (survivor, evicted) =>
-          survivor ++ evicted ++ (merged filter { m => m.evicted || m.problem.isDefined })
+          (survivor ++ (merged filter { m => m.evicted || m.problem.isDefined }), evicted)
       }
     }
   /**

notes/0.13.8/cached-resolution-fixes.markdown

@@ -4,6 +4,7 @@
   [@jsuereth]: https://github.com/jsuereth
   [1711]: https://github.com/sbt/sbt/issues/1711
   [1752]: https://github.com/sbt/sbt/pull/1752
+  [1760]: https://github.com/sbt/sbt/pull/1760
 
 ### Fixes with compatibility implications
 
@@ -13,3 +14,4 @@
 
 - Fixes cached resolution handling of internal depdendencies. [#1711][1711] by [@eed3si9n][@eed3si9n]
 - Fixes cached resolution being too verbose. [#1752][1752] by [@eed3si9n][@eed3si9n]
+- Fixes cached resolution not evicting modules transitively. [#1760][#1760] by [@eed3si9n][@eed3si9n]

sbt/src/sbt-test/dependency-management/cached-resolution-conflicts/multi.sbt

@@ -1,4 +1,5 @@
 // https://github.com/sbt/sbt/issues/1710
+// https://github.com/sbt/sbt/issues/1760
 lazy val check = taskKey[Unit]("Runs the check")
 
 def commonSettings: Seq[Def.Setting[_]] =
@@ -17,7 +18,7 @@ def cachedResolutionSettings: Seq[Def.Setting[_]] =
   )
 
 lazy val X1 = project.
-  settings(commonSettings: _*).
+  settings(cachedResolutionSettings: _*).
   settings(
     libraryDependencies ++= Seq(
       "com.example" %% "y1" % "0.1.0" % "compile->compile;runtime->runtime",
@@ -25,7 +26,7 @@ lazy val X1 = project.
   )
 
 lazy val Y1 = project.
-  settings(commonSettings: _*).
+  settings(cachedResolutionSettings: _*).
   settings(
     name := "y1",
     libraryDependencies ++= Seq(
@@ -33,19 +34,23 @@ lazy val Y1 = project.
       "com.ning" % "async-http-client" % "1.8.14",
       // this includes slf4j 1.6.6
       "com.twitter" % "summingbird-core_2.10" % "0.5.0",
-      "org.slf4j" % "slf4j-api" % "1.6.6" force()
+      "org.slf4j" % "slf4j-api" % "1.6.6" force(),
+      // this includes servlet-api 2.3
+      "commons-logging" % "commons-logging" % "1.1"
     )
   )
 
 lazy val Y2 = project.
-  settings(commonSettings: _*).
+  settings(cachedResolutionSettings: _*).
   settings(
     name := "y2",
     libraryDependencies ++= Seq(
       // this includes slf4j 1.6.6
       "com.twitter" % "summingbird-core_2.10" % "0.5.0",
       // this includes slf4j 1.7.5
-      "com.ning" % "async-http-client" % "1.8.14")
+      "com.ning" % "async-http-client" % "1.8.14",
+      "commons-logging" % "commons-logging" % "1.1.3"
+    )
   )
 
 lazy val root = (project in file(".")).
@@ -53,10 +58,15 @@ lazy val root = (project in file(".")).
     organization in ThisBuild := "org.example",
     version in ThisBuild := "1.0",
     check := {
-      val x1cp = (externalDependencyClasspath in Compile in X1).value.sortBy {_.data.getName}
+      val x1cp = (externalDependencyClasspath in Compile in X1).value.map {_.data.getName}.sorted
       // sys.error("slf4j-api is not found on X1" + x1cp)
-      if (!(x1cp exists {_.data.getName contains "slf4j-api"})) {
-        sys.error("slf4j-api is not found on X1" + x1cp)
+      if (!(x1cp contains "slf4j-api-1.6.6.jar")) {
+        sys.error("slf4j-api-1.6.6.jar is not found on X1" + x1cp)
       }
+
+      //sys.error(x1cp.toString)
+      if (x1cp contains "servlet-api-2.3.jar") {
+        sys.error("servlet-api-2.3.jar is found when it should be evicted:" + x1cp)
+      } 
     }
   )