Skip to content

Commit 33fa953

Browse files
authored
Support module.sc files in subfolders (#3213)
This PR implements support for per-subfolder `build.sc` files, named `module.sc`. This allows large builds to be split up into multiple `build.sc` files each relevant to the sub-folder that they are placed in, rather than having a multi-thousand-line `build.sc` in the root configuring Mill for the entire codebase. ## Semantics 1. The `build.sc` in the project root and all nested `module.sc` files are recursively within the Mill directory tree are discovered (TODO ignoring files listed in `.millignore`), and compiled together 2. We ignore any subfolders that have their own `build.sc` file, indicating that they are the root of their own project and not part of the enclosing folder. 3. Each `foo/bar/qux/build.sc` file is compiled into a `millbuild.foo.bar.qux` `package object`, with the `build.sc` and `module.sc` files being compiled into a `millbuild` `package object` (rather than a plain `object` in the status quo) 4. An `object blah extends Module` within each `foo/bar/qux/build.sc` file can be referenced in code via `foo.bar.qux.blah`, or referenced from the command line via `foo.bar.qux.blah` 5. The base modules of `module.sc` files do not have the `MainModule` tasks: `init`, `version`, `clean`, etc.. Only the base module of the root `build.sc` file has those ## Design ### Uniform vs Non-uniform hierarchy One design decision here is whether a `module.sc` file in a subfolder `foo/bar/` containing `object qux{ def baz }` would have their targets referenced via `foo.bar.qux.baz` syntax, or via some alternative e.g. `foo/bar/qux.baz`. A non-uniform hierarchy `foo/bar/qux.baz` would be similar to how Bazel treats folders v.s. targets non-uniformly `foo/bar:qux-baz`, and also similar to how external modules in Mill are handled e.g. `mill.idea.GenIdea/idea`, as well as existing foreign modules. However, it introduces significant user-facing complexity: 1. What's the difference between `foo/bar/qux.baz` vs `foo/bar.qux.baz` or `foo/bar/qux/baz`? 2. What query syntax would we use to query targets in all nested `module.sc` files rather than just the top-level one e.g. `__.compile`? 3. Would there be a correspondingly different way of referencing nested `module.sc` modules and targets in Scala code as well? Bazel has significant complexity to handle these cases, e.g. query via `...` vs `:all` vs `*`. It works, but it does complicate the user-facing semantics. The alternative of a uniform hierarchy also has downsides: 1. How do you go from a module name e.g. `foo.bar.qux.baz` to the `build.sc` or `module.sc` file in which it is defined? 2. If a module is defined in both the root `build.sc` and in a nested `module.sc`, what happens? I decided to go with a uniform hierarchy where everything, both in top-level `build.sc` and in nested `module.sc`, end up squashed together in a single uniform `foo.bar.qux.baz` hierarchy. ### Package Objects The goal of this is to try and make modules defined in nested `module.sc` files "look the same" as modules defined in the root `build.sc`. There are two possible approaches: 1. Splice the source code of the various nested `module.sc` files into the top-level `object build`. This is possible, but very complex and error prone. Especially when it comes to reporting proper error locations in stack traces (filename/linenumber), this will likely require a custom compiler plugin similar to the `LineNumberPlugin` we have today 5. Convert the `object`s into `package object`s, such that module tree defined in the root `build.sc` becomes synonymous with the JVM package tree. While the `package object`s will cause the compiler to synthesize `object package { ... }` wrappers, that is mostly hidden from the end user. I decided to go with (2) because it seemed much simpler, making use of existing language features rather than trying to force the behavior we want using compiler hackery. Although `package object`s may go away at some point in Scala 3, they should be straightforward to replace with explicit `export foo.*` statements when that time comes. ### Existing Foreign Modules Mill already supports existing `foo.sc` files which support targets and modules being defined within them, but does not support referencing them from the command line. I have removed the ability to define targets and modules in random `foo.sc` files. We should encourage people to put things in `module.sc`, since that would allow the user to co-locate the build logic within the folder containing the files it is related to, rather than as a bunch of loose `foo.sc` scripts. Removing support for modules/targets in `foo.sc` files greatly simplifies the desugaring of these scripts, and since we are already making a breaking change by overhauling how per-folder `module.sc` files work we might as well bundle this additional breakage together (rather than making multiple breaking changes in series) ### `build.sc`/`module.sc` file discovery For this implementation, I chose to make `module.sc` files discovered automatically by traversing the filesystem: we recursively walk the subfolders of the root `build.sc` project, look for any files named `module.sc`. We only traverse folders with `module.sc` files to avoid having to traverse the entire filesystem structure every time. Empty `module.sc` files can be used as necessary to allow `module.sc` files to be placed deeper in the folder tree This matches the behavior of Bazel and SBT in discovering their `BUILD`/`build.sbt` files, and notably goes against Maven/Gradle which require submodules/subprojects to be declared in the top level build config. This design has the following characteristics: 1. In future, if we wish to allow `mill` invocations from within a subfolder, the distinction between `build.sc` and `module.sc` allows us to easily find the "enclosing" project root. 2. It ensures that any folders containing `build.sc`/`module.sc` files that accidentally get included within a Mill build do not end up getting picked up and confusing the top-level build, because we automatically skip any subfolders containing `build.sc` 3. Similarly, it ensures that adding a `build.sc` file "enclosing" an existing project, it would not affect Mill invocations in the inner project, because we only walk to the nearest enclosing `build.sc` file to find the project root 4. We do not automatically traverse deeply into sub-folders to discover `module.sc` files, which means that it should be almost impossible to accidentally pick up `module.sc` files that happen to be on the filesystem but you did not intend to include in the build This mechanism should do the right thing 99.9% of the time. For the last 0.1% where it doesn't do the right thing, we can add a `.millignore`/`.config/millignore` file to support ignoring things we don't want picked up, but I expect that should be a very rare edge case ## Task Resolution I have aimed to keep the logic in `resolve/` mostly intact. The core change is replacing `rootModule: BaseModule` with `baseModules: BaseModuleTree`, which provides enough metadata to allow `resolveDirectChildren` and `resolveTransitiveChildren` to find `BaseModule`s in sub-folders in addition to `Module` `object`s nested within the parent `Module`. Other than that, the logic should be basically unchanged, which hopefully should mitigate the risk of introducing new bugs ## Compatibility This change is not binary compatible, and the change in the `.sc` file desugaring is invasive enough we should consider it a major breaking change. This will need to go into Mill 0.12.x ## Out-Of-Scope/TODO 1. Running `mill` without a subfolder of the enclosing project. Shouldn't be hard to add given this design, but the PR is complicated enough as is that I'd like to leave it for follow up 2. Error reporting when a module is duplicated in an enclosing `object` and in a nested `module.sc` file. Again, probably not hard to add, but can happen in a follow up Pull request: #3213
1 parent e29eb1e commit 33fa953

File tree

68 files changed

+616
-656
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

68 files changed

+616
-656
lines changed

bsp/worker/src/mill/bsp/worker/State.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import mill.eval.Evaluator
99
private class State(evaluators: Seq[Evaluator], debug: String => Unit) {
1010
lazy val bspModulesById: Map[BuildTargetIdentifier, (BspModule, Evaluator)] = {
1111
val modules: Seq[(Module, Seq[Module], Evaluator)] = evaluators
12-
.map(ev => (ev.rootModule, JavaModuleUtils.transitiveModules(ev.rootModule), ev))
12+
.flatMap(ev => ev.rootModules.map(rm => (rm, JavaModuleUtils.transitiveModules(rm), ev)))
1313

1414
val map = modules
1515
.flatMap { case (rootModule, otherModules, eval) =>
@@ -30,7 +30,7 @@ private class State(evaluators: Seq[Evaluator], debug: String => Unit) {
3030
map
3131
}
3232

33-
lazy val rootModules: Seq[mill.define.BaseModule] = evaluators.map(_.rootModule)
33+
lazy val rootModules: Seq[mill.define.BaseModule] = evaluators.flatMap(_.rootModules)
3434

3535
lazy val bspIdByModule: Map[BspModule, BuildTargetIdentifier] =
3636
bspModulesById.view.mapValues(_._1).map(_.swap).toMap

build.sc

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,18 @@ trait MillStableScalaModule extends MillPublishScalaModule with Mima {
514514
),
515515
ProblemFilter.exclude[ReversedMissingMethodProblem](
516516
"mill.scalalib.JavaModule.mill$scalalib$JavaModule$$super$runMain"
517-
)
517+
),
518+
519+
// Not sure why mima is picking up this stuff which is private[mill]
520+
ProblemFilter.exclude[Problem]("mill.resolve.*.resolve0"),
521+
ProblemFilter.exclude[Problem]("mill.resolve.*.resolveRootModule"),
522+
523+
// These methods are private so it doesn't matter
524+
ProblemFilter.exclude[ReversedMissingMethodProblem]("mill.resolve.Resolve.handleResolved"),
525+
ProblemFilter.exclude[Problem]("mill.resolve.*.resolveNonEmptyAndHandle*"),
526+
ProblemFilter.exclude[Problem]("mill.resolve.ResolveCore*"),
527+
ProblemFilter.exclude[InheritedNewAbstractMethodProblem]("mill.main.MainModule.mill$define$BaseModule0$_setter_$watchedValues_="),
528+
ProblemFilter.exclude[InheritedNewAbstractMethodProblem]("mill.main.MainModule.mill$define$BaseModule0$_setter_$evalWatchedValues_="),
518529
)
519530
def mimaPreviousVersions: T[Seq[String]] = Settings.mimaBaseVersions
520531

ci/mill-bootstrap.patch

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
diff --git a/build.sc b/build.sc
2+
index 526ebaa9cf..616be491f9 100644
3+
--- a/build.sc
4+
+++ b/build.sc
5+
@@ -1968,7 +1968,7 @@ def uploadToGithub(authKey: String) = T.command {
6+
7+
private def resolveTasks[T](taskNames: String*): Seq[NamedTask[T]] = {
8+
mill.resolve.Resolve.Tasks.resolve(
9+
- build,
10+
+ build.`package`,
11+
taskNames,
12+
SelectMode.Separated
13+
).map(x => x.asInstanceOf[Seq[mill.define.NamedTask[T]]]).getOrElse(???)

contrib/bloop/src/mill/contrib/bloop/BloopImpl.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ class BloopImpl(evs: () => Seq[Evaluator], wd: os.Path) extends ExternalModule {
120120
val evals = evs()
121121
evals.flatMap { eval =>
122122
if (eval != null)
123-
JavaModuleUtils.transitiveModules(eval.rootModule, accept)
123+
eval.rootModules.flatMap(JavaModuleUtils.transitiveModules(_, accept))
124124
.collect { case jm: JavaModule => jm }
125125
else
126126
Seq.empty

contrib/scoverage/src/mill/contrib/scoverage/ScoverageReport.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,15 @@ trait ScoverageReport extends Module {
9191
dataTargets: String
9292
): Task[PathRef] = {
9393
val sourcesTasks: Seq[Task[Seq[PathRef]]] = Resolve.Tasks.resolve(
94-
evaluator.rootModule,
94+
evaluator.rootModules,
9595
Seq(sources),
9696
SelectMode.Separated
9797
) match {
9898
case Left(err) => throw new Exception(err)
9999
case Right(tasks) => tasks.asInstanceOf[Seq[Task[Seq[PathRef]]]]
100100
}
101101
val dataTasks: Seq[Task[PathRef]] = Resolve.Tasks.resolve(
102-
evaluator.rootModule,
102+
evaluator.rootModules,
103103
Seq(dataTargets),
104104
SelectMode.Separated
105105
) match {

example/javaweb/4-hello-micronaut/build.sc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ trait MicronautModule extends MavenModule{
7070
7171
> mill runBackground
7272
73-
> curl http://localhost:8087/hello
73+
> curl http://localhost:8088/hello
7474
...Hello World...
7575
7676
> mill clean runBackground
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
#Tue Jun 18 12:49:41 UTC 2024
22
micronaut.application.name=default
3-
micronaut.server.port=8087
3+
micronaut.server.port=8088

example/misc/8-multi-build-file/bar/module.sc

Whitespace-only changes.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import mill._, scalalib._
2+
3+
object module extends build.MyModule {
4+
def ivyDeps = Agg(ivy"com.lihaoyi::scalatags:0.8.2")
5+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package bar.qux
2+
import scalatags.Text.all._
3+
object BarQux {
4+
def printText(text: String): Unit = {
5+
val value = p("world")
6+
println("BarQux.value: " + value)
7+
}
8+
def main(args: Array[String]) = printText(args(0))
9+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import mill._, scalalib._
2+
3+
trait MyModule extends ScalaModule {
4+
def scalaVersion = "2.13.11"
5+
}
6+
7+
// Example Docs
8+
9+
10+
/** Usage
11+
12+
> ./mill resolve __
13+
bar
14+
...
15+
bar.qux.module
16+
...
17+
bar.qux.module.compile
18+
...
19+
foo
20+
...
21+
foo.compile
22+
23+
> ./mill bar.qux.module.compile
24+
25+
> ./mill foo.compile
26+
27+
> ./mill foo.run --foo-text hello --bar-qux-text world
28+
Foo.value: hello
29+
BarQux.value: <p>world</p>
30+
*/
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import mill._, scalalib._
2+
3+
object build extends RootModule with MyModule {
4+
def moduleDeps = Seq(bar.qux.module)
5+
def ivyDeps = Agg(ivy"com.lihaoyi::mainargs:0.4.0")
6+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package foo
2+
import mainargs.{main, ParserForMethods, arg}
3+
object Foo {
4+
val value = "hello"
5+
6+
@main
7+
def main(@arg(name = "foo-text") fooText: String,
8+
@arg(name = "bar-qux-text") barQuxText: String): Unit = {
9+
println("Foo.value: " + Foo.value)
10+
bar.qux.BarQux.printText(barQuxText)
11+
}
12+
13+
def main(args: Array[String]): Unit = ParserForMethods(this).runOrExit(args)
14+
}

example/src/mill/integration/ExampleTestSuite.scala

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,17 @@ object ExampleTestSuite extends IntegrationTestSuite {
7171
}
7272

7373
test("exampleUsage") {
74+
7475
val parsed = upickle.default.read[Seq[(String, String)]](sys.env("MILL_EXAMPLE_PARSED"))
7576
val usageComment = parsed.collect { case ("example", txt) => txt }.mkString("\n\n")
7677
val commandBlocks = ("\n" + usageComment.trim).split("\n> ").filter(_.nonEmpty)
7778

7879
retryOnTimeout(3) {
79-
try {
80-
for (commandBlock <- commandBlocks) processCommandBlock(workspaceRoot, commandBlock)
81-
if (integrationTestMode != "fork") evalStdout("shutdown")
82-
} finally {
83-
try os.remove.all(workspaceRoot / "out")
84-
catch { case e: Throwable => /*do nothing*/ }
85-
}
80+
try os.remove.all(workspaceRoot / "out")
81+
catch { case e: Throwable => /*do nothing*/ }
82+
83+
for (commandBlock <- commandBlocks) processCommandBlock(workspaceRoot, commandBlock)
84+
if (integrationTestMode != "fork") evalStdout("shutdown")
8685
}
8786
}
8887
}

idea/src/mill/idea/GenIdeaImpl.scala

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ case class GenIdeaImpl(
2222
)(implicit ctx: Ctx) {
2323
import GenIdeaImpl._
2424

25-
val workDir: os.Path = evaluators.head.rootModule.millSourcePath
25+
val workDir: os.Path = evaluators.head.rootModules.head.millSourcePath
2626
val ideaDir: os.Path = workDir / ".idea"
2727

2828
val ideaConfigVersion = 4
@@ -67,7 +67,9 @@ case class GenIdeaImpl(
6767
fetchMillModules: Boolean = true
6868
): Seq[(os.SubPath, scala.xml.Node)] = {
6969

70-
val rootModules = evaluators.zipWithIndex.map { case (ev, idx) => (ev.rootModule, ev, idx) }
70+
val rootModules = evaluators.zipWithIndex.map { case (ev, idx) =>
71+
(ev.rootModules.head, ev, idx)
72+
}
7173
val transitive: Seq[(BaseModule, Seq[Module], Evaluator, Int)] = rootModules
7274
.map { case (rootModule, ev, idx) =>
7375
(rootModule, JavaModuleUtils.transitiveModules(rootModule), ev, idx)
@@ -118,7 +120,7 @@ case class GenIdeaImpl(
118120
}
119121

120122
// is head the right one?
121-
val buildDepsPaths = Classpath.allJars(evaluators.head.rootModule.getClass.getClassLoader)
123+
val buildDepsPaths = Classpath.allJars(evaluators.head.rootModules.head.getClass.getClassLoader)
122124
.map(url => os.Path(java.nio.file.Paths.get(url.toURI)))
123125

124126
def resolveTasks: Map[Evaluator, Seq[Task[ResolvedModule]]] =

integration/failure/compile-error/test/src/CompileErrorTests.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ object CompileErrorTests extends IntegrationTestSuite {
1515
assert(res.err.contains("""println(doesntExist)"""))
1616
assert(res.err.contains("""qux.sc:3:34: type mismatch;"""))
1717
assert(res.err.contains(
18-
"""build.sc:8:5: value noSuchMethod is not a member of object build.this.foo"""
18+
"""build.sc:8:5: value noSuchMethod is not a member of object"""
1919
))
2020
}
2121
}

integration/failure/cross-collisions/test/src/CrossCollisionsTests.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ object CrossCollisionsTests extends IntegrationTestSuite {
99
test("detect-collision") {
1010
val res = evalStdout("resolve", "foo._")
1111
assert(!res.isSuccess)
12+
assert(res.err.contains("Cross module "))
1213
assert(
1314
res.err.contains(
14-
"Cross module millbuild.build#foo contains colliding cross values: List(List(a, b), List(c)) and List(List(a), List(b, c))"
15+
" contains colliding cross values: List(List(a, b), List(c)) and List(List(a), List(b, c))"
1516
)
1617
)
1718
}

integration/failure/multiple-top-level-modules/test/src/MultipleTopLevelModulesTests.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ object MultipleTopLevelModulesTests extends IntegrationTestSuite {
1010
val res = evalStdout("resolve", "_")
1111
assert(!res.isSuccess)
1212
assert(res.err.contains(
13-
"Only one RootModule can be defined in a build, not 2: millbuild.build$bar$,millbuild.build$foo$"
13+
"Only one RootModule can be defined in a build, not 2:"
1414
))
1515
}
1616
}

integration/feature/docannotations/test/src/DocAnnotationsTests.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ object DocAnnotationsTests extends IntegrationTestSuite {
3737

3838
assert(eval("inspect", "core.target"))
3939
val target = ujson.read(meta("inspect"))("value").str
40-
pprint.log(target)
4140
assert(
4241
globMatches(
4342
"""core.target(build.sc:...)

integration/feature/foreign/repo/conflict/build.sc

Lines changed: 0 additions & 23 deletions
This file was deleted.

integration/feature/foreign/repo/conflict/inner/build.sc

Lines changed: 0 additions & 3 deletions
This file was deleted.

integration/feature/foreign/repo/outer/build.sc

Lines changed: 0 additions & 30 deletions
This file was deleted.

integration/feature/foreign/repo/outer/inner/build.sc

Lines changed: 0 additions & 13 deletions
This file was deleted.

0 commit comments

Comments
 (0)