Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BOM support for compileIvyDeps, runIvyDeps, and BOM test dependencies #4068

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
d35695e
Add BOM / dependency management support
alexarchambault Nov 25, 2024
793be6c
Remove JavaModule#allIvyDeps
alexarchambault Dec 2, 2024
e920c46
Re-organize things
alexarchambault Dec 2, 2024
45fb343
Add JavaModule#compileDependencyManagement
alexarchambault Dec 2, 2024
a5af7eb
Take BOMs into account for tests
alexarchambault Dec 2, 2024
3bee9eb
Add JavaModule#runDependencyManagement
alexarchambault Dec 2, 2024
383d065
Apply BOM and dep mgmt to transitive dependencies from module deps
alexarchambault Dec 2, 2024
b8e50e1
ivy.xml stuff
alexarchambault Dec 2, 2024
1570f37
Add BOM scope tests
alexarchambault Dec 2, 2024
766990d
Merge branch 'main' into pr/bom-scopes
alexarchambault Dec 5, 2024
93e1d52
more
alexarchambault Dec 5, 2024
d7eaa35
Merge branch 'main' into pr/bom-scopes
alexarchambault Dec 6, 2024
140b787
Tweaking / more tests
alexarchambault Dec 6, 2024
5785874
some more
alexarchambault Dec 6, 2024
a34b2ee
Add comments
alexarchambault Dec 6, 2024
c4b5ef8
mima
alexarchambault Dec 6, 2024
674e23f
Add comment
alexarchambault Dec 6, 2024
ac711fa
Merge branch 'main' into pr/bom-scopes
alexarchambault Dec 10, 2024
42af383
Merge branch 'main' into pr/bom-scopes
alexarchambault Dec 11, 2024
6bbbaca
Rename {all,extra}BomDeps to {all,extra}BomIvyDeps
alexarchambault Dec 11, 2024
ac69ceb
Merge branch 'main' into bom-scopes
lihaoyi Dec 14, 2024
d61b25e
Merge branch 'main' into pr/bom-scopes
alexarchambault Dec 15, 2024
70a85e1
fmt
alexarchambault Dec 15, 2024
baf330b
Fix tests
alexarchambault Dec 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions build.mill
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,9 @@ trait MillStableScalaModule extends MillPublishScalaModule with Mima {
ProblemFilter.exclude[ReversedMissingMethodProblem](
"mill.scalalib.JavaModule.mill$scalalib$JavaModule$$super$runMain"
),
// Added overrides for a new targets, should be safe
ProblemFilter.exclude[ReversedMissingMethodProblem]("mill.scalalib.JavaModule#JavaModuleTests.mill$scalalib$JavaModule$JavaModuleTests$$super$extraBomIvyDeps"),
ProblemFilter.exclude[ReversedMissingMethodProblem]("mill.scalalib.JavaModule#JavaModuleTests.mill$scalalib$JavaModule$JavaModuleTests$$super$extraDepManagement"),
// Terminal is sealed, not sure why MIMA still complains
ProblemFilter.exclude[ReversedMissingMethodProblem]("mill.eval.Terminal.task"),

Expand Down
158 changes: 149 additions & 9 deletions scalalib/src/mill/scalalib/JavaModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,15 @@ trait JavaModule
}
}

override def extraBomIvyDeps = Task.Anon[Agg[BomDependency]] {
super.extraBomIvyDeps() ++
outer.allBomIvyDeps().map(_.withConfig(Configuration.test))
}
override def extraDepManagement = Task.Anon[Agg[Dep]] {
super.extraDepManagement() ++
outer.depManagement()
}

/**
* JavaModule and its derivatives define inner test modules.
* To avoid unexpected misbehavior due to the use of the wrong inner test trait
Expand Down Expand Up @@ -165,7 +174,7 @@ trait JavaModule
*/
def bomIvyDeps: T[Agg[Dep]] = Task { Agg.empty[Dep] }

