Merge pull request #27 from eed3si9n/wip/netty-jetty

Persist classifier and improve version comparison

multiversion/src/main/scala/multiversion/commands/LintCommand.scala

@@ -56,23 +56,23 @@ case class LintCommand(
         val errors = deps.groupBy(_.dependency.map(_.module)).collect {
           case (Some(dep), ts) if ts.size > 1 =>
             dep -> ts.collect {
-              case TargetIndex(_, _, _, Some(dep)) => dep.version
+              case TargetIndex(_, _, _, Some(dep)) => dep
             }
         }
         val isTransitive = errors.toList.flatMap {
           case (m, vs) =>
             for {
               v <- vs
-              dep = SimpleDependency(m, v)
+              dep = SimpleDependency(m, v.version, v.classifier)
               tdep <- index.dependencies(dep)
               if tdep.dependency != Some(dep)
             } yield tdep
         }.toSet
 
         val reportedErrors = errors.filter {
-          case (module, versions) =>
-            val deps = versions
-              .map(v => SimpleDependency(module, v))
+          case (module, deps0) =>
+            val deps = deps0
+              .map(v => SimpleDependency(module, v.version, v.classifier))
               .flatMap(index.byDependency.get(_))
             !deps.exists(isTransitive)
         }
@@ -89,7 +89,7 @@ case class LintCommand(
         (module, versions) <- errors
       } {
         log(
-          s"target '$root' depends on conflicting versions of the 3rdparty dependency '${module.repr}:{${versions.commas}}'.\n" +
+          s"target '$root' depends on conflicting versions of the 3rdparty dependency '${module.repr}:{${versions.map(_.version).commas}}'.\n" +
             s"\tTo fix this problem, modify the dependency list of this target so that it only depends on one version of the 3rdparty module '${module.repr}'"
         )
       }

multiversion/src/main/scala/multiversion/indexes/TargetIndex.scala

@@ -35,6 +35,14 @@ object TargetIndex {
         case _ => None
       }
   }
