Review and fix precedence of and in bomDeps / depManagement (#4073)

This adds tests to ensure the versions in `JavaModule#depManagement`
have precedence over those coming via `JavaModule#bomDeps`. The tests
fail for now (might need changes in coursier…)

Includes #3924 for now

Author: alexarchambault

build.mill

@@ -116,7 +116,7 @@ object Deps {
   val asmTree = ivy"org.ow2.asm:asm-tree:9.7.1"
   val bloopConfig = ivy"ch.epfl.scala::bloop-config:1.5.5"
 
-  val coursierVersion = "2.1.19"
+  val coursierVersion = "2.1.20"
   val coursier = ivy"io.get-coursier::coursier:$coursierVersion"
   val coursierInterface = ivy"io.get-coursier:interface:1.0.25"
   val coursierJvm = ivy"io.get-coursier::coursier-jvm:$coursierVersion"

main/util/src/mill/util/CoursierSupport.scala

@@ -1,6 +1,7 @@
 package mill.util
 
 import coursier.cache.{CacheLogger, FileCache}
+import coursier.core.BomDependency
 import coursier.error.FetchError.DownloadingArtifacts
 import coursier.error.ResolutionError.CantDownloadModule
 import coursier.params.ResolutionParams
@@ -253,7 +254,8 @@ trait CoursierSupport {
       customizer: Option[Resolution => Resolution] = None,
       ctx: Option[mill.api.Ctx.Log] = None,
       coursierCacheCustomizer: Option[FileCache[Task] => FileCache[Task]] = None,
-      resolutionParams: ResolutionParams = ResolutionParams()
+      resolutionParams: ResolutionParams = ResolutionParams(),
+      boms: IterableOnce[BomDependency] = Nil
   ): Result[Resolution] = {
 
     val rootDeps = deps.iterator
@@ -277,6 +279,7 @@ trait CoursierSupport {
       .withRepositories(repositories)
       .withResolutionParams(resolutionParams0)
       .withMapDependenciesOpt(mapDependencies)
+      .withBoms(boms.toSeq)
 
     resolve.either() match {
       case Left(error) =>
@@ -313,6 +316,29 @@ trait CoursierSupport {
     }
   }
 
+  // bin-compat shim
+  def resolveDependenciesMetadataSafe(
+      repositories: Seq[Repository],
+      deps: IterableOnce[Dependency],
+      force: IterableOnce[Dependency],
+      mapDependencies: Option[Dependency => Dependency],
+      customizer: Option[Resolution => Resolution],
+      ctx: Option[mill.api.Ctx.Log],
+      coursierCacheCustomizer: Option[FileCache[Task] => FileCache[Task]],
+      resolutionParams: ResolutionParams
+  ): Result[Resolution] =
+    resolveDependenciesMetadataSafe(
+      repositories,
+      deps,
+      force,
+      mapDependencies,
+      customizer,
+      ctx,
+      coursierCacheCustomizer,
+      resolutionParams,
+      Nil
+    )
+
   // bin-compat shim
   def resolveDependenciesMetadataSafe(
       repositories: Seq[Repository],
@@ -331,7 +357,8 @@ trait CoursierSupport {
       customizer,
       ctx,
       coursierCacheCustomizer,
-      ResolutionParams()
+      ResolutionParams(),
+      Nil
     )
 
 }

scalalib/src/mill/scalalib/CoursierModule.scala

@@ -1,9 +1,9 @@
 package mill.scalalib
 
 import coursier.cache.FileCache
+import coursier.core.{BomDependency, Resolution}
 import coursier.params.ResolutionParams
 import coursier.{Dependency, Repository, Resolve, Type}
-import coursier.core.Resolution
 import mill.define.Task
 import mill.api.PathRef
 
@@ -247,20 +247,24 @@ object CoursierModule {
      */
     def processDeps[T: CoursierModule.Resolvable](
         deps: IterableOnce[T],
-        resolutionParams: ResolutionParams = ResolutionParams()
-    ): Seq[Dependency] = {
+        resolutionParams: ResolutionParams = ResolutionParams(),
+        boms: IterableOnce[BomDependency] = Nil
+    ): (Seq[coursier.core.Dependency], coursier.core.DependencyManagement.Map) = {
       val deps0 = deps
         .map(implicitly[CoursierModule.Resolvable[T]].bind(_, bind))
+      val boms0 = boms.toSeq
       val res = Lib.resolveDependenciesMetadataSafe(
         repositories = repositories,
         deps = deps0,
         mapDependencies = mapDependencies,
         customizer = customizer,
         coursierCacheCustomizer = coursierCacheCustomizer,
         ctx = ctx,
-        resolutionParams = resolutionParams
+        resolutionParams = resolutionParams,
+        boms = boms0
       ).getOrThrow
-      res.processedRootDependencies
+
+      (res.processedRootDependencies, res.bomDepMgmt)
     }
   }
 

scalalib/src/mill/scalalib/JavaModule.scala

@@ -214,7 +214,7 @@ trait JavaModule
       depMgmt: Seq[(DependencyManagement.Key, DependencyManagement.Values)],
       overrideVersions: Boolean
   ): coursier.core.Dependency => coursier.core.Dependency = {
-    val depMgmtMap = depMgmt.toMap
+    val depMgmtMap = DependencyManagement.add(Map.empty, depMgmt)
     dep =>
       val depMgmtKey = DependencyManagement.Key(
         dep.module.organization,
@@ -223,7 +223,7 @@ trait JavaModule
         dep.publication.classifier
       )
       val versionOverrideOpt =
-        if (dep.version == "_") depMgmtMap.get(depMgmtKey).map(_.version)
+        if (dep.version == "_") depMgmtMap.get(depMgmtKey).map(_.version).filter(_.nonEmpty)
         else None
       val extraExclusions = depMgmtMap.get(depMgmtKey).map(_.minimizedExclusions)
       dep
@@ -236,7 +236,7 @@ trait JavaModule
         // - overrides meant to apply to transitive dependencies
         // - fill version if it's empty
         // - add extra exclusions from dependency management
-        .withOverrides(dep.overrides ++ depMgmt)
+        .addOverrides(depMgmt)
         .withVersion(versionOverrideOpt.getOrElse(dep.version))
         .withMinimizedExclusions(
           extraExclusions.fold(dep.minimizedExclusions)(dep.minimizedExclusions.join(_))
@@ -247,7 +247,7 @@ trait JavaModule
    * Data from depManagement, converted to a type ready to be passed to coursier
    * for dependency resolution
    */
-  private def processedDependencyManagement(deps: Seq[coursier.core.Dependency])
+  protected def processedDependencyManagement(deps: Seq[coursier.core.Dependency])
       : Seq[(DependencyManagement.Key, DependencyManagement.Values)] = {
     val keyValuesOrErrors =
       deps.map { depMgmt =>

scalalib/src/mill/scalalib/Lib.scala

@@ -1,6 +1,7 @@
 package mill
 package scalalib
 
+import coursier.core.BomDependency
 import coursier.params.ResolutionParams
 import coursier.util.Task
 import coursier.{Dependency, Repository, Resolution, Type}
@@ -61,7 +62,8 @@ object Lib {
       coursierCacheCustomizer: Option[
         coursier.cache.FileCache[Task] => coursier.cache.FileCache[Task]
       ] = None,
-      resolutionParams: ResolutionParams = ResolutionParams()
+      resolutionParams: ResolutionParams = ResolutionParams(),
+      boms: IterableOnce[BomDependency] = Nil
   ): Result[Resolution] = {
     val depSeq = deps.iterator.toSeq
     mill.util.Jvm.resolveDependenciesMetadataSafe(
@@ -72,10 +74,34 @@ object Lib {
       customizer = customizer,
       ctx = ctx,
       coursierCacheCustomizer = coursierCacheCustomizer,
-      resolutionParams = resolutionParams
+      resolutionParams = resolutionParams,
+      boms = boms
     )
   }
 
+  // bin-compat shim
+  def resolveDependenciesMetadataSafe(
+      repositories: Seq[Repository],
+      deps: IterableOnce[BoundDep],
+      mapDependencies: Option[Dependency => Dependency],
+      customizer: Option[coursier.core.Resolution => coursier.core.Resolution],
+      ctx: Option[Ctx.Log],
+      coursierCacheCustomizer: Option[
+        coursier.cache.FileCache[Task] => coursier.cache.FileCache[Task]
+      ],
+      resolutionParams: ResolutionParams
+  ): Result[Resolution] =
+    resolveDependenciesMetadataSafe(
+      repositories,
+      deps,
+      mapDependencies,
+      customizer,
+      ctx,
+      coursierCacheCustomizer,
+      resolutionParams,
+      Nil
+    )
+
   // bin-compat shim
   def resolveDependenciesMetadataSafe(
       repositories: Seq[Repository],
@@ -94,7 +120,8 @@ object Lib {
       customizer,
       ctx,
       coursierCacheCustomizer,
-      ResolutionParams()
+      ResolutionParams(),
+      Nil
     )
 
   /**

scalalib/src/mill/scalalib/PublishModule.scala

@@ -1,6 +1,7 @@
 package mill
 package scalalib
 
+import coursier.core.Configuration
 import mill.define.{Command, ExternalModule, Task}
 import mill.api.{JarManifest, PathRef, Result}
 import mill.main.Tasks
@@ -125,21 +126,11 @@ trait PublishModule extends JavaModule { outer =>
    */
   def bomDetails: T[(Map[coursier.core.Module, String], coursier.core.DependencyManagement.Map)] =
     Task {
-      val processedDeps = defaultResolver().processDeps(
-        transitiveCompileIvyDeps() ++ transitiveIvyDeps(),
-        resolutionParams = resolutionParams()
+      val (processedDeps, depMgmt) = defaultResolver().processDeps(
+        processedIvyDeps(),
+        resolutionParams = resolutionParams(),
+        boms = allBomDeps().toSeq.map(_.withConfig(Configuration.compile))
       )
-      val depMgmt: coursier.core.DependencyManagement.Map =
-        if (processedDeps.isEmpty) Map.empty
-        else {
-          val overrides = processedDeps.map(_.overrides)
-          overrides.tail.foldLeft(overrides.head) { (acc, map) =>
-            acc.filter {
-              case (key, values) =>
-                map.get(key).contains(values)
-            }
-          }
-        }
       (processedDeps.map(_.moduleVersion).toMap, depMgmt)
     }
 
@@ -162,20 +153,28 @@ trait PublishModule extends JavaModule { outer =>
       else
         dep
     }
-    val overrides =
-      depManagement().toSeq.map(bindDependency()).map(_.dep)
-        .filter(depMgmt => depMgmt.version.nonEmpty && depMgmt.version != "_")
-        .map { depMgmt =>
-          Ivy.Override(
-            depMgmt.module.organization.value,
-            depMgmt.module.name.value,
-            depMgmt.version
-          )
-        } ++
-        bomDepMgmt.map {
+    val overrides = {
+      val depMgmtEntries = processedDependencyManagement(
+        depManagement().toSeq
+          .map(bindDependency())
+          .map(_.dep)
+          .filter(depMgmt => depMgmt.version.nonEmpty && depMgmt.version != "_")
+      )
+      val entries = coursier.core.DependencyManagement.add(
+        Map.empty,
+        depMgmtEntries ++ bomDepMgmt
+      )
+      entries.toVector
+        .map {
           case (key, values) =>
-            Ivy.Override(key.organization.value, key.name.value, values.version)
+            Ivy.Override(
+              key.organization.value,
+              key.name.value,
+              values.version
+            )
         }
+        .sortBy(value => (value.organization, value.name, value.version))
+    }
     val ivy = Ivy(artifactMetadata(), publishXmlDeps0, extraPublish(), overrides)
     val ivyPath = T.dest / "ivy.xml"
     os.write.over(ivyPath, ivy)