def allBomDeps: Task[Agg[BomDependency]] = Task.Anon {
def allBomIvyDeps: Task[Agg[BomDependency]] = Task.Anon {
val modVerOrMalformed =
bomIvyDeps().map(bindDependency()).map { bomDep =>
val fromModVer = coursier.core.Dependency(bomDep.dep.module, bomDep.dep.version)
Expand All @@ -191,6 +200,23 @@ trait JavaModule
)
}

/**
* BOM dependencies that are not meant to be overridden or changed by users.
*
* This is mainly used to add BOM dependencies of the main module to its test
* modules, while ensuring test dependencies of the BOM are taken into account too
* in the test module.
*/
def extraBomIvyDeps: Task[Agg[BomDependency]] = Task.Anon { Agg.empty[BomDependency] }

/**
* Dependency management entries that are not meant to be overridden or changed by users.
*
* This is mainly used to add dependency management entries of the main module to its test
* modules.
*/
def extraDepManagement: Task[Agg[Dep]] = Task.Anon { Agg.empty[Dep] }
Comment on lines +210 to +218
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these really need to be Task.Anons? Not sure about BomDependency, but at least Agg[Dep] is serializable and can be a normal Task right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be made normal Tasks, yes


/**
* Dependency management data
*
Expand All @@ -206,9 +232,32 @@ trait JavaModule
* ivy"com.lihaoyi::cask:0.9.4".exclude("org.slf4j", "slf4j-api")
* )
* }}}
*
* Versions and exclusions specified here apply to dependencies pulled via
* `ivyDeps`, `compileIvyDeps` (compile-only or "provided" dependencies), and
* `runIvyDeps` (runtime dependencies).
*/
def depManagement: T[Agg[Dep]] = Task { Agg.empty[Dep] }

/**
* Dependency management data for the compile only or "provided" scope.
*
* Versions and exclusions specified here apply only to dependencies pulled via
* `compileIvyDeps`.
*/
def compileDepManagement: T[Agg[Dep]] = Task { Agg.empty[Dep] }

/**
* Dependency management data for the runtime scope.
*
* Versions and exclusions specified here apply only to dependencies pulled via
* `runIvyDeps`.
*
* @param overrideVersions Whether versions in BOMs should override those of the passed
* dependencies themselves
*/
def runDepManagement: T[Agg[Dep]] = Task { Agg.empty[Dep] }

private def addBoms(
bomIvyDeps: Seq[coursier.core.BomDependency],
depMgmt: Seq[(DependencyManagement.Key, DependencyManagement.Values)],
Expand Down Expand Up @@ -455,13 +504,62 @@ trait JavaModule
/**
* Returns a function adding BOM and dependency management details of
* this module to a `coursier.core.Dependency`
*
* Meant to be called on values originating from `ivyDeps`
*/
def processDependency(
overrideVersions: Boolean = false
): Task[coursier.core.Dependency => coursier.core.Dependency] = Task.Anon {
val bomDeps0 = allBomDeps().toSeq.map(_.withConfig(Configuration.compile))
val bomDeps0 =
allBomIvyDeps().toSeq.map(_.withConfig(Configuration.compile)) ++ extraBomIvyDeps().toSeq
val depMgmt = processedDependencyManagement(
(extraDepManagement().toSeq ++ depManagement().toSeq)
.map(bindDependency())
.map(_.dep)
)

addBoms(bomDeps0, depMgmt, overrideVersions = overrideVersions)
Comment on lines +514 to +521
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we refactor processDependency, processCompileDependency, and processRunDependency to share the logic between them via an anonymous task. That will help make it easier to see the core differences between them.

I'm also not clear about the differences here:

  • processCompileDependency uses compileDepManagement(), and processRunDependency uses runDepManagement
  • processDependency and processCompileDependency both use extraDepManagement but processRunDependency does not. Why?
  • Only processDependency seems to use extraBomIvyDeps, while processCompileDependency and processRunDependency do not. Why?

Copy link
Contributor Author

@alexarchambault alexarchambault Dec 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying a different approach to this PR's changes in #4145. Rebasing the PR here on top of it should get rid of these additional methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also simplify the way BOMs are handled when taking scopes into account. I've been having issues reconciling Mill's compileIvyDeps / ivyDeps / runIvyDeps (where scopes are known beforehand) with how scopes are handled in BOMs (not known beforehand, dependencies from all scopes are mixed, and BOMs can change the scope if it's undefined).

}

/**
* Returns a function adding BOM and dependency management details of
* this module to a `coursier.core.Dependency`
*
* Meant to be called on values originating from `compileIvyDeps`
*
* @param overrideVersions Whether versions in BOMs should override those of the passed
* dependencies themselves
*/
def processCompileDependency(
overrideVersions: Boolean = false
): Task[coursier.core.Dependency => coursier.core.Dependency] = Task.Anon {
val bomDeps0 = allBomIvyDeps().toSeq.map(_.withConfig(Configuration.provided))
val depMgmt = processedDependencyManagement(
depManagement().toSeq.map(bindDependency()).map(_.dep)
(compileDepManagement().toSeq ++ extraDepManagement().toSeq ++ depManagement().toSeq)
.map(bindDependency())
.map(_.dep)
)

addBoms(bomDeps0, depMgmt, overrideVersions = overrideVersions)
}

/**
* Returns a function adding BOM and dependency management details of
* this module to a `coursier.core.Dependency`
*
* Meant to be called on values originating from `runIvyDeps`
*
* @param overrideVersions Whether versions in BOMs should override those of the passed
* dependencies themselves
*/
def processRunDependency(
overrideVersions: Boolean = false
): Task[coursier.core.Dependency => coursier.core.Dependency] = Task.Anon {
val bomDeps0 = allBomIvyDeps().toSeq.map(_.withConfig(Configuration.defaultCompile))
val depMgmt = processedDependencyManagement(
(runDepManagement().toSeq ++ depManagement())
.map(bindDependency())
.map(_.dep)
)

addBoms(bomDeps0, depMgmt, overrideVersions = overrideVersions)
Expand All @@ -479,11 +577,38 @@ trait JavaModule
}
}

