Skip to content

Commit

Permalink
Remove Agg in favor of Seq (#4525)
Browse files Browse the repository at this point in the history
Fixes #3587

* In most scenarios `Seq` is fine. 
* `Loose.Agg.Mutable` can be safely replaced by
`collection.mutable.LinkedHashSet`, which behaves the same
* `Strict.Agg` can be safely replaced by `Seq`, since any divergence
from `Seq` behavior would throw an exception today. There is a slight
loss in safety (e.g. if future bugs cause duplicates to occur) and with
a slight gain in performance (since we no longer maintain two duplicate
`Set`/`Seq` data structures
* We leave a `type Agg[+T] = Seq[T]; val Agg = Seq` alias in the
`package.scala` file to ease in the migration. This lets downstream
users continue to use the name `Agg` in an _almost_ source compatible
way (exception: `Agg.when` doesn't have a `Seq.when` equivalent), and
also means we don't need to update `ci/mill-bootstrap.patch` due to
source compatibility
  • Loading branch information
lihaoyi authored Feb 11, 2025
1 parent 147e0f1 commit 15f55d8
Show file tree
Hide file tree
Showing 276 changed files with 1,266 additions and 1,445 deletions.
4 changes: 2 additions & 2 deletions bsp/src/mill/bsp/BSP.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package mill.bsp

import mill.api.{Ctx, PathRef}
import mill.{Agg, T, Task, given}
import mill.{T, Task, given}
import mill.define.{Command, Discover, ExternalModule}
import mill.main.BuildInfo
import mill.eval.Evaluator
Expand All @@ -12,7 +12,7 @@ object BSP extends ExternalModule with CoursierModule {

lazy val millDiscover = Discover[this.type]

private def bspWorkerLibs: T[Agg[PathRef]] = Task {
private def bspWorkerLibs: T[Seq[PathRef]] = Task {
millProjectModule("mill-bsp-worker", repositoriesTask())
}

Expand Down
19 changes: 6 additions & 13 deletions bsp/worker/src/mill/bsp/worker/MillBuildServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,8 @@ package mill.bsp.worker
import ch.epfl.scala.bsp4j
import ch.epfl.scala.bsp4j._
import com.google.gson.JsonObject
import mill.api.Loose.Agg
import mill.api.{
ColorLogger,
CompileProblemReporter,
DummyTestReporter,
Result,
Strict,
TestReporter
}

import mill.api.{ColorLogger, CompileProblemReporter, DummyTestReporter, Result, TestReporter}
import mill.bsp.{BspServerResult, Constants}
import mill.bsp.worker.Utils.{makeBuildTarget, outputPaths, sanitizeUri}
import mill.define.Segment.Label
Expand Down Expand Up @@ -479,7 +472,7 @@ private class MillBuildServer(
val runTask = module.run(Task.Anon(Args(args)))
val runResult = evaluate(
ev,
Strict.Agg(runTask),
Seq(runTask),
Utils.getBspLoggedReporterPool(runParams.getOriginId, state.bspIdByModule, client),
logger = new MillBspLogger(client, runTask.hashCode(), ev.baseLogger)
)
Expand Down Expand Up @@ -544,7 +537,7 @@ private class MillBuildServer(

val results = evaluate(
ev,
Strict.Agg(testTask),
Seq(testTask),
Utils.getBspLoggedReporterPool(
testParams.getOriginId,
state.bspIdByModule,
Expand Down Expand Up @@ -605,7 +598,7 @@ private class MillBuildServer(
val cleanTask = mainModule.clean(ev, Seq(compileTargetName)*)
val cleanResult = evaluate(
ev,
Strict.Agg(cleanTask),
Seq(cleanTask),
logger = new MillBspLogger(client, cleanTask.hashCode, ev.baseLogger)
)
if (cleanResult.failing.size > 0) (
Expand Down Expand Up @@ -807,7 +800,7 @@ private class MillBuildServer(

private def evaluate(
evaluator: Evaluator,
goals: Agg[Task[?]],
goals: Seq[Task[?]],
reporter: Int => Option[CompileProblemReporter] = _ => Option.empty[CompileProblemReporter],
testReporter: TestReporter = DummyTestReporter,
logger: ColorLogger = null
Expand Down
8 changes: 4 additions & 4 deletions bsp/worker/src/mill/bsp/worker/MillScalaBuildServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import ch.epfl.scala.bsp4j.{
ScalacOptionsParams,
ScalacOptionsResult
}
import mill.{Agg, Task}
import mill.Task
import mill.bsp.worker.Utils.sanitizeUri
import mill.util.Jvm
import mill.scalalib.{JavaModule, ScalaModule, TestModule, UnresolvedPath}
Expand Down Expand Up @@ -41,7 +41,7 @@ private trait MillScalaBuildServer extends ScalaBuildServer { this: MillBuildSer
val compileClasspathTask =
if (enableJvmCompileClasspathProvider) {
// We have a dedicated request for it
Task.Anon { Agg.empty[UnresolvedPath] }
Task.Anon { Seq.empty[UnresolvedPath] }
} else {
m.bspCompileClasspath
}
Expand Down Expand Up @@ -119,7 +119,7 @@ private trait MillScalaBuildServer extends ScalaBuildServer { this: MillBuildSer
}
) {
case (ev, state, id, m: TestModule, Some((classpath, testFramework, testClasspath))) =>
val (frameworkName, classFingerprint): (String, Agg[(Class[?], Fingerprint)]) =
val (frameworkName, classFingerprint): (String, Seq[(Class[?], Fingerprint)]) =
Jvm.withClassLoader(
classPath = classpath.map(_.path).toVector,
sharedPrefixes = Seq("sbt.testing.")
Expand All @@ -128,7 +128,7 @@ private trait MillScalaBuildServer extends ScalaBuildServer { this: MillBuildSer
val discoveredTests = TestRunnerUtils.discoverTests(
classLoader,
framework,
Agg.from(testClasspath.map(_.path))
Seq.from(testClasspath.map(_.path))
)
(framework.name(), discoveredTests)
}: @unchecked
Expand Down
33 changes: 30 additions & 3 deletions ci/mill-bootstrap.patch
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
diff --git a/build.mill b/build.mill
index 7a8ea3a31a5..5b77aa31591 100644
index 019adfc8cb9..c9df11158ef 100644
--- a/build.mill
+++ b/build.mill
@@ -1,16 +1,16 @@
Expand Down Expand Up @@ -65,7 +65,25 @@ index 7a8ea3a31a5..5b77aa31591 100644
def scalaVersion = Deps.scalaVersion
def scalapVersion: T[String] = Deps.scala2Version
def scalafixScalaBinaryVersion = T {
@@ -535,7 +535,7 @@ trait MillScalaModule extends ScalaModule with MillJavaModule with ScalafixModul
@@ -520,8 +520,8 @@ trait MillScalaModule extends ScalaModule with MillJavaModule with ScalafixModul
val binaryVersion = ZincWorkerUtil.scalaBinaryVersion(sv)
val hasModuleDefs = binaryVersion == "2.13" || binaryVersion == "3"
super.scalacPluginIvyDeps() ++
- Agg.when(binaryVersion != "3")(Deps.acyclic) ++
- Agg.when(hasModuleDefs)(Deps.millModuledefsPlugin)
+ Option.when(binaryVersion != "3")(Deps.acyclic) ++
+ Option.when(hasModuleDefs)(Deps.millModuledefsPlugin)
}

def mandatoryIvyDeps = T {
@@ -529,13 +529,13 @@ trait MillScalaModule extends ScalaModule with MillJavaModule with ScalafixModul
val binaryVersion = ZincWorkerUtil.scalaBinaryVersion(sv)
val hasModuleDefs = binaryVersion == "2.13" || binaryVersion == "3"
super.mandatoryIvyDeps() ++
- Agg.when(hasModuleDefs)(Deps.millModuledefs)
+ Option.when(hasModuleDefs)(Deps.millModuledefs)
}

/** Default tests module. */
lazy val test: MillScalaTests = new MillScalaTests {}
trait MillScalaTests extends ScalaTests with MillJavaModule with MillBaseTestsModule
Expand Down Expand Up @@ -94,7 +112,7 @@ index 7a8ea3a31a5..5b77aa31591 100644

object bridge extends Cross[BridgeModule](compilerBridgeScalaVersions)
diff --git a/contrib/package.mill b/contrib/package.mill
index 421b2955955..7277bba90ff 100644
index 421b2955955..30c453234b9 100644
--- a/contrib/package.mill
+++ b/contrib/package.mill
@@ -3,13 +3,12 @@ package build.contrib
Expand Down Expand Up @@ -141,6 +159,15 @@ index 421b2955955..7277bba90ff 100644
}

def scalaVersion = build.Deps.play(playBinary).scalaVersion
@@ -139,7 +138,7 @@ object `package` extends RootModule {
build.Deps.scalacScoverage2Reporter,
build.Deps.scalacScoverage2Domain,
build.Deps.scalacScoverage2Serializer
- ) ++ Agg.when(!ZincWorkerUtil.isScala3(scalaVersion()))(build.Deps.scalacScoverage2Plugin)
+ ) ++ Option.when(!ZincWorkerUtil.isScala3(scalaVersion()))(build.Deps.scalacScoverage2Plugin)
}
def mandatoryIvyDeps = Agg.empty[Dep]
}
diff --git a/core/codesig/package.mill b/core/codesig/package.mill
index e49f218f4be..9d03aa073f3 100644
--- a/core/codesig/package.mill
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 @@ -191,7 +191,7 @@ class BloopImpl(evs: () => Seq[Evaluator], wd: os.Path) extends ExternalModule {

val classpath = Task.Anon {
val transitiveCompileClasspath = Task.traverse(module.transitiveModuleCompileModuleDeps)(m =>
Task.Anon { m.localCompileClasspath().map(_.path) ++ Agg(classes(m)) }
Task.Anon { m.localCompileClasspath().map(_.path) ++ Seq(classes(m)) }
)().flatten

module.resolvedIvyDeps().map(_.path) ++
Expand Down
6 changes: 3 additions & 3 deletions contrib/bloop/test/src/mill/contrib/bloop/BloopTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,18 @@ object BloopTests extends TestSuite {
val bloopVersion = mill.contrib.bloop.Versions.bloop
override def mainClass = Some("foo.bar.Main")

override def ivyDeps = Agg(
override def ivyDeps = Seq(
ivy"ch.epfl.scala::bloop-config:$bloopVersion"
)
override def scalacOptions = Seq(
"-language:higherKinds"
)

override def compileIvyDeps = Agg(
override def compileIvyDeps = Seq(
ivy"org.reactivestreams:reactive-streams:1.0.3"
)

override def runIvyDeps = Agg(
override def runIvyDeps = Seq(
ivy"org.postgresql:postgresql:42.3.3"
)

Expand Down
2 changes: 1 addition & 1 deletion contrib/flyway/readme.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ object foo extends ScalaModule with FlywayModule {
//region flyway
def flywayUrl = "jdbc:postgresql:myDb" // required
def flywayDriverDeps = Agg(ivy"org.postgresql:postgresql:42.2.5") // required
def flywayDriverDeps = Seq(ivy"org.postgresql:postgresql:42.2.5") // required
def flywayUser = "postgres" // optional
// def flywayPassword = "" // optional
//endregion
Expand Down
4 changes: 2 additions & 2 deletions contrib/flyway/src/mill/contrib/flyway/FlywayModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import org.flywaydb.core.internal.configuration.{ConfigUtils => flyway}
import org.flywaydb.core.internal.info.MigrationInfoDumper
import scala.jdk.CollectionConverters._

import mill.{Agg, T, Task}
import mill.{T, Task}
import mill.api.PathRef
import mill.define.Command
import mill.scalalib.{Dep, JavaModule}
Expand All @@ -26,7 +26,7 @@ trait FlywayModule extends JavaModule {
resources().map(pr => PathRef(pr.path / "db/migration", pr.quick))
}

def flywayDriverDeps: T[Agg[Dep]]
def flywayDriverDeps: T[Seq[Dep]]

def jdbcClasspath = Task {
defaultResolver().resolveDeps(flywayDriverDeps())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ object BuildTest extends TestSuite {
def h2 = ivy"com.h2database:h2:2.1.214"

def flywayUrl = "jdbc:h2:mem:test_db;DB_CLOSE_DELAY=-1"
def flywayDriverDeps = Agg(h2)
def flywayDriverDeps = Seq(h2)
}

val millDiscover: Discover = Discover[this.type]
Expand Down
4 changes: 2 additions & 2 deletions contrib/jmh/src/mill/contrib/jmh/JmhModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ trait JmhModule extends JavaModule {
def jmhCoreVersion: T[String]
def jmhGeneratorByteCodeVersion: T[String] = jmhCoreVersion

def ivyDeps = super.ivyDeps() ++ Agg(ivy"org.openjdk.jmh:jmh-core:${jmhCoreVersion()}")
def ivyDeps = super.ivyDeps() ++ Seq(ivy"org.openjdk.jmh:jmh-core:${jmhCoreVersion()}")

def runJmh(args: String*) =
Task.Command {
Expand Down Expand Up @@ -105,7 +105,7 @@ trait JmhModule extends JavaModule {

def generatorDeps = Task {
defaultResolver().resolveDeps(
Agg(ivy"org.openjdk.jmh:jmh-generator-bytecode:${jmhGeneratorByteCodeVersion()}")
Seq(ivy"org.openjdk.jmh:jmh-generator-bytecode:${jmhGeneratorByteCodeVersion()}")
)
}
}
4 changes: 2 additions & 2 deletions contrib/playlib/readme.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ The following compile dependencies will automatically be added to your build:

[source,scala]
----
Agg(
Seq(
ivy"com.typesafe.play::play:${playVersion()}",
ivy"com.typesafe.play::play-guice:${playVersion()}",
ivy"com.typesafe.play::play-server:${playVersion()}",
Expand Down Expand Up @@ -162,7 +162,7 @@ object core extends PlayApiModule {
object test extends PlayTests
override def ivyDeps = Task { super.ivyDeps() ++ Agg(ws(), filters()) }
override def ivyDeps = Task { super.ivyDeps() ++ Seq(ws(), filters()) }
}
----

Expand Down
4 changes: 2 additions & 2 deletions contrib/playlib/src/mill/playlib/Dependencies.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package mill.playlib

import mill.{Agg, Task}
import mill.Task
import mill.scalalib._

private[playlib] trait Dependencies extends ScalaModule with Version {
Expand All @@ -15,7 +15,7 @@ private[playlib] trait Dependencies extends ScalaModule with Version {
def caffeine = Task { component("play-caffeine-cache")() }

override def ivyDeps = Task {
super.ivyDeps() ++ Agg(
super.ivyDeps() ++ Seq(
core(),
guice(),
server(),
Expand Down
4 changes: 2 additions & 2 deletions contrib/playlib/src/mill/playlib/PlayModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package mill.playlib
import mill.define.Task
import mill.playlib.api.Versions
import mill.scalalib._
import mill.{Agg, Args, T}
import mill.{Args, T}
import mill.api.PathRef
import mill.define.Target

Expand All @@ -17,7 +17,7 @@ trait PlayApiModule extends Dependencies with Router with Server {
case Versions.PLAY_2_9 => "6.0.0"
case _ => "7.0.0"
}
Agg(ivy"org.scalatestplus.play::scalatestplus-play::${scalatestPlusPlayVersion}")
Seq(ivy"org.scalatestplus.play::scalatestplus-play::${scalatestPlusPlayVersion}")
}
override def sources: Target[Seq[PathRef]] = Task.Sources { moduleDir }
}
Expand Down
6 changes: 3 additions & 3 deletions contrib/playlib/src/mill/playlib/RouteCompilerWorker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ package mill.playlib
import mill.api.{Ctx, PathRef, Result}
import mill.playlib.api.{RouteCompilerType, RouteCompilerWorkerApi}
import mill.scalalib.api.CompilationResult
import mill.{Agg, Task}
import mill.Task

private[playlib] class RouteCompilerWorker extends AutoCloseable {

private var routeCompilerInstanceCache =
Option.empty[(Long, mill.playlib.api.RouteCompilerWorkerApi)]

protected def bridge(toolsClasspath: Agg[PathRef])(
protected def bridge(toolsClasspath: Seq[PathRef])(
implicit ctx: Ctx
): RouteCompilerWorkerApi = {
val classloaderSig = toolsClasspath.hashCode
Expand All @@ -35,7 +35,7 @@ private[playlib] class RouteCompilerWorker extends AutoCloseable {
}

def compile(
routerClasspath: Agg[PathRef],
routerClasspath: Seq[PathRef],
files: Seq[os.Path],
additionalImports: Seq[String],
forwardsRouter: Boolean,
Expand Down
10 changes: 5 additions & 5 deletions contrib/playlib/src/mill/playlib/RouterModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import mill.util.MillModuleUtil.millProjectModule
import mill.playlib.api.RouteCompilerType
import mill.scalalib._
import mill.scalalib.api._
import mill.{Agg, T, Task}
import mill.{T, Task}

trait RouterModule extends ScalaModule with Version {

Expand Down Expand Up @@ -45,15 +45,15 @@ trait RouterModule extends ScalaModule with Version {
*/
def generatorType: RouteCompilerType = RouteCompilerType.InjectedGenerator

def routerClasspath: T[Agg[PathRef]] = Task {
def routerClasspath: T[Seq[PathRef]] = Task {
defaultResolver().resolveDeps(
playMinorVersion() match {
case "2.6" | "2.7" | "2.8" =>
Agg(ivy"com.typesafe.play::routes-compiler:${playVersion()}")
Seq(ivy"com.typesafe.play::routes-compiler:${playVersion()}")
case "2.9" =>
Agg(ivy"com.typesafe.play::play-routes-compiler:${playVersion()}")
Seq(ivy"com.typesafe.play::play-routes-compiler:${playVersion()}")
case _ =>
Agg(ivy"org.playframework::play-routes-compiler:${playVersion()}")
Seq(ivy"org.playframework::play-routes-compiler:${playVersion()}")
}
)
}
Expand Down
4 changes: 2 additions & 2 deletions contrib/playlib/src/mill/playlib/Server.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package mill.playlib

import mill.scalalib._
import mill.{Agg, Task}
import mill.Task

private[playlib] trait Server extends ScalaModule with Version {

Expand All @@ -19,7 +19,7 @@ private[playlib] trait Server extends ScalaModule with Version {
}

override def runIvyDeps = Task {
super.runIvyDeps() ++ Agg(playServerProvider())
super.runIvyDeps() ++ Seq(playServerProvider())
}

override def mainClass = Task { Some("play.core.server.ProdServerStart") }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ object PlayModuleTests extends TestSuite with PlayTestSuite {
override def playVersion = crossPlayVersion
override def scalaVersion = crossScalaVersion
object test extends PlayTests
override def ivyDeps = Task { super.ivyDeps() ++ Agg(ws()) }
override def ivyDeps = Task { super.ivyDeps() ++ Seq(ws()) }
}

lazy val millDiscover = Discover[this.type]
Expand Down
Loading

0 comments on commit 15f55d8

Please sign in to comment.