Skip to content

Commit

Permalink
Break the dependency between repositoriesTask and ivyDeps
Browse files Browse the repository at this point in the history
mill-scalablytyped (but possibly other plugins or user setups too) adds
a dependency on `repositoriesTask` from `ivyDeps`. Since the introduction
of `JavaModule#coursierProject`, Mill had a dependency the other way around,
`repositoriesTask` depends on `ivyDeps`.

This creates a cycle, leading to StackOverflowException-s.

In order to work around that, this PR splits both `repositoriesTask` and
`defaultResolver`, adding:
- `allRepositoriesTask`: basically `repositoriesTask`, with the Mill internal
  repository added
- `internalResolver`: same as `defaultResolver`, with the Mill internal
  repository added (via `allRepositoriesTask`)

If users need to resolve purely external modules (most common case it seems),
they can keep using `repositoriesTask` or `defaultResolver`.

If they need to resolve some Mill internal modules (usually brought in
via `JavaModule#coursierDependency`), they now need to use `allRepositoriesTask`
and `internalResolver` instead of `repositoriesTask` and `defaultResolver`.

That way, no cycle is introduced when users only need to resolve external
modules.

Fixes com-lihaoyi#4457
  • Loading branch information
alexarchambault committed Feb 3, 2025
1 parent f540871 commit cdfadf2
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 12 deletions.
4 changes: 2 additions & 2 deletions bsp/worker/src/mill/bsp/worker/MillBuildServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -296,15 +296,15 @@ private class MillBuildServer(
case m: JavaModule =>
Task.Anon {
(
m.defaultResolver().resolveDeps(
m.internalResolver().resolveDeps(
Seq(
m.coursierDependency.withConfiguration(coursier.core.Configuration.provided),
m.coursierDependency
),
sources = true
),
m.unmanagedClasspath(),
m.repositoriesTask()
m.allRepositoriesTask()
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion contrib/bloop/src/mill/contrib/bloop/BloopImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ class BloopImpl(evs: () => Seq[Evaluator], wd: os.Path) extends ExternalModule {
}

val bloopResolution: Task[BloopConfig.Resolution] = Task.Anon {
val repos = module.repositoriesTask()
val repos = module.allRepositoriesTask()
// same as input of resolvedIvyDeps
val coursierDeps = Seq(
module.coursierDependency.withConfiguration(coursier.core.Configuration.provided),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ trait ScalaPBModule extends ScalaModule {
}

def scalaPBProtoClasspath: T[Agg[PathRef]] = Task {
defaultResolver().resolveDeps(
internalResolver().resolveDeps(
Seq(
coursierDependency.withConfiguration(coursier.core.Configuration.provided),
coursierDependency
Expand Down
2 changes: 1 addition & 1 deletion idea/src/mill/idea/GenIdeaImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ case class GenIdeaImpl(
GenIdeaException(
s"Failure during resolving repositories: ${Evaluator.formatFailing(r)}"
)
)(modules.map(_._2.repositoriesTask))
)(modules.map(_._2.allRepositoriesTask))
}
Lib.resolveMillBuildDeps(moduleRepos.flatten, Option(ctx), useSources = true)
Lib.resolveMillBuildDeps(moduleRepos.flatten, Option(ctx), useSources = false)
Expand Down
48 changes: 47 additions & 1 deletion scalalib/src/mill/scalalib/CoursierModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,34 @@ trait CoursierModule extends mill.Module {
Lib.depToDependencyJava(_: Dep)
}

/**
* A `CoursierModule.Resolver` to resolve dependencies.
*
* Unlike `defaultResolver`, this resolver can resolve Mill internal modules
* (obtained via `JavaModule#coursierDependency`).
*
* @return `CoursierModule.Resolver` instance
*/
def internalResolver: Task[CoursierModule.Resolver] = Task.Anon {
new CoursierModule.Resolver(
repositories = allRepositoriesTask(),
bind = bindDependency(),
mapDependencies = Some(mapDependencies()),
customizer = resolutionCustomizer(),
coursierCacheCustomizer = coursierCacheCustomizer(),
ctx = Some(implicitly[mill.api.Ctx.Log]),
resolutionParams = resolutionParams()
)
}

/**
* A `CoursierModule.Resolver` to resolve dependencies.
*
* Can be used to resolve external dependencies, if you need to download an external
* tool from Maven or Ivy repositories, by calling `CoursierModule.Resolver#resolveDeps`.
*
* @return `CoursierModule.Resolver` instance
*/
def defaultResolver: Task[CoursierModule.Resolver] = Task.Anon {
new CoursierModule.Resolver(
repositories = repositoriesTask(),
Expand Down Expand Up @@ -94,14 +122,32 @@ trait CoursierModule extends mill.Module {

/**
* The repositories used to resolved dependencies with [[resolveDeps()]].
*
* See [[allRepositoriesTask]] if you need to resolve Mill internal modules.
*/
def repositoriesTask: Task[Seq[Repository]] = Task.Anon {
val resolve = Resolve()
val repos = Await.result(
resolve.finalRepositories.future()(resolve.cache.ec),
Duration.Inf
)
internalRepositories() ++ repos
repos
}

/**
* The repositories used to resolved dependencies
*
* Unlike [[repositoriesTask]], this includes the Mill internal repositories,
* which allow to resolve Mill internal modules (usually brought in via
* `JavaModule#coursierDependency`).
*
* Beware that this needs to evaluate `JavaModule#coursierProject` of all
* module dependencies of the current module, which itself evaluates `JavaModule#ivyDeps`
* and related tasks. You shouldn't depend on this task from implementations of `ivyDeps`,
* which would introduce cycles between Mill tasks.
*/
def allRepositoriesTask: Task[Seq[Repository]] = Task.Anon {
internalRepositories() ++ repositoriesTask()
}

/**
Expand Down
12 changes: 6 additions & 6 deletions scalalib/src/mill/scalalib/JavaModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ trait JavaModule
override def resources = super[JavaModule].resources
override def moduleDeps: Seq[JavaModule] = Seq(outer)
override def repositoriesTask: Task[Seq[Repository]] = Task.Anon {
internalRepositories() ++ outer.repositoriesTask()
outer.repositoriesTask()
}
override def resolutionCustomizer: Task[Option[coursier.Resolution => coursier.Resolution]] =
outer.resolutionCustomizer
Expand Down Expand Up @@ -1001,7 +1001,7 @@ trait JavaModule
* Resolved dependencies
*/
def resolvedIvyDeps: T[Agg[PathRef]] = Task {
defaultResolver().resolveDeps(
internalResolver().resolveDeps(
Seq(
BoundDep(
coursierDependency.withConfiguration(cs.Configuration.provided),
Expand All @@ -1024,7 +1024,7 @@ trait JavaModule
}

def resolvedRunIvyDeps: T[Agg[PathRef]] = Task {
defaultResolver().resolveDeps(
internalResolver().resolveDeps(
Seq(
BoundDep(
coursierDependency.withConfiguration(cs.Configuration.runtime),
Expand Down Expand Up @@ -1222,7 +1222,7 @@ trait JavaModule
val dependencies =
(additionalDeps() ++ Seq(BoundDep(coursierDependency, force = false))).iterator.to(Seq)
val resolution: Resolution = Lib.resolveDependenciesMetadataSafe(
repositoriesTask(),
allRepositoriesTask(),
dependencies,
Some(mapDependencies()),
customizer = resolutionCustomizer(),
Expand Down Expand Up @@ -1465,7 +1465,7 @@ trait JavaModule
val tasks =
if (all.value) Seq(
Task.Anon {
defaultResolver().resolveDeps(
internalResolver().resolveDeps(
Seq(
coursierDependency.withConfiguration(cs.Configuration.provided),
coursierDependency
Expand All @@ -1478,7 +1478,7 @@ trait JavaModule
)
},
Task.Anon {
defaultResolver().resolveDeps(
internalResolver().resolveDeps(
Seq(coursierDependency.withConfiguration(cs.Configuration.runtime)),
sources = true
)
Expand Down

0 comments on commit cdfadf2

Please sign in to comment.