/**
* The compile-time Ivy dependencies of this module, with BOM and dependency management details
* added to them. This should be used when propagating the dependencies transitively
* to other modules.
*/
def processedCompileIvyDeps: Task[Agg[BoundDep]] = Task.Anon {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make these normal Tasks rather than Task.Anons?

val processDependency0 = processCompileDependency()()
compileIvyDeps().map(bindDependency()).map { dep =>
dep.copy(dep = processDependency0(dep.dep))
}
}

/**
* The runtime Ivy dependencies of this module, with BOM and dependency management details
* added to them. This should be used when propagating the dependencies transitively
* to other modules.
*/
def processedRunIvyDeps: Task[Agg[BoundDep]] = Task.Anon {
val processDependency0 = processRunDependency()()
runIvyDeps().map(bindDependency()).map { dep =>
dep.copy(dep = processDependency0(dep.dep))
}
}

/**
* The transitive ivy dependencies of this module and all it's upstream modules.
* This is calculated from [[ivyDeps]], [[mandatoryIvyDeps]] and recursively from [[moduleDeps]].
*/
def transitiveIvyDeps: T[Agg[BoundDep]] = Task {
// overrideVersions is true for dependencies originating from moduleDeps,
// as these are transitive (the direct dependency is the one in moduleDeps),
// so versions in BOMs should override their versions
val processDependency0 = processDependency(overrideVersions = true)()
processedIvyDeps() ++
T.traverse(moduleDepsChecked)(_.transitiveIvyDeps)().flatten.map { dep =>
Expand All @@ -493,20 +618,35 @@ trait JavaModule

/** The compile-only transitive ivy dependencies of this module and all it's upstream compile-only modules. */
def transitiveCompileIvyDeps: T[Agg[BoundDep]] = Task {
// overrideVersions is true for dependencies originating from moduleDeps,
// as these are transitive (the direct dependency is the one in moduleDeps),
// so versions in BOMs should override their versions
val processDependency0 = processCompileDependency(overrideVersions = true)()
// We never include compile-only dependencies transitively, but we must include normal transitive dependencies!
compileIvyDeps().map(bindDependency()) ++
T.traverse(compileModuleDepsChecked)(_.transitiveIvyDeps)().flatten
processedCompileIvyDeps() ++
T.traverse(compileModuleDepsChecked)(_.transitiveIvyDeps)().flatten.map { dep =>
dep.copy(dep = processDependency0(dep.dep))
}
}

/**
* The transitive run ivy dependencies of this module and all it's upstream modules.
* This is calculated from [[runIvyDeps]], [[mandatoryIvyDeps]] and recursively from [[moduleDeps]].
*/
def transitiveRunIvyDeps: T[Agg[BoundDep]] = Task {
runIvyDeps().map(bindDependency()) ++
T.traverse(moduleDepsChecked)(_.transitiveRunIvyDeps)().flatten ++
T.traverse(runModuleDepsChecked)(_.transitiveIvyDeps)().flatten ++
T.traverse(runModuleDepsChecked)(_.transitiveRunIvyDeps)().flatten
// overrideVersions is true for dependencies originating from moduleDeps,
// as these are transitive (the direct dependency is the one in moduleDeps),
// so versions in BOMs should override their versions
val processDependency0 = processRunDependency(overrideVersions = true)()
processedRunIvyDeps() ++ {
val viaModules =
T.traverse(moduleDepsChecked)(_.transitiveRunIvyDeps)().flatten ++
T.traverse(runModuleDepsChecked)(_.transitiveIvyDeps)().flatten ++
T.traverse(runModuleDepsChecked)(_.transitiveRunIvyDeps)().flatten
viaModules.map { dep =>
dep.copy(dep = processDependency0(dep.dep))
}
}
}

/**
Expand Down
53 changes: 47 additions & 6 deletions scalalib/src/mill/scalalib/PublishModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,14 @@ trait PublishModule extends JavaModule { outer =>
* Dependency management to specify in the POM
*/
def publishXmlDepMgmt: Task[Agg[Dependency]] = Task.Anon {
depManagement().map(resolvePublishDependency.apply().apply(_))
(extraDepManagement() ++ depManagement())
.map(resolvePublishDependency.apply().apply(_)) ++
compileDepManagement()
.map(resolvePublishDependency.apply().apply(_))
.map(_.copy(scope = Scope.Provided)) ++
runDepManagement()
.map(resolvePublishDependency.apply().apply(_))
.map(_.copy(scope = Scope.Runtime))
}

def pom: T[PathRef] = Task {
Expand All @@ -137,9 +144,10 @@ trait PublishModule extends JavaModule { outer =>
def bomDetails: T[(Map[coursier.core.Module, String], coursier.core.DependencyManagement.Map)] =
Task {
val (processedDeps, depMgmt) = defaultResolver().processDeps(
processedIvyDeps(),
processedRunIvyDeps() ++ processedIvyDeps(),
resolutionParams = resolutionParams(),
boms = allBomDeps().toSeq.map(_.withConfig(Configuration.compile))
boms = allBomIvyDeps().toSeq.map(_.withConfig(Configuration.defaultCompile)) ++
extraBomIvyDeps().toSeq
)
(processedDeps.map(_.moduleVersion).toMap, depMgmt)
}
Expand All @@ -163,24 +171,57 @@ trait PublishModule extends JavaModule { outer =>
else
dep
}
val bomDepMgmt0 = {
// Ensure we don't override versions of root dependencies with overrides from the BOM
val rootDepsAdjustment = publishXmlDeps0.iterator.flatMap { dep =>
val key = coursier.core.DependencyManagement.Key(
coursier.core.Organization(dep.artifact.group),
coursier.core.ModuleName(dep.artifact.id),
coursier.core.Type.jar,
coursier.core.Classifier.empty
)
bomDepMgmt.get(key).flatMap { values =>
if (values.version.nonEmpty && values.version != dep.artifact.version)
Some(key -> values.withVersion(""))
else
None
}
}
bomDepMgmt ++ rootDepsAdjustment
}
lazy val moduleSet = publishXmlDeps0.map(dep => (dep.artifact.group, dep.artifact.id)).toSet
val overrides = {
val depMgmtEntries = processedDependencyManagement(
depManagement().toSeq
(extraDepManagement().toSeq ++ depManagement().toSeq)
.map(bindDependency())
.map(_.dep)
.filter(_.version.nonEmpty)
.filter { depMgmt =>
// Ensure we don't override versions of root dependencies with overrides from the BOM
!moduleSet.contains((depMgmt.module.organization.value, depMgmt.module.name.value))
}
)
val entries = coursier.core.DependencyManagement.add(
Map.empty,
depMgmtEntries ++ bomDepMgmt
depMgmtEntries ++ bomDepMgmt0
.filter {
case (key, _) =>
// Ensure we don't override versions of root dependencies with overrides from the BOM
!moduleSet.contains((key.organization.value, key.name.value))
}
)
entries.toVector
.map {
case (key, values) =>
val confString =
// we pull the runtime dependency of runtime dependencies, like Maven does
if (values.config == Configuration.runtime) "runtime->runtime"
else ""
Ivy.Override(
key.organization.value,
key.name.value,
values.version
values.version,
confString
)
}
.sortBy(value => (value.organization, value.name, value.version))
Expand Down
8 changes: 6 additions & 2 deletions scalalib/src/mill/scalalib/publish/Ivy.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ object Ivy {

val head = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"

case class Override(organization: String, name: String, version: String)
case class Override(organization: String, name: String, version: String, confString: String)

def apply(
artifact: Artifact,
Expand Down Expand Up @@ -89,7 +89,11 @@ object Ivy {
}

private def renderOverride(override0: Override): Elem =
<override org={override0.organization} module={override0.name} rev={override0.version} />
if (override0.confString.isEmpty)
<override org={override0.organization} module={override0.name} rev={override0.version} />
else
<override org={override0.organization} module={override0.name} rev={override0.version}
conf={override0.confString} />

private def depIvyConf(d: Dependency): String = {
def target(value: String) = d.configuration.getOrElse(value)
Expand Down
Loading
Loading