+  private object JvmClassifier {
+    private val Classifier: Regex = "jvm_classifier=(.+)".r
+    def unapply(s: String): Option[String] =
+      s match {
+        case Classifier(x) => Some(x)
+        case _             => None
+      }
+  }
   def fromQuery(t: Target): TargetIndex = {
     def getStringListAttribute(key: String): List[String] =
       (for {
@@ -44,16 +52,21 @@ object TargetIndex {
       } yield dep).toList
     val deps = getStringListAttribute("deps")
     val tags = getStringListAttribute("tags")
-    val dependency = tags match {
-      case collection.Seq(JvmModule(module), JvmVersion(version)) =>
+    val module = tags.collectFirst {
+      case JvmModule(module) => module
+    }
+    val version = tags.collectFirst {
+      case JvmVersion(version) => version
+    }
+    val classifier = tags.collectFirst {
+      case JvmClassifier(classifier) => classifier
+    }
+    val dependency = (module, version) match {
+      case (Some(m), Some(v)) =>
         Some(
-          SimpleDependency(
-            SimpleModule(module.organization.value, module.name.value),
-            version
-          )
+          SimpleDependency(SimpleModule(m.organization.value, m.name.value), v, classifier)
         )
-      case _ =>
-        None
+      case _ => None
     }
     TargetIndex(
       name = t.getRule().getName(),

multiversion/src/main/scala/multiversion/outputs/ArtifactOutput.scala

@@ -95,17 +95,21 @@ object ArtifactOutput {
         "jars" -> Docs.array(mavenLabel),
         "deps" -> Docs.array(depsRef: _*),
         "exports" -> Docs.array(depsRef: _*),
-        "tags" -> Docs.array(
-          s"jvm_module=${dependency.module.repr}",
-          s"jvm_version=${dependency.version}"
-        ),
+        "tags" -> Docs.array(tags(dependency): _*),
         "visibility" -> Docs.array("//visibility:public")
       )
 
     genrule.toDoc /
       scalaImport.toDoc
   }
 
+  def tags(dep: Dependency): List[String] =
+    List(s"jvm_module=${dep.module.repr}", s"jvm_version=${dep.version}") ::: {
+      if (dep.publication.classifier.nonEmpty)
+        List(s"jvm_classifier=${dep.publication.classifier.value}")
+      else Nil
+    }
+
   def buildEvictedDoc(
       dep: Dependency,
       winner: String,
@@ -122,11 +126,7 @@ object ArtifactOutput {
         "jars" -> Docs.array(),
         "deps" -> Docs.array(depsRef: _*),
         "exports" -> Docs.array(depsRef: _*),
-        "tags" -> Docs.array(
-          s"jvm_module=${dep.module.repr}",
-          s"jvm_version=${dep.version}",
-          s"evicted=True"
-        ),
+        "tags" -> Docs.array(tags(dep) ::: List("evicted=True"): _*),
         "visibility" -> Docs.array("//visibility:public")
       )
     scalaImport.toDoc

multiversion/src/main/scala/multiversion/outputs/LintOutput.scala

@@ -1,19 +1,20 @@
 package multiversion.outputs
 
+import multiversion.resolvers.SimpleDependency
 import multiversion.resolvers.SimpleModule
 import org.typelevel.paiges.Doc
 
 final case class LintOutput(
     root: String,
-    conflicts: Map[SimpleModule, Set[String]],
+    conflicts: Map[SimpleModule, Set[SimpleDependency]],
     isFailure: Boolean
 ) {
   def toDoc: Doc = {
     // Sort the conflicts to ensure the output is stable.
     val sortedConflicts = conflicts
       .map {
-        case (module, versions) =>
-          val versionsDoc = Docs.array(versions.toList.sorted: _*)
+        case (module, deps) =>
+          val versionsDoc = Docs.array(deps.toList.map(_.version).sorted: _*)
           module.repr -> versionsDoc
       }
       .toList

multiversion/src/main/scala/multiversion/outputs/ResolutionIndex.scala

@@ -8,6 +8,8 @@ import coursier.core.Dependency
 import coursier.core.Module
 import coursier.core.Version
 import coursier.version.VersionCompatibility
+import coursier.version.VersionInterval
+import coursier.version.VersionParse
 import multiversion.configs.ThirdpartyConfig
 import multiversion.diagnostics.MultidepsEnrichments.XtensionDependency
 import multiversion.resolvers.DependencyId
@@ -168,11 +170,7 @@ object ResolutionIndex {
     val versions = deps.map(d => Version(d.version))
     versions.foreach { version =>
       val isCompatible = winners.exists { winner =>
-        // we need to check both ways
-        if (
-          compat.isCompatible(version.repr, winner.repr) ||
-          compat.isCompatible(winner.repr, version.repr)
-        ) {
+        if (isCompat(version.repr, winner.repr, compat)) {
           if (lessThan(winner, version) && !isUnstable(version)) {
             winners.remove(winner)
             winners.add(version)
@@ -189,9 +187,21 @@ object ResolutionIndex {
     val result = (for {
       dep <- deps
       winner <- winners
-      if dep.version != winner.repr &&
-        compat.isCompatible(dep.version, winner.repr)
+      if (dep.version != winner.repr) && isCompat(dep.version, winner.repr, compat)
     } yield dep -> winner.repr).toMap
     result
   }
+
+  def isCompat(version1: String, version2: String, compat: VersionCompatibility): Boolean = {
+    val min1 = minimumVersion(version1, compat)
+    val min2 = minimumVersion(version2, compat)
+    (min1 == min2) || (min1 + ".0" == min2) || (min1 == min2 + ".0")
+  }
+
+  def minimumVersion(version: String, compat: VersionCompatibility): String = {
+    val c = VersionParse.versionConstraint(version)
+    if (c.interval != VersionInterval.zero && c.interval.from.isDefined)
+      compat.minimumCompatibleVersion(c.interval.from.get.repr)
+    else compat.minimumCompatibleVersion(version)
+  }
 }

multiversion/src/main/scala/multiversion/resolvers/SimpleDependency.scala

@@ -4,8 +4,17 @@ import scala.util.hashing.MurmurHash3
 
 case class SimpleDependency(
     module: SimpleModule,
-    version: String
+    version: String,
+    classifier: Option[String],
 ) {
-  def repr: String = s"${module.repr}:$version"
+  private[this] def classifierStr: String =
+    classifier match {
+      case Some(x) => s"-$x"
+      case _       => ""
+    }
+  // https://repo1.maven.org/maven2/org/apache/kafka/kafka-clients/2.4.0/ lists
+  // kafka-clients-2.4.0-test.jar
+  def repr: String = s"${module.repr}:$version${classifierStr}"
+
   override lazy val hashCode: Int = MurmurHash3.productHash(this)
 }

tests/src/test/scala/tests/commands/ExportCommandSuite.scala

@@ -27,6 +27,16 @@ class ExportCommandSuite extends tests.BaseSuite {
                        |""".stripMargin
   )
 
+  checkDeps(
+    "netty",
+    """|  - dependency: io.netty:netty:3.10.1.Final
+       |  - dependency: io.netty:netty:3.7.0.Final
+       |""".stripMargin,
+    queryArgs = allGenrules,
+    expectedQuery = """|@maven//:genrules/io.netty_netty_3.10.1.Final
+                       |""".stripMargin
+  )
+
   checkDeps(
     "basic",
     """|  - dependency: com.google.guava:guava:29.0-jre
@@ -61,7 +71,7 @@ class ExportCommandSuite extends tests.BaseSuite {
         |    versionScheme: pvp
         |  - dependency: com.lihaoyi:pprint_2.12:0.5.9
         |${scalaLibrary("MyApp.scala", "object MyApp { val x = 42 }")}
-        |""".stripMargin
+        |""".stripMargin,
   )
 
   checkDeps(